Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#1039 closed defect (fixed)

i.group segmentation fault using full path name

Reported by: huhabla Owned by: grass-dev@…
Priority: major Milestone: 7.0.0
Component: Imagery Version: svn-trunk
Keywords: i.group Cc: huhabla
CPU: x86-32 Platform: Linux

Description

i.group produces a segmentation fault when called with full path name.

GRASS 7.0.svn (spearfish60):~/src/grass7.0/grass_trunk > g.list group
----------------------------------------------
imagery group files available in mapset <user1>:
input_0    soil_group

GRASS 7.0.svn (spearfish60):~/src/grass7.0/grass_trunk > i.group -g group=soil_groupsoil.br.depth@PERMANENT
soils@PERMANENT
soils.Kfactor@PERMANENT
soils.Tfactor@PERMANENT
soils.ph@PERMANENT
soils.range@PERMANENT
i.group complete.

GRASS 7.0.svn (spearfish60):~/src/grass7.0/grass_trunk > valgrind --tool=memcheck /home/soeren/src/grass7.0/grass_trunk/dist.i686-pc-linux-gnu/bin/i.group -g group=soil_group
==5684== Memcheck, a memory error detector.
==5684== Copyright (C) 2002-2007, and GNU GPL'd, by Julian Seward et al.
==5684== Using LibVEX rev 1854, a library for dynamic binary translation.
==5684== Copyright (C) 2004-2007, and GNU GPL'd, by OpenWorks LLP.
==5684== Using valgrind-3.3.1, a dynamic binary instrumentation framework.
==5684== Copyright (C) 2000-2007, and GNU GPL'd, by Julian Seward et al.
==5684== For more details, rerun with: -v
==5684==
==5684== Invalid read of size 4
==5684==    at 0x8048FFF: main (main.c:112)
==5684==  Address 0x672d78a1 is not stack'd, malloc'd or (recently) free'd
==5684==
==5684== Process terminating with default action of signal 11 (SIGSEGV)
==5684==  Access not within mapped region at address 0x672D78A1
==5684==    at 0x8048FFF: main (main.c:112)
==5684==
==5684== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 3 from 1)
==5684== malloc/free: in use at exit: 824 bytes in 28 blocks.
==5684== malloc/free: 32 allocs, 4 frees, 1,683 bytes allocated.
==5684== For counts of detected errors, rerun with: -v
==5684== searching for pointers to 28 not-freed blocks.
==5684== checked 76,300 bytes.
==5684==
==5684== LEAK SUMMARY:
==5684==    definitely lost: 35 bytes in 1 blocks.
==5684==      possibly lost: 0 bytes in 0 blocks.
==5684==    still reachable: 789 bytes in 27 blocks.
==5684==         suppressed: 0 bytes in 0 blocks.
==5684== Rerun with --leak-check=full to see details of leaked memory.

Change History (4)

comment:1 Changed 10 years ago by huhabla

Resolution: fixed
Status: newclosed

The error is related to the I_location_info(title, argv[0]) call before line 112 and the size of the title buffer. The consequence is a buffer overflow resulting in a segfault.

The I_location_info() function is implemented quite unsafe using sprintf without checking the buffer length. It is always a bad idea to fill a buffer with an unknown length. I have fixed this in svn [41880] and removed the unused I_location_info() call from i.group.

It seems to me that I_location_info() is not used by any imagery module? Could it be removed?

comment:2 Changed 10 years ago by neteler

Is this backport relevant?

Markus

comment:3 in reply to:  2 Changed 10 years ago by huhabla

Replying to neteler:

Is this backport relevant?

This error appeared while testing grass as WPS backend for Windows using Python's subprocess.Popen() in GrassModuleStarter?.py. It seems that Windows expects always full path names to binaries??? Therefor this error may appear in this use-case on windows with older grass versions too.

So "Yes" for backport. At least the changes in i.group.

Soeren

Markus

comment:4 Changed 10 years ago by hamish

Keywords: i.group added; group removed

i.group part backported to 6.x in r41882,3. (took forever, the old server is not having a good day)

non-specific reminders:

Please do *not* backport API changes to the 6.x branches. (even for seemingly unused library functions, because 3rd-party addon modules might be using it)

Also please do not assume that because some function's return is unused in a trunk module that it is also unused in the 6.x branch code, especially for the heavily reduced imagery modules. The audit must start anew. (In this case it's fine)

thanks, Hamish

ps- see also #70 which covers the lack of @mapset support in the imagery library.

Note: See TracTickets for help on using tickets.