[GRASS5] [bug #2877] (grass) Insecure tempfile creation
Steve Halasz
debian at adkgis.org
Wed Dec 29 14:32:26 EST 2004
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.
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
> >
>
>
More information about the grass-dev
mailing list