partimage creates the file /tmp/partimage.count insecurely (FINISH_LAST_COUNTFILE) (note: it creates the lockfile securely, although using a hardcoded name like this obviously prevents multiple simulatneous uses) Ideally the countfile code should be using mkstemp. Strangely, there appears to be an undocumented `--runshell` option that executes a file in /tmp, obviously this is a terrible idea and support for this option should be #if 0'ed out. Ideally the countfile code should be using mkstemp. patch will be attached shortly.
Tavis do you have a patch for this?
anyone got a patch for this?
Created attachment 113491 [details, diff] Patch to fix hardcoded "FINISH_LAST_COUNTFILE" in /tmp I think this patch should do the trick.
Pulling in herd for advise.
that patch is weird mkstemp() fails yet you return EXIT_SUCCESS ? mkstemp() returns the file descriptor, yet you turn around and run open() on the tempfile a second time ? you probably want: nLockFile = mkstemp(...); if (nLockFile == -1) ... and then: int fd = mkstemp(...); if (fd == -1) ... nCountFile = fdopen(fd, ...);
Created attachment 115523 [details, diff] Patch to fix "FINISH_LAST_COUTNFILE" in /tmp This one should work much better (note to self: never try to write code when you haven't slept enough). Vapier: you're right about EXIT_SUCCESS, but if you look at the original code, the author also used it even in case of failure... Taviso told me not to touch it, so now it returns EXIT_FAILURE just in cases related to the patch, but not on other cases.I don't know if it's really better, but at least it fixes the issue.
any news here? someone tested the new patch?
still not there yet ;) the first do{}while() loop can leak file descriptors ... fd_lock = mkstemp(...); ... nLockFile = fdopen(fd_lock, "r"); if (nLockFile == NULL) { ... retry process ... really, the "if (fd_lock)" should be causing the retry and the "if (nLockFile == NULL)" should close(fd_lock) also, in the case where fd_countfile fails, you leak fd_lock and nLockFile ... and in the case where nCountFile fails, you leak those two plus fd_countfile
From the 0.6.6 ChangeLog: o applied gentoo patch that fixes the insecure temporary file creation (Thanks to Pierre-Yves Rofes) There's a version bump bug with ebuilds, bug #117882.
I had forgotten this one :/ Currently the patch is still not okay, I'll have to correct the things mentioned in comment #8. I'll try to do it soon.
Pierre-Yves, any update here?
already in cvs, thanks.
Please do not close security bugs. Christian, are the issues mentioned in comment 8 taken care of?
(In reply to comment #13) > Please do not close security bugs. > > > Christian, are the issues mentioned in comment 8 taken care of? > Nope, they're not.
ops, I've missed #8 ...sorry
What is the status here? Any news? 0.6.7 is the current version now... is it still affected?
so here's the thing: partimage included p-y's patch, but as vapier pointed out, this is not entirely correct. p-y: can you come up with a fixed patch?
Created attachment 176426 [details, diff] final? fix for the mkstemp problem I was just scratching my head on this one once more. Attached is a patch fixing the issue in comment #8. This would be a HELL of a lot easier if i'd be allowed to drop the debug printout and do if (mkstemp == -1 || fdopen == NULL) cleanup(); but i hope the patch is as clear as possible and i haven't missed a variable in the process.
Matti, are you posting this for review on this bug or did you propose it upstream?
the former. i'll propose it to upstream, if either we agree that it's correct or nobody responds :)
Reviewed, looks sane to me. Please propose it upstream.
(In reply to comment #21) > Reviewed, looks sane to me. Please propose it upstream. > Upstream fixed this in 0.6.8, now in Portage.
Arches, please test and mark stable: =sys-block/partimage-0.6.8 Target keywords : "amd64 ppc x86" Already stabled : "amd64 x86" Missing keywords: "ppc"
Marked ppc stable.
For some reason (target-tool bug?), sparc was missing. Sparc, please test and mark stable: =sys-block/partimage-0.6.8
sparc is not stable
GLSA Vote: Yes.
Yes, too, GLSA request filed.
This issue was resolved and addressed in GLSA 201412-08 at http://security.gentoo.org/glsa/glsa-201412-08.xml by GLSA coordinator Sean Amoss (ackle).
Think you for this patch, really helpful. JR | https://elmontecustomcabinets.com/
Greetings! Very useful advice in this particular article! It's the little changes that make the most significant changes. Thanks a lot for sharing!
