Debian Bug report logs - #287651
grass: Multiple vulnerabilities (symlink attacks) due to improper temporary files use in scripts and source code

grave

[Message part 1 (text/plain, inline)]
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

[signature.asc (application/pgp-signature, inline)]
[Message part 1 (text/plain, inline)]
> GRASS bugtracker:
> this bug's URL: http://intevation.de/rt/webrt?serial_num=2877

> Debian Bug #287651:
> http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=287651

Temporary files in GRASS C modules should use G_tempfile() or G__tempfile(), and GRASS scripts should be using the g.tempfile module:

GRASS 5.7.cvs:~ > g.tempfile --help
Description:
 Creates a temporary file and prints the file name.
Usage:
 g.tempfile pid=value
Parameters:
  pid   Process id to use when naming the tempfile

Which writes into $MAPSET/.tmp/ ....

Most of the GRASS scripts that have been rewritten for 5.7 use g.tempfile already.
Remaining to do:

grass57-cvs$ grep -r '/tmp/' scripts/* | cut -f1 -d: | uniq
scripts/i.oif/i.oif
scripts/i.oif/i.oifcalc
scripts/i.spectral/i.spectral
scripts/r.plane/r.plane
scripts/r.regression.line/r.regression.line
scripts/r.univar.sh/r.univar.sh

So not an unmanageable task.

Please audit:
general/g.tempfile/main.c
   http://freegis.org/cgi-bin/viewcvs.cgi/grass51/general/g.tempfile/main.c

lib/gis/tempfile.c
   http://freegis.org/cgi-bin/viewcvs.cgi/grass51/lib/gis/tempfile.c

The 5.0 line is pretty much EOL'd at this point. Any patching is up to the Debian packagers to do. Rather than spending time fixing all, I'd suggest checking which modules are actually built, a large percentage are not.

These should probably be fixed in the 5.4 CVS (subject to volunteer &/or external 5.0.x patching + merge [5.0->5.4 should be mostly drop in]).

If no one else gets to it first I can work on updating the remaining 5.7 scripts after I get back to work sometime after the new year (*Also I think there are still some if [ $opt = (null) ] tests which still need to be changed to if [ -z "$opt" ]).

It will be up to the debian packagers to decide if they want to back port these changes to 5.7.0 or wait for the 6.0beta series [6.0->5.7.0 should be mostly drop in].

A culled more general search of the 5.7 code turns up these as well:
imagery/i.ask/popup.c
lib/db/stubs/BUILD.PROTO
lib/db/dbmi_driver/mk_dbstubs_h.sh
lib/gis/unix_socks.c
lib/gis/gislib.dox
lib/gis/win32_pipes.c
lib/init/init.sh
lib/init/make_location_epsg_g57.sh
raster/r.digit/main.c
raster/r.median/main.c
raster/r.terraflow/description.html
raster/r.terraflow/main.cc
vector/v.out.ogr/description.html

from the top o' the year,
Hamish

---------------------------------
Do you Yahoo!?
 Yahoo! Mail - Find what you need with new enhanced search. Learn more.
[Message part 2 (text/html, inline)]
[Message part 1 (text/plain, inline)]
Tags 287651 +patch
thanks

I'm attaching an interdiff patch that fixes all the scripts in src/scripts.
There's a lot more source code in the package, but this is a start.

I'll keep working on the rest of the source code.

-- 
 Bessos,    (o_
    Marga.  (\)_

[all-scrips.diff (text/plain, attachment)]
[thanks for the 5.0.3 patch Marga]

Just an update re. less-insecure tempfiles ..

In the upstream GRASS 5.7 CVS[*] pretty much everything in the scripts/
directory now uses g.tempfile. C modules are next. I am not sure what to
do with the init scripts & libs where the GRASS tempfile fn's may not be
available..

These fixes are not in Steve Halasz's grass 6.0beta1 grass package[**],
I'm not sure when 6beta2 will be but maybe Steve & co. are willing to
backport these changes to 6beta1 and push for that to get into Sarge.

[*]  http://freegis.org/cgi-bin/viewcvs.cgi/grass51/
[**] http://pkg-grass.alioth.debian.org/cgi-bin/wiki.pl

a number of the instances on the offender list were actually commented 
out, etc. 

still to look at:

lib/db/stubs/BUILD.PROTO
lib/db/dbmi_driver/mk_dbstubs_h.sh
lib/gis/unix_socks.c
lib/gis/gislib.dox
lib/gis/win32_pipes.c
lib/init/init.sh
lib/init/make_location_epsg_g57.sh

raster/r.terraflow/description.html
raster/r.terraflow/main.cc

regards,
Hamish

Hamish wrote:

> Just an update re. less-insecure tempfiles ..
> 
> In the upstream GRASS 5.7 CVS[*] pretty much everything in the scripts/
> directory now uses g.tempfile. C modules are next. I am not sure what to
> do with the init scripts & libs where the GRASS tempfile fn's may not be
> available..

Re-write g.tempfile so that it doesn't rely upon GRASS having been
initialised, i.e. just use tempnam() or similar rather than relying
upon G_getenv() etc.

The only code which really needs to use G_tempfile() is code which
creates files within the GRASS database (e.g. G_open_cell_new() etc),
as the files have to reside on the same filesystem as the rest of the
database.

Everything else can use $TMPDIR.

-- 
Glynn Clements <glynn@gclements.plus.com>

[Message part 1 (text/plain, inline)]
This package has been dropped from testing, due to the fact that upstream
does not consider it to be a secure package at all, and that they don't
bother too much at all this insecurities.

In any case, I'm submitting now a patch that includes all the work till the
point I stopped, in the hopes that someone else might take it up from
there.

The patch is in the interdiff form.

-- 
 Bezos,     (o.
     Marga. (/)_

[last-work.diff (text/plain, attachment)]
[cc bug lists to archive the link]

> This page describes a way to create a secure tmp directory where you
> can create tmp files without worrying about their names:
> 
> http://www.linuxsecurity.com/content/view/115462/151/#mozTocId316364
..
> > Maybe someone can help me with this one:
> > lib/db/stubs/BUILD.PROTO

Thanks, but as I can't find anything that actually uses that script I'm
just going to remove it if no one objects.

That leaves r.terraflow as the only one left (I think); I'm waiting for
an update from the module's author.

Hamish

Hi, for those playing along at home, time for a status update:

r.terraflow is the only module in GRASS 6.0 CVS which hasn't been fixed
for this bug yet (end user set-able but uses "/var/tmp" as default).

You can make a GRASS package without the r.terraflow module by doing:
./configure --without-cxx

this has no repercussions on the rest of the package.

Hopefully we can have a GRASS 6beta2 release soon with r.terraflow fixed
and a new debian package made from that. If you don't want to wait, pull
from CVS and do --without-cxx.

see the pkg-grass mailing list at Alioth for more info.
  http://lists.alioth.debian.org/mailman/listinfo/pkg-grass-general

best,
Hamish

On Thu, 2005-02-03 at 12:55 +1300, Hamish wrote:
> Hi, for those playing along at home, time for a status update:
> 
> 
> r.terraflow is the only module in GRASS 6.0 CVS which hasn't been fixed
> for this bug yet (end user set-able but uses "/var/tmp" as default).
> 
> You can make a GRASS package without the r.terraflow module by doing:
> ./configure --without-cxx
> 
> this has no repercussions on the rest of the package.
> 
> 
> Hopefully we can have a GRASS 6beta2 release soon with r.terraflow fixed
> and a new debian package made from that. If you don't want to wait, pull
> from CVS and do --without-cxx.

Hamish,

You rock! I'll try to get a CVS package squared away tomorrow. Best to
do it as quickly as possible I think.

Thanks,
Steve

> I'll try to get a CVS package squared away tomorrow.

I have just reverted that init.sh $TMPDIR change now, so it should be
all set for a fresh checkout, AFAICT.

> Best to do it as quickly as possible I think.

Yes, I hadn't been keeping up with the Debian Weekly News & the sarge
release appears to be much closer than I thought it was.

Hamish

Note new experimental grass packages by Steve Halasz can be found at:

http://pkg-grass.alioth.debian.org/cgi-bin/wiki.pl?DebianGisRepository

Currently at GRASS version 6.0.0beta2 (which among other things fixes
this bug).

This should be ready for unstable soon.

thanks to the folks at the Debian GIS Project,
Hamish

Source: grass
Source-Version: 5.7.0+6.0.0beta2-1

We believe that the bug you reported is fixed in the latest version of
grass, which is due to be installed in the Debian FTP archive:

grass-doc_5.7.0+6.0.0beta2-1_all.deb
  to pool/main/g/grass/grass-doc_5.7.0+6.0.0beta2-1_all.deb
grass_5.7.0+6.0.0beta2-1.diff.gz
  to pool/main/g/grass/grass_5.7.0+6.0.0beta2-1.diff.gz
grass_5.7.0+6.0.0beta2-1.dsc
  to pool/main/g/grass/grass_5.7.0+6.0.0beta2-1.dsc
grass_5.7.0+6.0.0beta2-1_alpha.deb
  to pool/main/g/grass/grass_5.7.0+6.0.0beta2-1_alpha.deb
grass_5.7.0+6.0.0beta2.orig.tar.gz
  to pool/main/g/grass/grass_5.7.0+6.0.0beta2.orig.tar.gz
libgrass-dev_5.7.0+6.0.0beta2-1_alpha.deb
  to pool/main/g/grass/libgrass-dev_5.7.0+6.0.0beta2-1_alpha.deb
libgrass_5.7.0+6.0.0beta2-1_alpha.deb
  to pool/main/g/grass/libgrass_5.7.0+6.0.0beta2-1_alpha.deb

A summary of the changes between this version and the previous one is
attached.

Thank you for reporting the bug, which will now be closed.  If you
have further comments please address them to 287651@bugs.debian.org,
and the maintainer will reopen the bug report if appropriate.

Debian distribution maintenance software
pp.
Steve Halasz <debian@adkgis.org> (supplier of updated grass package)

(This message was generated automatically at their request; if you
believe that there is a problem with it please contact the archive
administrators by mailing ftpmaster@debian.org)

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Format: 1.7
Date: Fri,  4 Feb 2005 15:13:26 -0500
Source: grass
Binary: libgrass-dev grass libgrass grass-doc
Architecture: source alpha all
Version: 5.7.0+6.0.0beta2-1
Distribution: unstable
Urgency: high
Maintainer: Debian GIS Project <pkg-grass-devel@lists.alioth.debian.org>
Changed-By: Steve Halasz <debian@adkgis.org>
Description: 
 grass      - Geographic Resources Analysis Support System
 grass-doc  - Geographic Resources Analysis Support System documentation
 libgrass   - GRASS GIS development libraries
 libgrass-dev - GRASS GIS library development files
Closes: 234275 259655 261726 264566 282567 287590 287591 287651 287763 287764
Changes: 
 grass (5.7.0+6.0.0beta2-1) unstable; urgency=high
 .
   * New upstream release (Closes: #264566)
     - Safe tmpdir creation (Closes: #287651)
     - tcltkgrass replaced by d.m (Closes: #282567)
     - r.in.gdal segfault fixed (Closes: #234275)
     - r.in.bin segfault fixed (Closes: #259655)
     - r.lags.1grass.gz: "name" section too long (removed) (Closes: #261726)
     - raster.html: non explained commands (removed) (Closes: #287590)
     - i.rectify.html: links to i.vpoints.html work (Closes: #287764)
     - i.points.html: imagery.ps link fixed (Closes: #287763:)
   * Help button doc path fixed (Closes: #287591)
   * Change libgrass0 -> libgrass
Files: 
 c4c9302c14771ab6577fafde6980521a 1079 science optional grass_5.7.0+6.0.0beta2-1.dsc
 01b722319bdefe95a6525d769d564b5b 7676197 science optional grass_5.7.0+6.0.0beta2.orig.tar.gz
 4d6fd61a5e1597e7a5491722222a708a 24211 science optional grass_5.7.0+6.0.0beta2-1.diff.gz
 f077ff9b86c66c42b003347d2bed6559 5808222 science optional grass_5.7.0+6.0.0beta2-1_alpha.deb
 4c30d0c84fd5678ff494383819774535 381080 science optional grass-doc_5.7.0+6.0.0beta2-1_all.deb
 94363f4d5e0a2b8168938fcd5e4931ec 974758 libs optional libgrass_5.7.0+6.0.0beta2-1_alpha.deb
 4d76a06afd90ab15b6246bf2661bfb61 246992 libdevel optional libgrass-dev_5.7.0+6.0.0beta2-1_alpha.deb

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.5 (GNU/Linux)

iD8DBQFCDRVc0fhX0Y/ocz0RAsc1AKCWH69mEeDPc1Hhtv9zT9oREdeXigCeMojJ
Ky/4U2P+Y0VstZ0DwgT08XQ=
=ra2Z
-----END PGP SIGNATURE-----

Debian Bug report logs - #287651 grass: Multiple vulnerabilities (symlink attacks) due to improper temporary files use in scripts and source code

Debian Bug report logs - #287651
grass: Multiple vulnerabilities (symlink attacks) due to improper temporary files use in scripts and source code