Re: [suse-security] Insecure temp file creation fix - peer review please
On Saturday 28 August 2004 05:15, Christian Boltz wrote:
aka "symlink attack", i assume.
Yup... :o)
TDIR=${TMPDIR:-/tmp}/aview_$$
Insecure. $$ is guessable (or, worst case: for i in `seq 2 33000 ; do ln -s /home/victim/Mail/inbox /tmp/aview_$i ; done - no more need to guess ;-)
Use mktemp instead:
TDIR=`mktemp -d /tmp/aview.XXXXXXXXXX` || { echo "unable to create temp dir" >&2; exit 1; }
I avoided using mktemp because the aalib code runs on lots of different platforms. From what I've read, I can't be sure that mktemp is available on all of them. So....:
trap clear 0 (umask 077 && mkdir $TDIR) || { echo "Unable to create temp directory $TDIR" exit 1 } mkfifo $FIFO || { echo "Unable to create FIFO $FIFO" exit 1 }
These blocks are no longer needed because mktemp already creates the temp dir and fifo.
...these blocks are needed! I think that's the platform independent, secure way to create a temporary directory, and if there's a nasty link in place it will fail.
if anytopnm $1 >$FIFO 2>/dev/null ; then
^^ Variables should be quoted: "$1"
Good point! I only worried about the symlink issues (slaps own wrists)!
while true; do echo "0 " done
This is an endless loop just printing "0 " on your screen.
Yeah, weird eh? I decided to only concentrate on the security aspects of the script. Presumably the original author felt good reason to fill the screen with 0s!
Yours,
Christian Boltz
Thanks, I've learnt more useful stuff... :o)
Hello, Am Samstag, 28. August 2004 04:26 schrieb Derek's Lists:
On Saturday 28 August 2004 05:15, Christian Boltz wrote:
aka "symlink attack", i assume.
Yup... :o)
TDIR=${TMPDIR:-/tmp}/aview_$$
Insecure. $$ is guessable [...]
Use mktemp instead: [...]
I avoided using mktemp because the aalib code runs on lots of different platforms. From what I've read, I can't be sure that mktemp is available on all of them. So....:
... the "other" systems are just insecure because you can't create a secure tempfile
trap clear 0 (umask 077 && mkdir $TDIR) || { echo "Unable to create temp directory $TDIR" exit 1 } mkfifo $FIFO || { echo "Unable to create FIFO $FIFO" exit 1 }
These blocks are no longer needed because mktemp already creates the temp dir and fifo.
...these blocks are needed! I think that's the platform independent, secure way to create a temporary directory,
No, this is not really secure. As I already wrote, $$ is guessable (it's just a number between 2 and 32567 (or larger? Don't ask. Anyway, it's guessable.) If you want to avoid using mktemp for any reason, at least call test -e $TMPFILE && exit 1 This isn't really secure because it creates a race condition (someone could create $TMPFILE between the test and the mkdir call), but it's better than before.
and if there's a nasty link in place it will fail.
Not in every case. cb@cboltz:/tmp/test> md homedir # as a replacement of /home/victim/ cb@cboltz:/tmp/test> ln -s homedir/ tempdir cb@cboltz:/tmp/test> ls -l total 4 drwxr-xr-x 2 cb users 4096 Aug 28 17:57 homedir lrwxrwxrwx 1 cb users 8 Aug 28 17:57 tempdir -> homedir/ cb@cboltz:/tmp/test> md tempdir cb@cboltz:/tmp/test> If an attacker wants to hit you, he just has to run for i in `seq 2 33000 ; do ln -s /home/victim/Mail/ /tmp/aview_$i ; done Hope your script doesn't use a filename like "inbox"... Note: alias md='mkdir -p', without -p it will fail. But an attacker may lead your script into a denial of service by creating /tmp/aview_[0-9]* (not really dangerous with a manually called script, but maybe not so good if called in a cron job). OK, your script creates a tempdir, so there should be an error message and no overwritten files. But if you want to create a temp _file_, $$ can't be secure.
while true; do echo "0 " done
This is an endless loop just printing "0 " on your screen.
Yeah, weird eh? I decided to only concentrate on the security aspects of the script. Presumably the original author felt good reason to fill the screen with 0s!
;-) Gruß Christian Boltz -- noch bis Montag, 30.8.: Weinkerwe in Insheim 3.-5.9.2004: Hoffest der Landjugend Insheim www.landjugend-insheim.de
Christian Boltz wrote:
As I already wrote, $$ is guessable (it's just a number between 2 and 32567 (or larger? Don't ask. Anyway, it's guessable.)
Correct.
and if there's a nasty link in place it will fail.
Not in every case.
In your example you are using md which is an alias for "mkdir -p". "mkdir -p" was the source of some security flaws I found, but Derek is talking about "mkdir" without the -p option. Mkdir without -p will _always_ give you an error if there already is _anything_ with the same name (files, directories, symlinks, fifos...), so it is secure.
If an attacker wants to hit you, he just has to run
for i in `seq 2 33000 ; do ln -s /home/victim/Mail/ /tmp/aview_$i ; done
This will 'only' lead to denial of service (which you pointed out). But in case of an image viewer this is not really a threat. Apart from that, there are other, much easier ways to DoS SuSE/vanilla kernels if you are a local user. So if you are scared of DoS from local users, asciiview is your least concern. Btw, a nice read about creating tempfiles in shell programs can be found at [1], chapter 3.4 and 3.5. Regards nordi [1]http://www.linuxsecurity.com/articles/documentation_article-8886.html
participants (3)
-
Christian Boltz -
Derek's Lists -
nordi