From 3cfa75cfc378547b4bdf864815a4250fda1f15c5 Mon Sep 17 00:00:00 2001 From: Folkert de Vries Date: Fri, 16 Aug 2024 11:43:50 +0200 Subject: [PATCH] handle partially uninitialized streams properly --- libz-rs-sys/src/lib.rs | 76 ++++++++++++++++++++++++++++++++++++- test-libz-rs-sys/src/lib.rs | 76 +++++++++++++++++++++++++++++++++++++ 2 files changed, 150 insertions(+), 2 deletions(-) diff --git a/libz-rs-sys/src/lib.rs b/libz-rs-sys/src/lib.rs index c37ae6b1..a28eaea3 100644 --- a/libz-rs-sys/src/lib.rs +++ b/libz-rs-sys/src/lib.rs @@ -393,6 +393,10 @@ pub unsafe extern "C" fn inflateEnd(strm: *mut z_stream) -> i32 { /// * Either /// - `version` is NULL /// - `version` satisfies the requirements of [`core::ptr::read::`] +/// * If `strm` is not `NULL`, the following fields must be initialized +/// - `zalloc` +/// - `zfree` +/// - `opaque` #[export_name = prefix!(inflateBackInit_)] pub unsafe extern "C" fn inflateBackInit_( _strm: z_streamp, @@ -585,6 +589,12 @@ pub unsafe extern "C" fn inflateSyncPoint(strm: *mut z_stream) -> i32 { /// * Either /// - `version` is NULL /// - `version` satisfies the requirements of [`core::ptr::read::`] +/// * If `strm` is not `NULL`, the following fields must be initialized +/// - `next_in` +/// - `avail_in` +/// - `zalloc` +/// - `zfree` +/// - `opaque` #[export_name = prefix!(inflateInit_)] pub unsafe extern "C" fn inflateInit_( strm: z_streamp, @@ -614,6 +624,12 @@ pub unsafe extern "C" fn inflateInit_( /// * Either /// - `version` is NULL /// - `version` satisfies the requirements of [`core::ptr::read::`] +/// * If `strm` is not `NULL`, the following fields must be initialized +/// - `next_in` +/// - `avail_in` +/// - `zalloc` +/// - `zfree` +/// - `opaque` #[export_name = prefix!(inflateInit2_)] pub unsafe extern "C" fn inflateInit2_( strm: z_streamp, @@ -628,6 +644,24 @@ pub unsafe extern "C" fn inflateInit2_( } } +/// Helper that implements the actual initialization logic +/// +/// # Safety +/// +/// The caller must guarantee that +/// +/// * Either +/// - `strm` is `NULL` +/// - `strm` satisfies the requirements of `&mut *(strm as *mut MaybeUninit)` +/// * Either +/// - `version` is NULL +/// - `version` satisfies the requirements of [`core::ptr::read::`] +/// * If `strm` is not `NULL`, the following fields must be initialized +/// - `next_in` +/// - `avail_in` +/// - `zalloc` +/// - `zfree` +/// - `opaque` unsafe extern "C" fn inflateInit2(strm: z_streamp, windowBits: c_int) -> c_int { if strm.is_null() { ReturnCode::StreamError as _ @@ -636,7 +670,16 @@ unsafe extern "C" fn inflateInit2(strm: z_streamp, windowBits: c_int) -> c_int { window_bits: windowBits, }; - let stream = &mut *strm; + let mut stream = z_stream::default(); + + // SAFETY: the caller guarantees these fields are initialized + unsafe { + stream.next_in = *core::ptr::addr_of!((*strm).next_in); + stream.avail_in = *core::ptr::addr_of!((*strm).avail_in); + stream.zalloc = *core::ptr::addr_of!((*strm).zalloc); + stream.zfree = *core::ptr::addr_of!((*strm).zfree); + stream.opaque = *core::ptr::addr_of!((*strm).opaque); + } if stream.zalloc.is_none() { stream.zalloc = DEFAULT_ZALLOC; @@ -647,6 +690,13 @@ unsafe extern "C" fn inflateInit2(strm: z_streamp, windowBits: c_int) -> c_int { stream.zfree = DEFAULT_ZFREE; } + // SAFETY: the caller guarantees this pointer is writable + unsafe { core::ptr::write(strm, stream) }; + + // SAFETY: we have now properly initialized this memory + // the caller guarantees the safety of `&mut *` + let stream = unsafe { &mut *strm }; + zlib_rs::inflate::init(stream, config) as _ } } @@ -1386,6 +1436,10 @@ pub unsafe extern "C" fn deflateCopy(dest: z_streamp, source: z_streamp) -> c_in /// * Either /// - `version` is NULL /// - `version` satisfies the requirements of [`core::ptr::read::`] +/// * If `strm` is not `NULL`, the following fields must be initialized +/// - `zalloc` +/// - `zfree` +/// - `opaque` /// /// # Example /// @@ -1468,6 +1522,10 @@ pub unsafe extern "C" fn deflateInit_( /// * Either /// - `version` is NULL /// - `version` satisfies the requirements of [`core::ptr::read::`] +/// * If `strm` is not `NULL`, the following fields must be initialized +/// - `zalloc` +/// - `zfree` +/// - `opaque` /// /// # Example /// @@ -1529,7 +1587,14 @@ pub unsafe extern "C" fn deflateInit2_( strategy, }; - let stream = &mut *strm; + let mut stream = z_stream::default(); + + // SAFETY: the caller guarantees these fields are initialized + unsafe { + stream.zalloc = *core::ptr::addr_of!((*strm).zalloc); + stream.zfree = *core::ptr::addr_of!((*strm).zfree); + stream.opaque = *core::ptr::addr_of!((*strm).opaque); + } if stream.zalloc.is_none() { stream.zalloc = DEFAULT_ZALLOC; @@ -1540,6 +1605,13 @@ pub unsafe extern "C" fn deflateInit2_( stream.zfree = DEFAULT_ZFREE; } + // SAFETY: the caller guarantees this pointer is writable + unsafe { core::ptr::write(strm, stream) }; + + // SAFETY: we have now properly initialized this memory + // the caller guarantees the safety of `&mut *` + let stream = unsafe { &mut *strm }; + zlib_rs::deflate::init(stream, config) as _ } } diff --git a/test-libz-rs-sys/src/lib.rs b/test-libz-rs-sys/src/lib.rs index f98fae7f..bbad16e8 100644 --- a/test-libz-rs-sys/src/lib.rs +++ b/test-libz-rs-sys/src/lib.rs @@ -713,6 +713,82 @@ mod null { assert_eq!(err, Z_OK); } } + + #[test] + fn inflate_init_uninitialized() { + use libz_rs_sys::*; + + unsafe { + let mut strm = MaybeUninit::::uninit(); + + core::ptr::write(core::ptr::addr_of_mut!((*strm.as_mut_ptr()).avail_in), 0); + core::ptr::write( + core::ptr::addr_of_mut!((*strm.as_mut_ptr()).next_in), + core::ptr::null(), + ); + + core::ptr::write( + core::ptr::addr_of_mut!((*strm.as_mut_ptr()).zalloc).cast(), + zlib_rs::allocate::Allocator::C.zalloc, + ); + + core::ptr::write( + core::ptr::addr_of_mut!((*strm.as_mut_ptr()).zfree).cast(), + zlib_rs::allocate::Allocator::C.zfree, + ); + + core::ptr::write( + core::ptr::addr_of_mut!((*strm.as_mut_ptr()).opaque), + core::ptr::null_mut(), + ); + + let err = inflateInit_( + strm.as_mut_ptr(), + zlibVersion(), + core::mem::size_of::() as _, + ); + assert_eq!(err, Z_OK); + + let err = inflateEnd(strm.as_mut_ptr()); + assert_eq!(err, Z_OK); + } + } + + #[test] + #[cfg_attr(miri, ignore = "slow")] + fn deflate_init_uninitialized() { + use libz_rs_sys::*; + + unsafe { + let mut strm = MaybeUninit::::uninit(); + + core::ptr::write( + core::ptr::addr_of_mut!((*strm.as_mut_ptr()).zalloc).cast(), + zlib_rs::allocate::Allocator::C.zalloc, + ); + + core::ptr::write( + core::ptr::addr_of_mut!((*strm.as_mut_ptr()).zfree).cast(), + zlib_rs::allocate::Allocator::C.zfree, + ); + + core::ptr::write( + core::ptr::addr_of_mut!((*strm.as_mut_ptr()).opaque), + core::ptr::null_mut(), + ); + + let err = deflateInit_( + strm.as_mut_ptr(), + 6, + zlibVersion(), + core::mem::size_of::() as _, + ); + assert_eq!(err, Z_OK); + + let err = deflateEnd(strm.as_mut_ptr()); + assert_eq!(err, Z_OK); + } + } } #[cfg(test)]