zstd decompression regressions (fuzz test results)

[squeek502]

Zig Version

master

Steps to Reproduce and Observed Behavior

These results are just testing for crashes/hangs, not correctness (yet).

zstd-fuzz-20250812-1.zip

Easiest way to reproduce the errors:

• Clone https://github.com/squeek502/zig-std-lib-fuzzing
• zig build fuzz-zstandard-debug (no AFL++ required, this just builds the pure Zig version for debugging)
• ./zig-out/bin/fuzz-zstandard-debug < 'path-to-unzipped-repros/crashes/id:000000,sig:06,src:000000,time:8842,execs:3848,op:ext_UO,pos:0'
  • etc, for the rest of the repros

All the crashes have this stack trace:

thread 753422 panic: access of union field 'in_frame' while field 'skipping_frame' is active
/home/ryan/Programming/zig/zig/lib/std/compress/zstd/Decompress.zig:167:53: 0x1159222 in stream (std.zig)
            return readInFrame(d, w, limit, &d.state.in_frame) catch |err| switch (err) {
                                                    ^
/home/ryan/Programming/zig/zig/lib/std/Io/Reader.zig:178:34: 0x107edbe in stream (std.zig)
    const n = try r.vtable.stream(r, w, limit);
                                 ^
/home/ryan/Programming/zig/zig/lib/std/Io/Reader.zig:257:27: 0x114fb75 in streamRemaining (std.zig)
        offset += r.stream(w, .unlimited) catch |err| switch (err) {
                          ^
/home/ryan/Programming/zig/zig-std-lib-fuzzing/fuzzers/zstandard.zig:27:43: 0x114c95d in zigMain (zstandard.zig)
    _ = zstd_stream.reader.streamRemaining(&out.writer) catch {
                                          ^
/home/ryan/Programming/zig/zig-std-lib-fuzzing/fuzzers/zstandard.zig:4:12: 0x114ce61 in main (zstandard.zig)
    zigMain() catch unreachable;
           ^
/home/ryan/Programming/zig/zig/lib/std/start.zig:618:22: 0x114ac1d in posixCallMainAndExit (std.zig)
            root.main();
                     ^
/home/ryan/Programming/zig/zig/lib/std/start.zig:232:5: 0x114a4b1 in _start (std.zig)
    asm volatile (switch (native_arch) {
    ^
???:?:?: 0x0 in ??? (???)

The hangs seem to be the same as the hangs found with flate in #24741, where there's an infinite loop here:

zig/lib/std/Io/Writer.zig

Lines 381 to 383 in 0d0f09f

	while (w.buffer.len - w.end < minimum_length) {
	try drainPreserve(w, preserve_len);
	} else {

but it's being called from here:

zig/lib/std/compress/zstd/Decompress.zig

Line 260 in 59de7e3

const dest = (try w.writableSliceGreedyPreserve(window_len, frame_block_size_max))[0..frame_block_size_max];

Expected Behavior

No crashes/hangs

[squeek502]

After #24853 the hangs are gone.

I'm now fuzzing for correctness against the zstd reference implementation. To rule out false positives (where the Zig implementation is intentionally more strict than the zstd implementation), I've also compiled a version of the test code with 0.14.1 to ensure that all found discrepancies also existed then. Luckily, almost all of the findings fall into that category.

The only real finding so far are cases where the zstd reference implementation errors with "Data corruption detected" but the Zig implementation succeeds:

zstd-DataCorruptionDetected-zig-NoError.zip

In the zip file, I've also included the output of running each input through the test code, which includes logging from the zstd reference implementation.

Example log

-----------------------------
./outputs/zstandard-compare-stream/default/crashes/zstd-DataCorruptionDetected-zig-NoError//id:000027,sig:06,src:000085,time:52035683,execs:3197212,op:havoc,rep:2
-----------------------------
/home/ryan/Programming/zig/zig-std-lib-fuzzing/lib/zstd/lib/decompress/zstd_decompress.c: ZSTD_createDCtx 
/home/ryan/Programming/zig/zig-std-lib-fuzzing/lib/zstd/lib/decompress/zstd_decompress.c: ZSTD_decompressStream 
/home/ryan/Programming/zig/zig-std-lib-fuzzing/lib/zstd/lib/decompress/zstd_decompress.c: input size : 318 
/home/ryan/Programming/zig/zig-std-lib-fuzzing/lib/zstd/lib/decompress/zstd_decompress.c: stage zdss_init => transparent reset  
/home/ryan/Programming/zig/zig-std-lib-fuzzing/lib/zstd/lib/decompress/zstd_decompress.c: stage zdss_loadHeader (srcSize : 318) 
/home/ryan/Programming/zig/zig-std-lib-fuzzing/lib/zstd/lib/decompress/zstd_decompress.c: ZSTD_getFrameHeader_advanced: minInputSize = 5, srcSize = 0 
/home/ryan/Programming/zig/zig-std-lib-fuzzing/lib/zstd/lib/decompress/zstd_decompress.c: stage zdss_loadHeader (srcSize : 313) 
/home/ryan/Programming/zig/zig-std-lib-fuzzing/lib/zstd/lib/decompress/zstd_decompress.c: ZSTD_getFrameHeader_advanced: minInputSize = 5, srcSize = 5 
/home/ryan/Programming/zig/zig-std-lib-fuzzing/lib/zstd/lib/decompress/zstd_decompress.c: stage zdss_loadHeader (srcSize : 310) 
/home/ryan/Programming/zig/zig-std-lib-fuzzing/lib/zstd/lib/decompress/zstd_decompress.c: ZSTD_getFrameHeader_advanced: minInputSize = 5, srcSize = 8 
/home/ryan/Programming/zig/zig-std-lib-fuzzing/lib/zstd/lib/decompress/zstd_decompress.c: ZSTD_getFrameHeader_advanced: minInputSize = 5, srcSize = 318 
/home/ryan/Programming/zig/zig-std-lib-fuzzing/lib/zstd/lib/decompress/zstd_decompress.c: ZSTD_decompressMultiFrame 
/home/ryan/Programming/zig/zig-std-lib-fuzzing/lib/zstd/lib/decompress/zstd_decompress.c: reading magic number FD2FB528 (expecting FD2FB528) 
/home/ryan/Programming/zig/zig-std-lib-fuzzing/lib/zstd/lib/decompress/zstd_decompress.c: ZSTD_decompressFrame (srcSize:318) 
/home/ryan/Programming/zig/zig-std-lib-fuzzing/lib/zstd/lib/decompress/zstd_decompress.c: ZSTD_getFrameHeader_advanced: minInputSize = 5, srcSize = 8 
/home/ryan/Programming/zig/zig-std-lib-fuzzing/lib/zstd/lib/decompress/zstd_decompress_block.c: ZSTD_decompressBlock_internal (size : 303) 
/home/ryan/Programming/zig/zig-std-lib-fuzzing/lib/zstd/lib/decompress/zstd_decompress_block.c: ZSTD_decodeLiteralsBlock 
/home/ryan/Programming/zig/zig-std-lib-fuzzing/lib/zstd/lib/decompress/zstd_decompress_block.c:223: ERROR!: check ERR_isError(hufSuccess) failed, returning ERROR(corruption_detected): 
/home/ryan/Programming/zig/zig-std-lib-fuzzing/lib/zstd/lib/decompress/zstd_decompress_block.c: ZSTD_decodeLiteralsBlock : cSize=4294967276, nbLiterals=0 
ZSTD ERROR: Data corruption detected
zstd error: error.DecompressError, zig error: error.NoError
thread 753961 panic: attempt to unwrap error: MismatchedErrors
/home/ryan/Programming/zig/zig-std-lib-fuzzing/fuzzers/zstandard-compare-stream.zig:99:13: 0x115b50f in zigMain (zstandard-compare-stream.zig)
            return error.MismatchedErrors;
            ^
/home/ryan/Programming/zig/zig-std-lib-fuzzing/fuzzers/zstandard-compare-stream.zig:6:21: 0x115c172 in main (zstandard-compare-stream.zig)
    zigMain() catch unreachable;
                    ^
../sysdeps/nptl/libc_start_call_main.h:58:16: 0x7ffff7c29d8f in __libc_start_call_main (../sysdeps/x86/libc-start.c)
../csu/libc-start.c:392:3: 0x7ffff7c29e3f in __libc_start_main_impl (../sysdeps/x86/libc-start.c)
???:?:?: 0x11fb964 in ??? (???)
???:?:?: 0x0 in ??? (???)

To reproduce with a test case in zstd.zig:

test "data corruption" {
    const input_raw = @embedFile("path-to-unzipped-repros/id:000027,sig:06,src:000085,time:52035683,execs:3197212,op:havoc,rep:2")

    // Not sure which error should actually be expected, this is just a random error for example purposes
    try testExpectDecompressError(error.MalformedFrame, input_raw);
}

To reproduce while comparing against zstd:

• Clone https://github.com/squeek502/zig-std-lib-fuzzing (and initialize submodules, haven't yet moved to using the package manager for the zstd sources)
• zig build fuzz-zstandard-compare-stream-debug (don't need AFL++ for this)
• ./zig-out/bin/fuzz-zstandard-compare-stream-debug < 'path-to-unzipped-repros/id:000027,sig:06,src:000085,time:52035683,execs:3197212,op:havoc,rep:2'

[andrewrk]

I made checksum verification off by default, and I also haven't implemented it yet so if you turn it on it panics. So, I expect not to be able to detect data corruption until that feature is reimplemented.

Note that, for fetch purposes, checksum verification disabled is desirable, because it's only a waste of time given the package hash. Many real world scenarios follow the same pattern - towers of checksums upon checksums, but only the top one is technically needed. The other ones are useful for debugging/troubleshooting but not required for correctness.

[squeek502]

Yeah, just realized that the old version errors with ChecksumFailure for all these inputs, so that makes sense.

[squeek502]

Ok, this one is actually real:

zstd-index-out-of-bounds.zip

zstd reference implementation errors with Destination buffer is too small here. From what I can tell, in the old implementation this scenario was handled here:

zig/lib/std/compress/zstandard/decode/block.zig

Line 351 in d03a147

if (sequence_length > dest[write_pos..].len) return error.DestTooSmall;

The new implementation hits index out-of-bounds:

thread 1179805 panic: index out of bounds: index 4240, len 128
/home/ryan/Programming/zig/zig/lib/std/compress/zstd/Decompress.zig:779:56: 0x1150273 in decodeSequence (fuzz-zstandard-compare-stream-debug)
                    dest[write_pos + literal_length ..][0..match_length],
                                                       ^
/home/ryan/Programming/zig/zig/lib/std/compress/zstd/Decompress.zig:322:63: 0x1151c45 in readInFrame (fuzz-zstandard-compare-stream-debug)
                    bytes_written += try decode.decodeSequence(w.buffer, write_pos + bytes_written, &bit_stream);
                                                              ^
/home/ryan/Programming/zig/zig/lib/std/compress/zstd/Decompress.zig:233:31: 0x114cf97 in stream (fuzz-zstandard-compare-stream-debug)
            return readInFrame(d, w, limit, in_frame) catch |err| switch (err) {
                              ^
/home/ryan/Programming/zig/zig/lib/std/compress/zstd/Decompress.zig:111:18: 0x114ce1d in streamDirect (fuzz-zstandard-compare-stream-debug)
    return stream(d, w, limit);
                 ^
/home/ryan/Programming/zig/zig/lib/std/Io/Reader.zig:178:34: 0x10a25d5 in stream (fuzz-zstandard-compare-stream-debug)
    const n = try r.vtable.stream(r, w, limit);
                                 ^
/home/ryan/Programming/zig/zig/lib/std/Io/Reader.zig:257:27: 0x10cc3ff in streamRemaining (fuzz-zstandard-compare-stream-debug)
        offset += r.stream(w, .unlimited) catch |err| switch (err) {
                          ^
/home/ryan/Programming/zig/zig-std-lib-fuzzing/fuzzers/zstandard-compare-stream.zig:56:43: 0x1119887 in zigZstdStreaming (fuzz-zstandard-compare-stream-debug)
    _ = zstd_stream.reader.streamRemaining(&out.writer) catch |err| {
                                          ^
/home/ryan/Programming/zig/zig-std-lib-fuzzing/fuzzers/zstandard-compare-stream.zig:86:49: 0x111d1ce in zigMain (fuzz-zstandard-compare-stream-debug)
    const actual_bytes: ?[]u8 = zigZstdStreaming(allocator, data) catch |err| blk: {
                                                ^
/home/ryan/Programming/zig/zig-std-lib-fuzzing/fuzzers/zstandard-compare-stream.zig:6:12: 0x111ec17 in main (fuzz-zstandard-compare-stream-debug)
    zigMain() catch unreachable;
           ^
../sysdeps/nptl/libc_start_call_main.h:58:16: 0x7ffff7c29d8f in __libc_start_call_main (../sysdeps/x86/libc-start.c)
../csu/libc-start.c:392:3: 0x7ffff7c29e3f in __libc_start_main_impl (../sysdeps/x86/libc-start.c)
???:?:?: 0x104a6a4 in ??? (???)
???:?:?: 0x0 in ??? (???)
Aborted

Reproduction steps in #24817 (comment) still apply.