Opened 3 years ago
Closed 3 years 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 , 3 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
comment:4 by , 3 years ago
Resolution: | fixed |
---|---|
Status: | closed → reopened |
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 , 3 years ago
Odd that we were clean, there's a bunch of sprintf() calls that also don't do anything with the return.
comment:6 by , 3 years ago
Or maybe capture then ignore them. Or maybe a new error will arise about an unused variable…
comment:7 by , 3 years ago
Resolution: | → fixed |
---|---|
Status: | reopened → closed |
More broad clean-up in master, ugly cleanup in stables now.
In 111d080/git: