Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix for reading/writing uninitialized memory #211

Merged
merged 5 commits into from
Oct 4, 2024
Merged

fix for reading/writing uninitialized memory #211

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
48cf2c5
add some buffer in the window so SIMD reads from it are always allowed
folkertdev Oct 3, 2024
0306b20
fix a number of miri bugs (mostly leaking memory)
folkertdev Oct 3, 2024
01e3f9a
fix out of bounds write
folkertdev Oct 3, 2024
a0b333e
fix memory leak in test
folkertdev Oct 3, 2024
1162053
disable very slow test for miri
folkertdev Oct 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
191 changes: 52 additions & 139 deletions test-libz-rs-sys/src/inflate.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ use zlib_rs::deflate::{compress_slice, DeflateConfig};
use zlib_rs::inflate::{set_mode_dict, uncompress_slice, InflateConfig, INFLATE_STATE_SIZE};
use zlib_rs::{InflateFlush, ReturnCode, MAX_WBITS};

use crate::assert_eq_rs_ng;

const VERSION: *const c_char = libz_rs_sys::zlibVersion();
const STREAM_SIZE: c_int = core::mem::size_of::<libz_rs_sys::z_stream>() as c_int;

Expand Down Expand Up @@ -203,9 +205,9 @@ fn gzip_header_check() {
}

let mut out = vec![0u8; len];
let extra: [u8; 14] = [0; 14];
let name: [u8; 9] = [0; 9];
let comment: [u8; 10] = [0; 10];
let mut extra: [u8; 14] = [0; 14];
let mut name: [u8; 9] = [0; 9];
let mut comment: [u8; 10] = [0; 10];

// Set header
// See: https://www.zlib.net/manual.html
Expand All @@ -214,12 +216,12 @@ fn gzip_header_check() {
time: 0,
xflags: 0,
os: 0,
extra: extra.as_ptr() as *mut u8,
extra: extra.as_mut_ptr(),
extra_len: 0,
extra_max: 14,
name: name.as_ptr() as *mut u8,
name: name.as_mut_ptr(),
name_max: 9, // How / where should this be set?
comment: comment.as_ptr() as *mut u8,
comment: comment.as_mut_ptr(),
comm_max: 10,
hcrc: 0,
done: 0,
Expand Down Expand Up @@ -291,9 +293,9 @@ fn inf(input: &[u8], _what: &str, step: usize, win: i32, len: usize, err: c_int)

let mut out = vec![0u8; len];

let extra: [u8; 1024] = [0; 1024];
let name: [u8; 64] = [0; 64];
let comment: [u8; 64] = [0; 64];
let mut extra: [u8; 1024] = [0; 1024];
let mut name: [u8; 64] = [0; 64];
let mut comment: [u8; 64] = [0; 64];

// Set header
// See: https://www.zlib.net/manual.html
Expand All @@ -302,12 +304,12 @@ fn inf(input: &[u8], _what: &str, step: usize, win: i32, len: usize, err: c_int)
time: 0,
xflags: 0,
os: 0,
extra: extra.as_ptr() as *mut u8,
extra: extra.as_mut_ptr(),
extra_len: 0,
extra_max: 1024,
name: name.as_ptr() as *mut u8,
name: name.as_mut_ptr(),
name_max: 64, // How / where should this be set?
comment: comment.as_ptr() as *mut u8,
comment: comment.as_mut_ptr(),
comm_max: 64,
hcrc: 0,
done: 0,
Expand Down Expand Up @@ -1030,7 +1032,7 @@ fn hello_world_uncompressed() {

#[test]
fn copy_direct_from_next_in_to_next_out() {
crate::assert_eq_rs_ng!({
assert_eq_rs_ng!({
let input = [120, 1, 1, 2, 0, 253, 255, 6, 10, 0, 24, 0, 17];
let mut dest_vec = vec![0u8; 1 << 16];

Expand Down Expand Up @@ -1143,6 +1145,11 @@ fn inflate_get_header_non_gzip_stream() {
unsafe { inflateGetHeader(&mut stream, &mut header) },
ReturnCode::StreamError as i32
);

let ret = unsafe { inflateEnd(&mut stream) };
assert_eq!(ret, Z_OK);

mem_done(&mut stream);
}

#[test]
Expand Down Expand Up @@ -1424,6 +1431,8 @@ fn chunked_output_rs() {
let err = unsafe { inflate(stream, InflateFlush::Finish as _) };
assert_eq!(ReturnCode::from(err), ReturnCode::StreamEnd);

unsafe { inflateEnd(stream) };

assert_eq!(stream.total_out, 33);
}

Expand All @@ -1440,6 +1449,9 @@ fn version_error() {
};
assert_eq!(ret, Z_OK);

let ret = unsafe { inflateEnd(stream.as_mut_ptr()) };
assert_eq!(ret, Z_OK);

// invalid stream size
let ret = unsafe { inflateInit_(stream.as_mut_ptr(), zlibVersion(), 1) };
assert_eq!(ret, Z_VERSION_ERROR);
Expand Down Expand Up @@ -1480,14 +1492,13 @@ fn version_error() {
fn issue_109() {
let input = &include_bytes!("test-data/issue-109.gz")[10..][..32758];

let mut output_rs: Vec<u8> = Vec::with_capacity(1 << 15);
let mut output_ng: Vec<u8> = Vec::with_capacity(1 << 15);

let window_bits = -15;

let mut buf = [0; 8192];
assert_eq_rs_ng!({
let mut output: Vec<u8> = Vec::with_capacity(1 << 15);

let mut buf = [0; 8192];

{
let mut stream = MaybeUninit::<z_stream>::zeroed();

let err = unsafe {
Expand All @@ -1512,48 +1523,17 @@ fn issue_109() {
let err = unsafe { inflate(stream, InflateFlush::NoFlush as _) };

if ReturnCode::from(err) == ReturnCode::BufError {
output_rs.extend(&buf[..stream.avail_out as usize]);
output.extend(&buf[..stream.avail_out as usize]);
stream.avail_out = buf.len() as _;
continue;
}
}
}

{
use libz_sys::*;

let mut stream = MaybeUninit::<libz_sys::z_stream>::zeroed();

let err = unsafe {
inflateInit2_(
stream.as_mut_ptr(),
window_bits,
zlibVersion(),
core::mem::size_of::<z_stream>() as c_int,
)
};
let err = inflateEnd(stream);
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 {
output_ng.extend(&buf[..stream.avail_out as usize]);
stream.avail_out = buf.len() as _;
continue;
}
}
}

assert_eq!(output_rs, output_ng);
output
});
}

#[test]
Expand All @@ -1564,12 +1544,10 @@ fn window_match_bug() {

let window_bits = -10;

let mut output_rs: Vec<u8> = Vec::with_capacity(1 << 15);
let mut output_ng: Vec<u8> = Vec::with_capacity(1 << 15);

let mut buf = [0; 402];
assert_eq_rs_ng!({
let mut output: Vec<u8> = Vec::with_capacity(1 << 15);
let mut buf = [0; 402];

{
let mut stream = MaybeUninit::<z_stream>::zeroed();

let err = unsafe {
Expand All @@ -1593,7 +1571,7 @@ fn window_match_bug() {

let err = unsafe { inflate(stream, InflateFlush::Finish as _) };

output_rs.extend(&buf[..buf.len() - stream.avail_out as usize]);
output.extend(&buf[..buf.len() - stream.avail_out as usize]);

match ReturnCode::from(err) {
ReturnCode::BufError => {
Expand All @@ -1604,48 +1582,11 @@ fn window_match_bug() {
other => panic!("unexpected {:?}", other),
}
}
}

{
use libz_sys::*;

let mut stream = MaybeUninit::<z_stream>::zeroed();

let err = unsafe {
inflateInit2_(
stream.as_mut_ptr(),
window_bits,
zlibVersion(),
core::mem::size_of::<z_stream>() 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::Finish as _) };

output_ng.extend(&buf[..buf.len() - stream.avail_out as usize]);

match ReturnCode::from(err) {
ReturnCode::BufError => {
assert_eq!(stream.avail_out, 0);
stream.avail_out = buf.len() as _;
}
ReturnCode::StreamEnd => break,
other => panic!("unexpected {:?}", other),
}
}
}
inflateEnd(stream);

assert_eq!(output_rs, output_ng);
output
});
}

#[test]
Expand All @@ -1654,12 +1595,11 @@ fn op_len_edge_case() {

let input = &include_bytes!("test-data/op-len-edge-case.zraw");

let mut output_rs: Vec<u8> = Vec::with_capacity(1 << 15);
let mut output_ng: Vec<u8> = Vec::with_capacity(1 << 15);
assert_eq_rs_ng!({
let mut output: Vec<u8> = Vec::with_capacity(1 << 15);

let mut buf = [0; 266];
let mut buf = [0; 266];

{
let mut stream = MaybeUninit::<z_stream>::zeroed();

let err = unsafe {
Expand All @@ -1684,48 +1624,17 @@ fn op_len_edge_case() {
let err = unsafe { inflate(stream, InflateFlush::NoFlush as _) };

if ReturnCode::from(err) == ReturnCode::BufError {
output_rs.extend(&buf[..stream.avail_out as usize]);
output.extend(&buf[..stream.avail_out as usize]);
stream.avail_out = buf.len() as _;
continue;
}
}
}

{
use libz_sys::*;

let mut stream = MaybeUninit::<libz_sys::z_stream>::zeroed();

let err = unsafe {
inflateInit2_(
stream.as_mut_ptr(),
window_bits,
zlibVersion(),
core::mem::size_of::<z_stream>() as c_int,
)
};
let err = inflateEnd(stream);
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 {
output_ng.extend(&buf[..stream.avail_out as usize]);
stream.avail_out = buf.len() as _;
continue;
}
}
}

assert_eq!(output_rs, output_ng);
output
});
}

// Fills the provided buffer with pseudorandom bytes based on the given seed
Expand All @@ -1745,6 +1654,7 @@ fn prng_bytes(seed: u64, bytes: &mut [u8], step: usize) {
}

#[test]
#[cfg_attr(miri, ignore = "slow")]
fn test_inflate_flush_block() {
let window_bits = -15; // Raw
const CHUNK: usize = 16384;
Expand Down Expand Up @@ -1847,7 +1757,7 @@ fn issue_172() {
45, 1, 0, 176, 1, 57, 179, 15, 0, 0, 0,
];

crate::assert_eq_rs_ng!({
assert_eq_rs_ng!({
let mut exitcode = 0;
for chunk in 1..BUF.len() {
let mut ret;
Expand Down Expand Up @@ -1881,6 +1791,9 @@ fn issue_172() {
eprintln!("Output did not match at chunk size {}\n", chunk);
exitcode = 1;
}

let err = inflateEnd(strm);
assert_eq!(ReturnCode::from(err), ReturnCode::Ok);
}

assert!(exitcode == 0);
Expand Down
8 changes: 4 additions & 4 deletions zlib-rs/src/inflate.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -707,9 +707,8 @@ impl<'a> State<'a> {
if (self.gzip_flags & 0x0400) != 0 {
// self.length is the number of remaining `extra` bytes. But they may not all be available
let extra_available = Ord::min(self.length, self.bit_reader.bytes_remaining());
let extra_slice = &self.bit_reader.as_slice()[..extra_available];

if !extra_slice.is_empty() {
if extra_available > 0 {
if let Some(head) = self.head.as_mut() {
if !head.extra.is_null() {
// at `head.extra`, the caller has reserved `head.extra_max` bytes.
Expand All @@ -723,7 +722,7 @@ impl<'a> State<'a> {
// min of number of bytes available at dst and at src
let count = Ord::min(
(head.extra_max as usize).saturating_sub(written_so_far),
extra_slice.len(),
extra_available,
);

// location where we'll write: this saturates at the
Expand All @@ -732,7 +731,7 @@ impl<'a> State<'a> {

unsafe {
core::ptr::copy_nonoverlapping(
self.bit_reader.as_ptr(),
self.bit_reader.as_mut_ptr(),
head.extra.add(next_write_offset),
count,
);
Expand All @@ -742,6 +741,7 @@ impl<'a> State<'a> {

// Checksum
if (self.gzip_flags & 0x0200) != 0 && (self.wrap & 4) != 0 {
let extra_slice = &self.bit_reader.as_slice()[..extra_available];
self.checksum = crc32(self.checksum, extra_slice)
}

Expand Down
Loading
Loading