memorysafety / zlib-rs

A safer zlib
zlib License
75 stars 6 forks source link

fix bug in match copying #110

Closed folkertdev closed 3 weeks ago

folkertdev commented 4 weeks ago

fix #109

The true cause is that in many calculations, the padding that is added to the window (so that SIMD operations don't need to perform bounds checks) was included in calculations where it should not be. This caused wrapping behavior in the window to be incorrect, and things go sideways from there.

Correcting this bug and subsequent fuzzing found test cases that hit all the branches in the match copying logic, which is now fully implemented.

thanks to @VorpalBlade for the original bug report and easy reproduction of the problem!

folkertdev commented 4 weeks ago

I used this fuzzer to find input that triggers the op < len case in the else branch of inffast

Click me ```rust #![no_main] use libfuzzer_sys::fuzz_target; use crate::arbitrary::{Arbitrary, Unstructured}; use core::ffi::c_int; use core::mem::MaybeUninit; use libfuzzer_sys::arbitrary; use zlib_rs::{deflate::DeflateConfig, InflateFlush, ReturnCode}; #[derive(Debug)] struct Input { level: i32, // valid range 1..=9 window_bits: i32, // valid range is -15..=-1 mem_level: i32, // valid range = 8..=9 data: Vec, buf_size: usize, // valid range is 1..=(8 * 1024) } impl Arbitrary<'_> for Input { fn arbitrary(u: &mut Unstructured<'_>) -> arbitrary::Result { let level = u.int_in_range(1..=9)?; let window_bits = u.int_in_range(-15..=-9)?; let mem_level = u.int_in_range(8..=9)?; let buf_size = u.int_in_range(1..=8 * 1024)?; let data = Vec::::arbitrary(u)?; Ok(Input { level, window_bits, mem_level, data, buf_size, }) } } fuzz_target!(|data: Input| { use libz_rs_sys::*; let Input { level, window_bits, mem_level, data, buf_size, } = data; if data.is_empty() { return; } let config = DeflateConfig { level, window_bits, mem_level, ..Default::default() }; let mut input = vec![0; 1 << 17]; let (input, err) = zlib_rs::deflate::compress_slice(&mut input, &data, config); assert_eq!(err, ReturnCode::Ok); // dbg!(level, window_bits, mem_level, buf_size,); // std::fs::write( // "/home/folkertdev/rust/zlib-rs/libz-rs-sys/src/tests/test-data/op-len-edge-case.zraw", // &input, // ) // .unwrap(); let mut buf = vec![0; buf_size]; { let mut stream = MaybeUninit::::zeroed(); let err = unsafe { inflateInit2_( stream.as_mut_ptr(), window_bits, zlibVersion(), core::mem::size_of::() as c_int, ) }; assert_eq!(ReturnCode::from(err), ReturnCode::Ok); let stream = unsafe { stream.assume_init_mut() }; stream.next_in = input.as_ptr() as *mut u8; stream.avail_in = input.len() as _; while stream.avail_in != 0 { stream.next_out = buf.as_mut_ptr(); stream.avail_out = buf.len() as _; let err = unsafe { inflate(stream, InflateFlush::NoFlush as _) }; if ReturnCode::from(err) == ReturnCode::BufError { stream.avail_out = buf.len() as _; continue; } } unsafe { inflateEnd(stream) }; } }); ```