Re: fix mktemp (remove mktemp ;)
Dear diary, on Sun, Apr 17, 2005 at 01:27:43AM CEST, I got a letter where Paul Jackson [EMAIL PROTECTED] told me that... Remove mktemp usage - it doesn't work on some Mandrakes, nor on my SuSE 8.2 with mktemp-1.5-531. Replace with simple use of $$ (pid). I've been using this same pattern for 20 years on many production scripts; it's fast, solid and simple. And racy. And not guaranteed to come up with fresh new files. More robust tmp file removal, using trap, so that scripts interrupted by signals HUP, INT, QUIT or PIPE will cleanup. But I like this! I'm deferring those changes to the introduction of a git shell library, which several people volunteered to do so far, but noone sent me any patches for (the last one was probably Martin Mares, only few hours ago though). -- Petr Pasky Baudis Stuff: http://pasky.or.cz/ C++: an octopus made by nailing extra legs onto a dog. -- Steve Taylor - To unsubscribe from this list: send the line unsubscribe git in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: fix mktemp (remove mktemp ;)
And racy. And not guaranteed to come up with fresh new files. In theory perhaps. In practice no. Even mktemp(1) can collide, in theory, since there is no practical way in shell scripts to hold open and locked the file from the instant of it is determined to be a unique name. The window of vulnerability for shell script tmp files is the lifetime of the script - while the file sits there unlocked. Anyone else with permissions can mess with it. More people will fail, and are already failing, using mktemp than I have ever seen using $$ (I've never seen a documented case, and since such files are not writable to other user accounts, such a collision would typically not go hidden.) Fast, simple portable solutions that work win over solutions with some theoretical advantage that don't matter in practice, but also that are less portable or less efficient. -- I won't rest till it's the best ... Programmer, Linux Scalability Paul Jackson [EMAIL PROTECTED] 1.650.933.1373, 1.925.600.0401 - To unsubscribe from this list: send the line unsubscribe git in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: fix mktemp (remove mktemp ;)
On Sat, Apr 16, 2005 at 05:02:21PM -0700, Paul Jackson wrote: And racy. And not guaranteed to come up with fresh new files. In theory perhaps. In practice no. Even mktemp(1) can collide, in theory, since there is no practical way in shell scripts to hold open and locked the file from the instant of it is determined to be a unique name. Using the pid as a 'random' number is a bad idea. all an attacker has to do is create 65535 symlinks in /usr/tmp, and he can now overwrite any file you own. mktemp is being used here to provide randomness in the filename, not just a uniqueness. The window of vulnerability for shell script tmp files is the lifetime of the script - while the file sits there unlocked. Anyone else with permissions can mess with it. Attacker doesnt need to touch the script. Just take advantage of flaws in it, and wait for someone to run it. More people will fail, and are already failing, using mktemp than I have ever seen using $$ (I've never seen a documented case, and since such files are not writable to other user accounts, such a collision would typically not go hidden.) Fast, simple portable solutions that work win over solutions with some theoretical advantage that don't matter in practice, but also that are less portable or less efficient. I'd suggest fixing your distributions mktemp over going with an inferior solution. Dave - To unsubscribe from this list: send the line unsubscribe git in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: fix mktemp (remove mktemp ;)
On Sat, Apr 16, 2005 at 08:33:25PM -0400, Dave Jones wrote: On Sat, Apr 16, 2005 at 05:02:21PM -0700, Paul Jackson wrote: And racy. And not guaranteed to come up with fresh new files. In theory perhaps. In practice no. Even mktemp(1) can collide, in theory, since there is no practical way in shell scripts to hold open and locked the file from the instant of it is determined to be a unique name. Using the pid as a 'random' number is a bad idea. all an attacker has to do is create 65535 symlinks in /usr/tmp, and he can now overwrite any file you own. mktemp is being used here to provide randomness in the filename, not just a uniqueness. How about putting using .git/tmp.$$ or similar as tempfile? This should satisfy both the portability and security requirements, since the warnings against using $$ only apply to public directories. Regards, Erik - To unsubscribe from this list: send the line unsubscribe git in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: fix mktemp (remove mktemp ;)
Dave wrote:
http://www.linuxsecurity.com/content/view/115462/151/
Nice - thanks.
Pasky - would you be interested in a patch that used a more robust tmp
file creation, along the lines of replacing
t=${TMPDIR:-/usr/tmp}/gitdiff.$$
trap 'set +f; rm -fr $t.?; trap 0; exit 0' 0 1 2 3 15
with:
tmp=${TMPDIR-/tmp}
tmp=$tmp/gitdiff-do.$RANDOM.$RANDOM.$RANDOM.$$
(umask 077 mkdir $tmp) || {
echo Could not create temporary directory! Exiting. 12
exit 1
}
t=$tmp/tmp
trap 'rm -fr $tmp; trap 0; exit 0' 0 1 2 3 15
If interested, would you want it instead of my previous mktemp removal
patch, or on top of it?
--
I won't rest till it's the best ...
Programmer, Linux Scalability
Paul Jackson [EMAIL PROTECTED] 1.650.933.1373,
1.925.600.0401
-
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: fix mktemp (remove mktemp ;)
Erik wrote: How about putting using .git/tmp.$$ or similar as tempfile? One could, but best to normally honor the users TMPDIR setting. Could one 'git diff' a readonly git repository? Perhaps someone has a reason for putting their tmp files where they choose - say a local file system in a heavy NFS environment. -- I won't rest till it's the best ... Programmer, Linux Scalability Paul Jackson [EMAIL PROTECTED] 1.650.933.1373, 1.925.600.0401 - To unsubscribe from this list: send the line unsubscribe git in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: fix mktemp (remove mktemp ;)
No, you have to:
(a) create a unique, pid specific file name /var/tmp/myapp.$$.xyzzy
(b) create it in O_EXCL mode, so you wont smash another's held lock
(b-1) It worked, OK
(b-2) open failed, try ...xyzzz
repeat until (b-1)
There are thousands of examples of how to do this with bash.
Paul Jackson wrote:
Dave wrote:
mktemp is being used here to provide randomness in the filename,
not just a uniqueness.
Ok - useful point.
How about:
t=${TMPDIR:-/usr/tmp}/gitdiff.$$.$RANDOM
all an attacker has to do is create 65535 symlinks in /usr/tmp
the point of the xyzzy seed is to make creating all possible files
in-feasable.
And how about if I removed the tmp files at the top:
t=${TMPDIR:-/usr/tmp}/gitdiff.$$.$RANDOM
trap 'rm -fr $t.?; trap 0; exit 0' 0 1 2 3 15
rm -fr $t.?
... rest of script ...
How close does that come to providing the same level of safety, while
remaining portable over a wider range of systems, and not requiring that
a separate command be forked?
I'd suggest fixing your distributions ...
It's not just my distro; it's the distros of all git users.
If apps can avoid depending on inessential details of their
environment, that's friendlier to all concerned.
And actually my distro is fine - it's just that I am running an old
version of it on one of my systems. Newer versions of the mktemp -t
option.
--
mit freundlichen Grüßen, Brian.
Dr. Brian O'Mahoney
Mobile +41 (0)79 334 8035 Email: [EMAIL PROTECTED]
Bleicherstrasse 25, CH-8953 Dietikon, Switzerland
PGP Key fingerprint = 33 41 A2 DE 35 7C CE 5D F5 14 39 C9 6D 38 56 D5
-
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: fix mktemp (remove mktemp ;)
No, you have to:
How does this compare with the one I posted about 1 hour 30
minuts ago:
tmp=${TMPDIR-/tmp}
tmp=$tmp/gitdiff-do.$RANDOM.$RANDOM.$RANDOM.$$
(umask 077 mkdir $tmp) || {
echo Could not create temporary directory! Exiting. 12
exit 1
}
t=$tmp/tmp
trap 'rm -fr $tmp; trap 0; exit 0' 0 1 2 3 15
derived from the reference that Dave Jones provided?
create it in O_EXCL mode,
What can one do that and hold that O_EXCL from within bash?
There are thousands of examples of how to do this with bash.
Care to provide one?
--
I won't rest till it's the best ...
Programmer, Linux Scalability
Paul Jackson [EMAIL PROTECTED] 1.650.933.1373,
1.925.600.0401
-
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
