Re: [suse-security] Insecure temp file creation fix - peer review please

27 Aug 2004

      Hello,

Am Mittwoch, 25. August 2004 03:24 schrieb Derek Fountain:
...
A few days back I noticed that the /usr/bin/asciiview script from the
aalib-1.4.0-275 package in SUSE-9.1 used insecure temp file creation.
The exploit is trivial and allows an attacker to cause a victim to
overwrite any of the victim's files.
aka "symlink attack", i assume.
...
The project over at Sourceforge (http://aa-project.sourceforge.net)
appears to be dead, having had no update for 3 years. Emails to the
two maintainers (at least the email addresses found in the SUSE RPM
information) came bouncing back. So I thought I'd fix the bug
myself... :) Since the script is small, I can post it here - see
below. Perhaps someone with a bit more experience at this sort of
thing can have a look at it to see if I've done it properly?
[...]
...
#!/bin/bash
# asciiview - an ascii art image browser script. Front end for
aview/aaflip
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;
}
...
FIFO=$TDIR/aview$$.pgm
Also insecure, see above.
...
clear()
{
  kill $! 2>/dev/null
  rm -f $FIFO 2>/dev/null
  rmdir $TDIR 2>/dev/null
}
myconvert()
{
   if anytopnm $1 >$FIFO 2>/dev/null ; then
                 ^^
Variables should be quoted:   "$1"
...
exit
   elif convert -colorspace gray $1 pgm:- 2>/dev/null ; then
^^
missing quoting again.
...
exit
   fi
   echo "Failed to convert file format to PNM by both convert and
anytopnm" >&2
while true; do
     echo "0 "
   done
This is an endless loop just printing "0 " on your screen.
...
}
...
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.
...
for name in $filenames ; do
if test -r $name ; then
case $name in
       ^^^^^
       Quoting!   "$name"
...
*.fli | *.lfc | *.flic )
  PATH="$PATH:."
  aaflip $options $name
                    ^^^^^
                    Quoting!
...
;;
*)
  myconvert $name >$FIFO &
              ^^^^^
              guess what ;-)
...
[...]
Yours,

Christian Boltz
-- 
noch bis Montag, 30.8.: Weinkerwe in Insheim - www.insheim.de

3.-5.9.2004: Hoffest der Landjugend Insheim
www.landjugend-insheim.de