trifectatechfoundation / zlib-rs

A safer zlib
zlib License
139 stars 15 forks source link

OOB panic in Crc32HashCalc::insert_string() #219

Closed inahga closed 1 month ago

inahga commented 1 month ago

Fuzzing finds a possible bug in Crc32HashCalc::insert_string() when we invoke deflateParams() to change the compression level midway through a streaming session.

    #[test]
    fn inahga_3() {
        let mut source = "`0q\0\u{19}\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0G\0\0\0\0\0\0\u{7}\0\0\0@\0\0\0&\0\0\0\0\0\0\0\0\0\0\0VVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVV\0\0\0\0y\u{1f}\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0y\u{1f}\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\u{8}\0\0\0\0\0\u{1}\0\0\0y\u{1f}\0\0\0\0\0\0\0\0\0\0\0\0@\0\0y\u{1f}\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0y\u{1f}\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0".to_string();

        let config = DeflateConfig {
            level: 0,
            method: Method::Deflated,
            window_bits: 31,
            mem_level: 3,
            strategy: Strategy::Default,
        };

        let mut stream = z_stream::default();
        let err = unsafe {
            deflateInit2_(
                &mut stream,
                config.level,
                config.method as i32,
                config.window_bits,
                config.mem_level,
                config.strategy as i32,
                zlibVersion(),
                size_of::<z_stream>() as c_int,
            )
        };
        assert_eq!(ReturnCode::from(err), ReturnCode::Ok);

        let buf_size = unsafe { deflateBound(&mut stream, source.len() as u64) };

        let mut dest = vec![0; buf_size as usize];
        let chunk = 2u32;
        let flush = DeflateFlush::NoFlush;

        stream.next_in = source.as_mut_ptr().cast();
        stream.avail_in = chunk; // First chunk.
        stream.next_out = dest.as_mut_ptr().cast();
        stream.avail_out = dest.len().try_into().unwrap();

        // Deflate first chunk.
        let err = unsafe { deflate(&mut stream, flush as i32) };
        assert_eq!(ReturnCode::from(err), ReturnCode::Ok);

        // Change the parameters.
        let new_level = 4;
        let err = unsafe { deflateParams(&mut stream, new_level, config.strategy as _) };
        match ReturnCode::from(err) {
            ReturnCode::Ok => {}
            ReturnCode::BufError => {
                // Flushing the current pending data may run us out of buffer space.
                // Worst case double the buffer size.
                let add_space = Ord::min(chunk, buf_size as u32);
                dest.extend(core::iter::repeat_n(0, add_space.try_into().unwrap()));

                // If extend() reallocates, it may have moved in memory.
                stream.next_out = dest.as_mut_ptr();
                stream.avail_out += add_space;

                let err = unsafe { deflateParams(&mut stream, new_level, config.strategy as _) };
                assert_eq!(ReturnCode::from(err), ReturnCode::Ok);
            }
            err => panic!("fatal {:?}", err),
        }

        // Deflate the rest in chunks.
        let mut left: u32 = source.len() as u32 - chunk;
        while left > 0 {
            // Write the chunk.
            let avail = Ord::min(chunk, left).try_into().unwrap();
            stream.avail_in = avail;
            let err = unsafe { deflate(&mut stream, flush as i32) };
            match ReturnCode::from(err) {
                ReturnCode::Ok => {
                    left -= avail;
                }
                ReturnCode::BufError => {
                    // Worst case double the buffer size.
                    let add_space = Ord::min(chunk, buf_size as u32);
                    dest.extend(core::iter::repeat_n(0, add_space.try_into().unwrap()));

                    // If extend() reallocates, it may have moved in memory.
                    stream.next_out = dest.as_mut_ptr();
                    stream.avail_out += add_space;

                    left -= avail - stream.avail_in;
                }
                err => panic!("fatal {:?}", err),
            }
        }

        assert_eq!(left, 0);
    }
---- tests::inahga_3 stdout ----
thread 'tests::inahga_3' panicked at /home/inahga/Projects/zlib-rs/zlib-rs/src/deflate/hash_calc.rs:209:28:
range end index 5 out of range for slice of length 4
stack backtrace:
   0: rust_begin_unwind
             at /rustc/7608018cbdac9e55d0d13529cf43adc33d53efcf/library/std/src/panicking.rs:665:5
   1: core::panicking::panic_fmt
             at /rustc/7608018cbdac9e55d0d13529cf43adc33d53efcf/library/core/src/panicking.rs:74:14
   2: core::slice::index::slice_end_index_len_fail_rt
             at /rustc/7608018cbdac9e55d0d13529cf43adc33d53efcf/library/core/src/slice/index.rs:64:5
   3: core::slice::index::slice_end_index_len_fail
             at /rustc/7608018cbdac9e55d0d13529cf43adc33d53efcf/library/core/src/slice/index.rs:58:5
   4: <core::ops::range::Range<usize> as core::slice::index::SliceIndex<[T]>>::index
             at /home/inahga/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/slice/index.rs:465:13
   5: <core::ops::range::RangeTo<usize> as core::slice::index::SliceIndex<[T]>>::index
             at /home/inahga/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/slice/index.rs:550:9
   6: core::slice::index::<impl core::ops::index::Index<I> for [T]>::index
             at /home/inahga/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/slice/index.rs:16:9
   7: zlib_rs::deflate::hash_calc::Crc32HashCalc::insert_string
             at /home/inahga/Projects/zlib-rs/zlib-rs/src/deflate/hash_calc.rs:209:28
   8: zlib_rs::deflate::State::insert_string
             at /home/inahga/Projects/zlib-rs/zlib-rs/src/deflate.rs:1357:48
   9: zlib_rs::deflate::fill_window
             at /home/inahga/Projects/zlib-rs/zlib-rs/src/deflate.rs:1720:17
  10: zlib_rs::deflate::algorithm::medium::deflate_medium
             at /home/inahga/Projects/zlib-rs/zlib-rs/src/deflate/algorithm/medium.rs:38:13
  11: zlib_rs::deflate::algorithm::run
             at /home/inahga/Projects/zlib-rs/zlib-rs/src/deflate/algorithm/mod.rs:36:13
  12: zlib_rs::deflate::deflate
             at /home/inahga/Projects/zlib-rs/zlib-rs/src/deflate.rs:2578:22
  13: LIBZ_RS_SYS_TEST_deflate
             at /home/inahga/Projects/zlib-rs/libz-rs-sys/src/lib.rs:894:26
  14: compress_gz::tests::inahga_3
             at ./fuzz_targets/compress_gz.rs:667:32
  15: compress_gz::tests::inahga_3::{{closure}}
             at ./fuzz_targets/compress_gz.rs:599:18
  16: core::ops::function::FnOnce::call_once
             at /home/inahga/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/function.rs:250:5
  17: core::ops::function::FnOnce::call_once
             at /rustc/7608018cbdac9e55d0d13529cf43adc33d53efcf/library/core/src/ops/function.rs:250:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

failures:
    tests::inahga_3

This appears sensitive to the chunk value being too small. Anything larger than 2 and this particular test passes, but I've seen a range of values trigger this bug in fuzzing.

Stock zlib passes this test.

No memory corruption or UB happens here, so this is not security sensitive. The inputs involved are uncommon (does anyone actually use deflateParams()?). So I think this is a minor bug.