zstd icon indicating copy to clipboard operation
zstd copied to clipboard

Undefined behavior in `ZSTD_decompress` for certain `dest` values when `dstCapacity` is zero

Open squeek502 opened this issue 2 years ago • 1 comments

Describe the bug When dstCapacity is zero, in theory the dest pointer should be irrelevant--any pointer value should be allowed (or, alternatively, the docs should specify what is and isn't allowed). However, if ZSTD_decompress is called with dstCapacity=0 and dest having a value of e.g. 0xffffffffffffffff, it will hit UB and may trip an assertion. I'm unsure exactly what range of pointer values will trigger this, but the maximum pointer value definitely does.

To Reproduce

  1. Inputs that can trigger the UB: zstd-zero-dest-capacity.zip (found when fuzzing a Zig implementation of a zstd decompressor). I'm not sure the input matters too much, the uncompressed size just needs to be zero; some inputs are the same as those in #3506
  2. Compile zstd with clang via make lib CFLAGS="-fsanitize=undefined -fPIC"
  3. Modify examples/simple_decompression.c with the following patch:
-    void* const rBuff = malloc_orDie((size_t)rSize);
+    CHECK(rSize == 0, "Uncompressed size must be zero");
+    void* const rBuff = (void*)(-1); // 0xffffffffffffffff
  1. Compile examples with make LDFLAGS="-fsanitize=undefined"
  2. Run one of the inputs through simple_decompression, e.g. ./simple_decompression 'id:000000,sig:04,src:000193,time:1035023,execs:294994,op:havoc,rep:4'

Example of the output with DEBUGLEVEL=10:

$ ./simple_decompression 'id:000000,sig:04,src:000193,time:1035023,execs:294994,op:havoc,rep:4'
.//decompress/zstd_decompress.c: ZSTD_getFrameHeader_advanced: minInputSize = 5, srcSize = 32 
.//decompress/zstd_decompress.c: ZSTD_decompressMultiFrame 
.//decompress/zstd_decompress.c: reading magic number FD2FB528 (expecting FD2FB528) 
.//decompress/zstd_decompress.c: ZSTD_decompressFrame (srcSize:32) 
.//decompress/zstd_decompress.c: ZSTD_getFrameHeader_advanced: minInputSize = 5, srcSize = 9 
.//decompress/zstd_decompress_block.c: ZSTD_decompressBlock_internal (size : 20) 
.//decompress/zstd_decompress_block.c: ZSTD_decodeLiteralsBlock 
.//decompress/zstd_decompress_block.c: ZSTD_decodeLiteralsBlock : cSize=1, nbLiterals=0 
.//decompress/zstd_decompress_block.c: ZSTD_decodeSeqHeaders 
.//decompress/zstd_decompress_block.c: ZSTD_getLongOffsetsShare: (tableLog=5) 
.//decompress/zstd_decompress_block.c: ZSTD_decompressSequencesLong 
.//decompress/zstd_decompress_block.c: ZSTD_initFseState : val=0 using 6 bits 
.//decompress/zstd_decompress_block.c: ZSTD_initFseState : val=0 using 5 bits 
.//decompress/zstd_decompress_block.c: ZSTD_initFseState : val=0 using 6 bits 
.//decompress/zstd_decompress_block.c: seq: litL=0, matchL=3, offset=4 
/decompress/zstd_decompress_block.c:1696:45: runtime error: applying non-zero offset 18446744073709551615 to null pointer
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /decompress/zstd_decompress_block.c:1696:45 in 
/decompress/zstd_decompress_block.c:1698:29: runtime error: pointer index expression with base 0xfffffffffffffffb overflowed to 0x00000000003b
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /decompress/zstd_decompress_block.c:1698:29 in 
.//decompress/zstd_decompress_block.c: seq: litL=0, matchL=3, offset=1 
.//decompress/zstd_decompress_block.c: seq: litL=0, matchL=3, offset=4 
.//decompress/zstd_decompress_block.c: seq: litL=0, matchL=41, offset=1 
.//decompress/zstd_decompress_block.c: seq: litL=0, matchL=3, offset=4 
.//decompress/zstd_decompress_block.c: seq: litL=10, matchL=3, offset=571 
/decompress/zstd_decompress_block.c:1696:59: runtime error: pointer index expression with base 0x00000000003e overflowed to 0xfffffffffffffe03
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /decompress/zstd_decompress_block.c:1696:59 in 
.//decompress/zstd_decompress_block.c: seq: litL=2, matchL=3, offset=37 
.//decompress/zstd_decompress_block.c:1755: ERROR!: check seqNb<seqAdvance failed, returning ERROR(corruption_detected): 
simple_decompression.c:39 CHECK(!ZSTD_isError(err)) failed: Data corruption detected

And one that trips an assertion:

$ ./simple_decompression 'id:000032,sig:06,src:000673,time:3423388,execs:797038,op:havoc,rep:2' 
.//decompress/zstd_decompress.c: ZSTD_getFrameHeader_advanced: minInputSize = 5, srcSize = 39 
.//decompress/zstd_decompress.c: ZSTD_decompressMultiFrame 
.//decompress/zstd_decompress.c: reading magic number FD2FB528 (expecting FD2FB528) 
.//decompress/zstd_decompress.c: ZSTD_decompressFrame (srcSize:39) 
.//decompress/zstd_decompress.c: ZSTD_getFrameHeader_advanced: minInputSize = 5, srcSize = 9 
.//decompress/zstd_decompress_block.c: ZSTD_decompressBlock_internal (size : 20) 
.//decompress/zstd_decompress_block.c: ZSTD_decodeLiteralsBlock 
.//decompress/zstd_decompress_block.c: ZSTD_decodeLiteralsBlock : cSize=1, nbLiterals=0 
.//decompress/zstd_decompress_block.c: ZSTD_decodeSeqHeaders 
.//decompress/zstd_decompress_block.c: ZSTD_getLongOffsetsShare: (tableLog=5) 
.//decompress/zstd_decompress_block.c: ZSTD_decompressSequencesLong 
.//decompress/zstd_decompress_block.c: ZSTD_initFseState : val=5 using 5 bits 
.//decompress/zstd_decompress_block.c: ZSTD_initFseState : val=24 using 5 bits 
.//decompress/zstd_decompress_block.c: ZSTD_initFseState : val=0 using 0 bits 
.//decompress/zstd_decompress_block.c: seq: litL=0, matchL=3, offset=8 
/decompress/zstd_decompress_block.c:1696:45: runtime error: applying non-zero offset 18446744073709551615 to null pointer
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /decompress/zstd_decompress_block.c:1696:45 in 
/decompress/zstd_decompress_block.c:1698:29: runtime error: pointer index expression with base 0xfffffffffffffff7 overflowed to 0x000000000037
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /decompress/zstd_decompress_block.c:1698:29 in 
.//decompress/zstd_decompress_block.c: seq: litL=2, matchL=3, offset=1 
.//decompress/zstd_decompress_block.c: seq: litL=0, matchL=3, offset=4 
.//decompress/zstd_decompress_block.c: seq: litL=0, matchL=3, offset=8 
.//decompress/zstd_decompress_block.c: seq: litL=0, matchL=3, offset=2 
.//decompress/zstd_decompress_block.c: seq: litL=0, matchL=3, offset=4 
.//decompress/zstd_decompress_block.c: seq: litL=0, matchL=3, offset=8 
.//decompress/zstd_decompress_block.c: seq: litL=0, matchL=3, offset=2 
.//decompress/zstd_decompress_block.c: seq: litL=2, matchL=3, offset=8 
/decompress/zstd_decompress_block.c:971:32: runtime error: pointer index expression with base 0xffffffffffffffff overflowed to 0x000000000002
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /decompress/zstd_decompress_block.c:971:32 in 
simple_decompression: .//decompress/zstd_decompress_block.c:996: size_t ZSTD_execSequence(BYTE *, BYTE *const, seq_t, const BYTE **, const BYTE *const, const BYTE *const, const BYTE *const, const BYTE *const): Assertion `oLitEnd < oMatchEnd' failed.
Aborted (core dumped)

Expected behavior No UB

Desktop:

  • OS: Linux
  • Compiler clang

Additional context Might be the same root cause as #3506

This was found because the Zig allocator interface will return a zero-length slice [a pointer + length pair] where the pointer part has the value 0xffffffffffffffff if you request a zero-sized allocation (in lieu of doing a real allocation). See this comment in the fuzzer implementation

squeek502 avatar Feb 14 '23 04:02 squeek502

Note: I somehow attached the wrong reproduction zip file to this issue originally. Fixed now.

squeek502 avatar Feb 16 '23 00:02 squeek502