Opened 2 months ago

Closed 2 months 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 2 months ago.

Download all attachments as: .zip

Change History (10)

comment:1 Changed 2 months ago by Algunenano

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 Changed 2 months ago by Algunenano

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 Changed 2 months ago by Algunenano

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

comment:4 Changed 2 months ago by Algunenano

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

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

comment:5 Changed 2 months ago by Raúl Marín <git@…>

In 07fa25e4/git:

Prevent stack overflow when parsing WKB

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

comment:6 Changed 2 months ago by Raúl Marín <git@…>

In 5481434/git:

Prevent stack overflow when parsing WKB

References #4621

comment:7 Changed 2 months ago by Raúl Marín <git@…>

In affe90f/git:

Prevent stack overflow when parsing WKB

References #4621

comment:8 Changed 2 months ago by Raúl Marín <git@…>

In 2ad0ddc/git:

Prevent stack overflow when parsing WKB

References #4621

comment:9 Changed 2 months ago by Raúl Marín <git@…>

Resolution: fixed
Status: newclosed

In 8475f49/git:

Prevent stack overflow when parsing WKB

Closes #4621

Note: See TracTickets for help on using tickets.