this bug's URL: http://intevation.de/rt/webrt?serial_num=2877
-------------------------------------------------------------------------
Subject: Insecure tempfile creation
Platform: GNU/Linux/i386
grass obtained from: Trento Italy site
grass binary for platform: Compiled from Sources
GRASS Version: 5.7.0
This is a showstopper bug for Debian. Packages with security bugs cannot enter the stable distribution. This is Debian Bug #287651:
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=287651
Received: (at submit) by bugs.debian.org; 29 Dec 2004 11:01:19 +0000
From jfs@dat.etsit.upm.es Wed Dec 29 03:01:19 2004
Return-path: <jfs@dat.etsit.upm.es>
Received: from tornado.dat.etsit.upm.es (dat.etsit.upm.es) [138.100.17.73]
by spohr.debian.org with smtp (Exim 3.35 1 (Debian))
id 1CjbZu-00028E-00; Wed, 29 Dec 2004 03:01:18 -0800
Received: (qmail 10639 invoked by uid 1013); 29 Dec 2004 11:01:16 -0000
Date: Wed, 29 Dec 2004 12:01:16 +0100
From: Javier =?iso-8859-1?Q?Fern=E1ndez-Sanguino_Pe=F1a?= <jfs@computer.org>
To: submit@bugs.debian.org
Subject: grass: Multiple vulnerabilities (symlink attacks) due to improper temporary files use in scripts and source code
Message-ID: <20041229110116.GB7144@dat.etsit.upm.es>
Mime-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha1;
protocol="application/pgp-signature"; boundary="VrqPEDrXMn8OVzN4"
Content-Disposition: inline
User-Agent: Mutt/1.5.6+20040722i
Delivered-To: submit@bugs.debian.org
X-Spam-Checker-Version: SpamAssassin 2.60-bugs.debian.org_2004_03_25
(1.212-2003-09-23-exp) on spohr.debian.org
X-Spam-Status: No, hits=-8.0 required=4.0 tests=BAYES_00,HAS_PACKAGE
autolearn=no version=2.60-bugs.debian.org_2004_03_25
X-Spam-Level:
Package: grass
Version: 5.0.3-5.1
Priority: grave
Tags: security sarge sid
A lot of scripts provided withing Grass are vulnerable to race conditions
through symlink attacks in temporary files. Many of these scripts either
create temporary files in an insecure manner (shell scripts do not use 'set
-e' and 'set -C', for example) or/and are easily guessable.
Some examples include:
grass-5.0.3/src/scripts/contrib/i.oif/i.oif:
---------------------------------------------------------
(...)
# save the Stddev for TM bands
echo "Calculation Standarddeviations for all bands:"
$GISBASE/etc/i.oif/r.stddev $1 |tail -1 >/tmp/i.oif.stddev
(...)
---------------------------------------------------------
grass-5.0.3/src/CMD/generic/GISGEN.sh:
--------------------------------------------------------
case $# in
0)
tmp1=/tmp/GISGEN1.$$
tmp2=/tmp/GISGEN2.$$
tmp3=/tmp/GISGEN3.$$
rm -f $tmp1
touch $tmp1
(...)
rm -f $tmp2
rm -f $tmp3
echo "a == 1 { print \$0 ; next }" > $tmp3
echo "\$0 == \"$STEP\" { a = 1; print \$0 }" >> $tmp3
awk -f $tmp3 $tmp1 > $tmp2
rm -f $tmp3 $tmp1
----------------------------------------------------------
grass-5.0.3/src/mapdev/v.in.arc.poly/script/v.in.arc.poly
----------------------------------------------------------
bindir=$GISBASE/etc
tempfile=/tmp/temp.ply
(...)
echo 'start eliminating double nodes in ' $1
$bindir/permut $GISDBASE/$LOCATION_NAME/$MAPSET/arc/temp.ply $tempfile
rm $tempfile
(...)
----------------------------------------------------------
[Note: permut just opens this output file without further checks:
./src/mapdev/v.in.arc.poly/permut/permut.c
(...)
if ((outfile = fopen (out_ply, "w")) == NULL)
{
printf ("can't open tempfile %s\n", out_ply);
exit (1);
}
(...)
]
./src/paint/Drivers/versatec/3236/DRIVER.sh
----------------------------------------------------------
(...)
TMPDIR1=/tmp/versatec
TMPDIR2=/tmp/versatec
(...)
RASTERFILE=$TMPDIR1/_paint
SPRINT="/bin/sprint >&2"
SPRINT_COMMAND="$SPRINT $RASTERFILE -v -p 3236 -w $TMPDIR2 -x $ZOOM -y
$ZOOM"
----------------------------------------------------------
grass-5.0.3/src/scripts/contrib/i.spectral/i.spectral
----------------------------------------------------------
.where -1 |r.what input=$RASTERMAPS > /tmp/spectr.dum1
cat /tmp/spectr.dum1 | cut -d'|' -f4,5,6,7,8,9,10| tr '|' '\012' >
/tmp/spectr.dum2
----------------------------------------------------------
Now those are just exmaples of the "easily guessable" temporary files used.
But a lot of scripts make use of the $$ construct (either within shell
scripts or C code using getpid()) that is not directly guessable but can be
infered in a system where a given user is running grass more or less
accurately either:
- by looking at the /tmp/ directory and detecting when a given temporary
file is created and symlink the "next one". For example in
./src/scripts/contrib/r.plane/r.plane the following tmp files are created
in succession: /tmp/$$, /tmp/$$dip, /tmp/$$, /tmp/$$ea, /tmp/$$, /tmp/$$no,
/tmp/$$ (removed and reused several times). So an attacker
- by bulk creating a huge number of temporary files using the current PID
of the grass program as a base
Just try a 'grep -r "/tmp/"' on the sources and you'll see what I mean.
I cannot determine, as I don't use grass, wether the scripts there are
actually used regularly by users. I would suggest however to patch those
either by:
a) Safely creating a per user temporary directory and have all scripts use
that as a location for all of the temporary files if defined. For example,
in a common startup script do:
TMPGRASS = `mktemp -dt grass-XXXXXX` || { echo "Cannot create temporary
directory"; exit 1 ; }
export TMPGRASS
and in auxiliary scripts do:
[ ! -n "TMPGRASS" ] && TMPGRASS=`mktemp -dt grass-XXXXXX` || { echo "Cannot
create temporary directory"; exit 1 ; }
(...)
tempfile="$TMPGRASS/tempfile"
b) Changing all shell scripts to use mktemp or tempfile (might
make those Debian-specific) when setting up temporary files.
All the C files, however, need to be modified so that they use mkstemp().
So, for example, instead of this:
(in grass-5.0.3/src/imagery/i.ask/popup.c):
char tempfile1[40], tempfile2[40];
(...)
sprintf (tempfile1, "/tmp/i.ask1.%d", getpid());
sprintf (tempfile2, "/tmp/i.ask2.%d", getpid());
it should use this:
int tempfd1; int tempfd2;
if ( ( tempfd1 = mkstemp("/tmp/i.ask1.XXXXXX") ) < 0 ) {
/* Do something if this breaks! */
}
if ( ( tempfd2 = mkstemp("/tmp/i.ask2.XXXXXX") ) < 0 ) {
/* Do something if this breaks! */
}
and pass the filedescriptor (instead of the filename) to functions later
on. This means that ./src/libes/raster/Panel.c needs to be rewritten (or
extended to use fd instead of names in its call.
BTW, what does this mean?
grass-5.0.3/src/libes/raster/Panel.c
(...)
/* make sure this file can be written by anybody */
num = umask(0);
close(creat(name,0666));
umask(num);
(...)
!!!
Doesn't look too safe to me.. What's the panel used for?
Now, I'm not sure I can provide a patch fixing all of those, but I'm
willing to provide a full patch (at least for the shell scripts) if time
permits.
However, IMHO this makes this software package unsuitable for release.
Regards
Javier
-------------------------------------------- Managed by Request Tracker