[GRASS5] [bug #2877] (grass) Insecure tempfile creation

Roger Bivand Roger.Bivand at nhh.no
Wed Dec 29 16:02:28 EST 2004


On Wed, 29 Dec 2004, Steve Halasz wrote:

> On Wed, 2004-12-29 at 19:58 +0100, Stephan Holl wrote:
> > Dear javier,
> > 
> > thanks for pointing out the insecurities.
> > But the package you refer to is quite old.
> > 
> > Perhaps you do not know the debian-gis-mailinglist, which can be joined
> > here: http://lists.alioth.debian.org/mailman/listinfo/pkg-grass-general
> > 
> > There are affords to prepare an unofficial GRASS5.7-package, and
> > actually there is an apt-getable repository available.
> > Probably you would like to give it a try.
> > deb http://pkg-grass.alioth.debian.org/archive	unstable main contrib non-free
> > deb-src http://pkg-grass.alioth.debian.org/archive unstable main contrib
> > non-free
> > 
> > If you would like to send patches for debian, I suggest to create the
> > patches against this version, if the problems are still in there.
> 
> I've been working on this package and the few examples I checked are
> still in the 5.7.0 release. I suspect they are in CVS as well. That's
> why I took the liberty of forwarding this bug to the grass bug system.
> As far as debian goes, it is probably best to patch the 5.0.3 package in
> most cases since that has the best chance of making it into the next
> release. I don't think it would be hard in most cases to forward port
> these fixes.

Thanks for doing this - the GRASS developers are so few and have their
noses so close to the wheel, that seeing things like this, which are
vital, can be hard. If a per-session temporary directory is a good
solution, there may be code in the R codebase that can help, because that
is the way it is done there. Probably the full path and directory name
would have to be kept in the users' .grassrc*. My guess would be that
rather than modify 5.0.*, the decision would fall between 5.4 and 5.7,
with my preference for 5.7, because that is where most of the developers'
attention is - 5.4 should then be clearly marked to suit.

Security does matter, and this bug report is very relevant, and also 
suggests remedies.

Roger

> 
> Steve
> 
> > Thanks
> > 
> > 	Stephan
> > 
> > 
> > On Wed, 29 Dec 2004 17:28:52 +0100 (CET) Request Tracker
> > <grass-bugs at intevation.de> wrote:
> > 
> > > 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 at dat.etsit.upm.es Wed Dec 29 03:01:19 2004
> > > Return-path: <jfs at 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 at computer.org> To: submit at bugs.debian.org
> > > Subject: grass: Multiple vulnerabilities (symlink attacks) due to
> > > improper temporary files use in scripts and source code Message-ID:
> > > <20041229110116.GB7144 at 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 at 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
> > > 
> > > _______________________________________________
> > > grass5 mailing list
> > > grass5 at grass.itc.it
> > > http://grass.itc.it/mailman/listinfo/grass5
> > > 
> > 
> > 
> 
> _______________________________________________
> grass5 mailing list
> grass5 at grass.itc.it
> http://grass.itc.it/mailman/listinfo/grass5
> 

-- 
Roger Bivand
Economic Geography Section, Department of Economics, Norwegian School of
Economics and Business Administration, Breiviksveien 40, N-5045 Bergen,
Norway. voice: +47 55 95 93 55; fax +47 55 95 93 93
e-mail: Roger.Bivand at nhh.no





More information about the grass-dev mailing list