Opened 5 months ago

Closed 3 months ago

#5114 closed defect (fixed)

pgsql2shp segfault with long or many truncated columns

Reported by: dfuhry2 Owned by: pramsey
Priority: medium Milestone: PostGIS 3.2.2
Component: postgis Version: 2.3.x
Keywords: Cc:

Description

I experienced this in a real-world query, but a short reproducible test case of pgsql2shp segfault is below. I think the problem may be state→message overflowing with messages about the truncated columns. As a workaround, raising SHPLOADERMSGLEN from 1024 to 8192 caused pgsql2shp to complete successfully. But I assume some bounds checking is needed to prevent the overflow from occurring in the first place. This is against 2.5.3. Sorry to report against an old version, but I did not find a bug report for this against any newer version either.

gdb --args ./pgsql2shp -p 5434 -f test.shp dfuhry 'SELECT 1 AS abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijk, 2 AS bcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijkl, 3 AS cdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklm, 4 AS defghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmn, 5 as efghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmno, 6 AS fghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnop, 7 AS ghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopq, 8 AS hijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqr, 9 AS ijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrs, 10 AS jklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrst'
...
(gdb) r
...
Initializing... 
*** buffer overflow detected ***: terminated

Program received signal SIGABRT, Aborted.
__GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
50	../sysdeps/unix/sysv/linux/raise.c: No such file or directory.
(gdb) bt
#0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
#1  0x00007ffff7d74537 in __GI_abort () at abort.c:79
#2  0x00007ffff7dcd768 in __libc_message (action=action@entry=do_abort, fmt=fmt@entry=0x7ffff7edbc24 "*** %s ***: terminated\n") at ../sysdeps/posix/libc_fatal.c:155
#3  0x00007ffff7e5c652 in __GI___fortify_fail (msg=msg@entry=0x7ffff7edbbba "buffer overflow detected") at fortify_fail.c:26
#4  0x00007ffff7e5b050 in __GI___chk_fail () at chk_fail.c:28
#5  0x00007ffff7e5a999 in __strncat_chk (s1=<optimized out>, 
    s1@entry=0x55555559b068 "Warning, field abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijk renamed to ABCDEFGHIJ\nWarning, field bcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijkl renamed to BCDEFGHIJ"..., s2=<optimized out>, 
    s2@entry=0x7fffffffdd00 "No geometry column found.\nThe DBF file will be created but not the shx or shp files.\n", n=<optimized out>, s1len=<optimized out>, s1len@entry=1024) at strncat_chk.c:33
#6  0x0000555555562c11 in strncat (__len=<optimized out>, __src=0x7fffffffdd00 "No geometry column found.\nThe DBF file will be created but not the shx or shp files.\n", 
    __dest=0x55555559b068 "Warning, field abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijk renamed to ABCDEFGHIJ\nWarning, field bcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijkl renamed to BCDEFGHIJ"...)
    at /usr/include/x86_64-linux-gnu/bits/string_fortified.h:136
#7  ShpDumperOpenTable (state=state@entry=0x55555559afb0) at pgsql2shp-core.c:1837
#8  0x000055555555774f in main (argc=<optimized out>, argv=<optimized out>) at pgsql2shp-cli.c:191

Change History (7)

comment:1 by Paul Ramsey <pramsey@…>, 3 months ago

Resolution: fixed
Status: newclosed

In 111d080/git:

Fix wraparound math problem in strncat calls, closes #5114

comment:2 by Paul Ramsey <pramsey@…>, 3 months ago

In 568790a/git:

Fix wraparound math problem in strncat calls, closes #5114

comment:3 by Paul Ramsey <pramsey@…>, 3 months ago

In 800a7f2/git:

Fix wraparound math problem in strncat calls, closes #5114

comment:4 by robe, 3 months ago

Resolution: fixed
Status: closedreopened

Reopening cause dronie is complaining about it. I know it's just a warning treated as an error, but we were clean before.

https://dronie.osgeo.org/postgis/postgis/3117/1/2

pgsql2shp-core.c:1954:2: error: ignoring return value of 'asprintf', declared with attribute warn_unused_result [-Werror=unused-result]

  asprintf(&(state->fetch_query), "FETCH %d FROM cur", state->config->fetchsize);

  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

cc1: all warnings being treated as errors

Makefile:97: recipe for target 'pgsql2shp-core.o' failed

make[1]: *** [pgsql2shp-core.o] Error 1

make[1]: *** Waiting for unfinished jobs....

make[1]: Leaving directory '/drone/src/loader'

GNUmakefile:22: recipe for target 'all' failed

make: *** [all] Error 1

comment:5 by pramsey, 3 months ago

Odd that we were clean, there's a bunch of sprintf() calls that also don't do anything with the return.

ef4a050a9/git:

Last edited 3 months ago by pramsey (previous) (diff)

comment:6 by pramsey, 3 months ago

Or maybe capture then ignore them. Or maybe a new error will arise about an unused variable…

b1e00a36a/git:

comment:7 by pramsey, 3 months ago

Resolution: fixed
Status: reopenedclosed

More broad clean-up in master, ugly cleanup in stables now.

Note: See TracTickets for help on using tickets.