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

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.

[Recipient list trimmed.]

Hamish wrote:

> 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:

However, G_tempfile() serves a dual purpose. It is used for both
normal temporary files and for internal GRASS data files (e.g. the
cell array).

[What follows is from memory; I haven't yet re-installed the GRASS
source code following a recent disk failure.]

In the latter case, G_open_cell_new() etc uses G_tempfile() to create
the actual cell file. Once the map is closed with G_close_cell(), the
new file is moved into place (possibly replacing an existing file)
using rename() or link().

In that situation, the file has to be created on the filesystem (i.e.
partition) where it will eventually end up, i.e. the one which
contains where mapset directory resides, which is why the files are
created unde $LOCATION/.tmp.

OTOH, "normal" temporary files could reasonably be created in e.g.
$TMPDIR (tempnam() automatically uses $TMPDIR if it is set), which
should eliminate any security issues ($TMPDIR typically resides under
$HOME, and is only writable by its owner).

That would require separate functions for the two cases. The
(relatively few) libgis functions which use G_tempfile() for the
specific reasons described above would use the function which behaves
like the existing G_tempfile(), while most other code would use the
function which uses /tmp or $TMPDIR.

Given that relatively little code relies upon the specific location of
the temporary file, the easiest solution would be to give the existing
G_tempfile() function a new name and call the new function
G_tempfile().

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