Opened 4 years ago

Closed 4 years ago

#4621 closed defect (fixed)

oss-fuzz: stack overflow in lwcollection_from_wkb_state

Reported by: komzpa Owned by: pramsey
Priority: medium Milestone: PostGIS 3.1.0
Component: postgis Version: master
Keywords: Cc:

Description

https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=20159

4 lines omitted
SCARINESS: 10 (stack-overflow)
    #0 0x4b5641 in __sanitizer::StackDepotBase<__sanitizer::StackDepotNode, 1, 20>::Put(__sanitizer::StackTrace, bool*) /src/llvm-project/compiler-rt/lib/sanitizer_common/sanitizer_stackdepotbase.h:98
    #1 0x4b5616 in __sanitizer::StackDepotPut(__sanitizer::StackTrace) /src/llvm-project/compiler-rt/lib/sanitizer_common/sanitizer_stackdepot.cpp:98:33
    #2 0x41df1e in __asan::Allocator::Allocate(unsigned long, unsigned long, __sanitizer::BufferedStackTrace*, __asan::AllocType, bool) /src/llvm-project/compiler-rt/lib/asan/asan_allocator.cpp:526:27
    #3 0x41d933 in __asan::asan_malloc(unsigned long, __sanitizer::BufferedStackTrace*) /src/llvm-project/compiler-rt/lib/asan/asan_allocator.cpp:892:34
    #4 0x49509b in malloc /src/llvm-project/compiler-rt/lib/asan/asan_malloc_linux.cpp:146:10
    #5 0x503bb6 in lwcollection_construct_empty /src/postgis/liblwgeom/lwcollection.c:101:8
    #6 0x510090 in lwcollection_from_wkb_state /src/postgis/liblwgeom/lwin_wkb.c:674:22
    #7 0x51012c in lwcollection_from_wkb_state /src/postgis/liblwgeom/lwin_wkb.c:690:10
    #8 0x51012c in lwcollection_from_wkb_state /src/postgis/liblwgeom/lwin_wkb.c:690:10
    #9 0x51012c in lwcollection_from_wkb_state /src/postgis/liblwgeom/lwin_wkb.c:690:10
    #10 0x51012c in lwcollection_from_wkb_state /src/postgis/liblwgeom/lwin_wkb.c:690:10
    #11 0x51012c in lwcollection_from_wkb_state /src/postgis/liblwgeom/lwin_wkb.c:690:10
    #12 0x51012c in lwcollection_from_wkb_state /src/postgis/liblwgeom/lwin_wkb.c:690:10
    #13 0x51012c in lwcollection_from_wkb_state /src/postgis/liblwgeom/lwin_wkb.c:690:10
    #14 0x51012c in lwcollection_from_wkb_state /src/postgis/liblwgeom/lwin_wkb.c:690:10
    #15 0x51012c in lwcollection_from_wkb_state /src/postgis/liblwgeom/lwin_wkb.c:690:10
    #16 0x51012c in lwcollection_from_wkb_state /src/postgis/liblwgeom/lwin_wkb.c:690:10
    #17 0x51012c in lwcollection_from_wkb_state /src/postgis/liblwgeom/lwin_wkb.c:690:10
    #18 0x51012c in lwcollection_from_wkb_state /src/postgis/liblwgeom/lwin_wkb.c:690:10
    #19 0x51012c in lwcollection_from_wkb_state /src/postgis/liblwgeom/lwin_wkb.c:690:10
    #20 0x51012c in lwcollection_from_wkb_state /src/postgis/liblwgeom/lwin_wkb.c:690:10
    #21 0x51012c in lwcollection_from_wkb_state /src/postgis/liblwgeom/lwin_wkb.c:690:10
    #22 0x51012c in lwcollection_from_wkb_state /src/postgis/liblwgeom/lwin_wkb.c:690:10
    #23 0x51012c in lwcollection_from_wkb_state /src/postgis/liblwgeom/lwin_wkb.c:690:10
483 lines omitted

Attachments (1)

clusterfuzz-testcase-minimized-wkb_import_fuzzer-5737413385388032.fuzz (1.3 MB ) - added by komzpa 4 years ago.

Download all attachments as: .zip

Change History (10)

comment:1 by Algunenano, 4 years ago

I'm not sure how to introduce this test without adding the 1M file to the repo, but the issue is with the recursion in lwgeom_from_wkb_state when you have an extremely deep collection (a collection of a collection of a collection…).

In my PC this crashes once it reaches 261962 (!!!) calls:

#261943 0x00005616418b3ff0 in lwcollection_from_wkb_state (s=0x7ffffa6f6940) at lwin_wkb.c:690
#261944 lwgeom_from_wkb_state (s=0x7ffffa6f6940) at lwin_wkb.c:786
#261945 0x00005616418b3ff0 in lwcollection_from_wkb_state (s=0x7ffffa6f6940) at lwin_wkb.c:690
#261946 lwgeom_from_wkb_state (s=0x7ffffa6f6940) at lwin_wkb.c:786
#261947 0x00005616418b3ff0 in lwcollection_from_wkb_state (s=0x7ffffa6f6940) at lwin_wkb.c:690
#261948 lwgeom_from_wkb_state (s=0x7ffffa6f6940) at lwin_wkb.c:786
#261949 0x00005616418b3ff0 in lwcollection_from_wkb_state (s=0x7ffffa6f6940) at lwin_wkb.c:690
#261950 lwgeom_from_wkb_state (s=0x7ffffa6f6940) at lwin_wkb.c:786
#261951 0x00005616418b3ff0 in lwcollection_from_wkb_state (s=0x7ffffa6f6940) at lwin_wkb.c:690
#261952 lwgeom_from_wkb_state (s=0x7ffffa6f6940) at lwin_wkb.c:786
#261953 0x00005616418b3ff0 in lwcollection_from_wkb_state (s=0x7ffffa6f6940) at lwin_wkb.c:690
#261954 lwgeom_from_wkb_state (s=0x7ffffa6f6940) at lwin_wkb.c:786
#261955 0x00005616418b3ff0 in lwcollection_from_wkb_state (s=0x7ffffa6f6940) at lwin_wkb.c:690
#261956 lwgeom_from_wkb_state (s=0x7ffffa6f6940) at lwin_wkb.c:786
#261957 0x00005616418b463f in lwgeom_from_wkb (wkb=<optimized out>, wkb_size=139656151874024, check=0 '\000') at lwin_wkb.c:830
#261958 0x0000561641891a4b in test_wkb_fuzz () at cu_in_wkb.c:287
#261959 0x00007f043c44b118 in ?? () from /usr/lib/libcunit.so.1
#261960 0x00007f043c44b3b2 in ?? () from /usr/lib/libcunit.so.1
#261961 0x00007f043c44b7b7 in CU_run_all_tests () from /usr/lib/libcunit.so.1
#261962 0x0000561641896925 in main (argc=1, argv=<optimized out>) at cu_tester.c:183

comment:2 by Algunenano, 4 years ago

A simpler repro:

	/* OSS-FUZZ: https://trac.osgeo.org/postgis/ticket/4621 */
	uint32_t big_size = 20000000;
	uint8_t *wkb5 = lwalloc(big_size);
	memset(wkb5, 0x01, big_size);
	g = lwgeom_from_wkb(wkb5, big_size, LW_PARSER_CHECK_NONE);
	lwgeom_free(g);
	lwfree(wkb5);

I think we should put a limit to the recursive calls that depend on user input, but I'm not sure what that would be. I'd say that if you have 1024 chained collection you are just trying to break things so we could discard it.

comment:3 by Algunenano, 4 years ago

Bison (which handles WKT) uses YYINITDEPTH 200, so I'll set 200 for WKB too.

comment:4 by Algunenano, 4 years ago

PR in https://github.com/postgis/postgis/pull/536

I haven't checked, but I'd say all version are affected.

comment:5 by Raúl Marín <git@…>, 4 years ago

In 07fa25e4/git:

Prevent stack overflow when parsing WKB

References #4621
Closes https://github.com/postgis/postgis/pull/536

comment:6 by Raúl Marín <git@…>, 4 years ago

In 5481434/git:

Prevent stack overflow when parsing WKB

References #4621

comment:7 by Raúl Marín <git@…>, 4 years ago

In affe90f/git:

Prevent stack overflow when parsing WKB

References #4621

comment:8 by Raúl Marín <git@…>, 4 years ago

In 2ad0ddc/git:

Prevent stack overflow when parsing WKB

References #4621

comment:9 by Raúl Marín <git@…>, 4 years ago

Resolution: fixed
Status: newclosed

In 8475f49/git:

Prevent stack overflow when parsing WKB

Closes #4621

Note: See TracTickets for help on using tickets.