Skip to content

Commit f6c91a5

Browse files
committed
use uninitialized output buffers for the fuzzers
we previously missed a bug in zlib-rs due to only using initialized output buffers
1 parent 9c0c105 commit f6c91a5

File tree

3 files changed

+58
-38
lines changed

3 files changed

+58
-38
lines changed

fuzz/fuzz_targets/compress_then_decompress_chunked.rs

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -13,17 +13,16 @@ use libfuzzer_sys::fuzz_target;
1313

1414
/// compress the data with the stock C bzip2
1515
fn compress_c(data: &[u8], compression_level: u8, work_factor: u8) -> Vec<u8> {
16-
1716
// output buffer for compression, will get resized later if needed
18-
let mut output = vec![0u8; 1024];
17+
let mut output = Vec::<u8>::with_capacity(1024);
1918

2019
let mut stream = libbz2_rs_sys::bz_stream {
2120
next_in: data.as_ptr() as *mut _,
2221
avail_in: data.len() as _,
2322
total_in_lo32: 0,
2423
total_in_hi32: 0,
24+
avail_out: output.capacity() as _,
2525
next_out: output.as_mut_ptr() as *mut _,
26-
avail_out: output.len() as _,
2726
total_out_lo32: 0,
2827
total_out_hi32: 0,
2928
state: std::ptr::null_mut(),
@@ -46,10 +45,14 @@ fn compress_c(data: &[u8], compression_level: u8, work_factor: u8) -> Vec<u8> {
4645
let error = loop {
4746
match unsafe { libbz2_rs_sys::BZ2_bzCompress(&mut stream, BZ_FINISH) } {
4847
BZ_FINISH_OK => {
49-
let used = output.len() - stream.avail_out as usize;
48+
let used = output.capacity() - stream.avail_out as usize;
49+
50+
// Safety: we've written this many (initialized!) bytes to the output.
51+
unsafe { output.set_len(used) };
52+
5053
// The output buffer is full, resize it
51-
let add_space: u32 = Ord::max(1024, output.len().try_into().unwrap());
52-
output.resize(output.len() + add_space as usize, 0);
54+
let add_space: u32 = Ord::max(1024, output.capacity().try_into().unwrap());
55+
output.reserve(add_space as usize);
5356

5457
// If resize() reallocates, it may have moved in memory
5558
stream.next_out = output.as_mut_ptr().cast::<i8>().wrapping_add(used);
@@ -70,11 +73,11 @@ fn compress_c(data: &[u8], compression_level: u8, work_factor: u8) -> Vec<u8> {
7073
assert_eq!(error, BZ_OK);
7174

7275
// truncate the output buffer down to the actual number of compressed bytes
73-
output.truncate(
74-
((u64::from(stream.total_out_hi32) << 32) + u64::from(stream.total_out_lo32))
75-
.try_into()
76-
.unwrap(),
77-
);
76+
let total = u64::from(stream.total_out_hi32) << 32 | u64::from(stream.total_out_lo32);
77+
unsafe { output.set_len(usize::try_from(total).unwrap()) };
78+
79+
// Just check that this byte is in fact initialized.
80+
std::hint::black_box(output.last() == Some(&0));
7881

7982
unsafe {
8083
// cleanup, should always succeed

fuzz/fuzz_targets/decompress_chunked.rs

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -46,10 +46,13 @@ fuzz_target!(|fuzz_data: &[u8]| -> Corpus {
4646
Corpus::Keep
4747
};
4848

49-
// initialize output buffer, resizing will happen later if needed
50-
// using the input length is just a quick heuristic and not otherwise meaningful or necessary
51-
let mut output = vec![0u8; fuzz_data.len()];
52-
let output_len = output.len() as _;
49+
// Initialize output buffer, resizing will happen later if needed
50+
// using the input length is just a quick heuristic and not otherwise meaningful or necessary.
51+
//
52+
// Deliberately use uninitialized memory for the output, to ensure we never
53+
// read/use uninitialized data.
54+
let mut output = Vec::<u8>::with_capacity(fuzz_data.len());
55+
let output_len = output.capacity() as _;
5356

5457
// set output buffer in decompression context
5558
stream.next_out = output.as_mut_ptr().cast::<core::ffi::c_char>();
@@ -73,10 +76,14 @@ fuzz_target!(|fuzz_data: &[u8]| -> Corpus {
7376
// there is still input from this chunk left to process but no more output buffer
7477
// this means we have to increase the output buffer and retry the decompress
7578
if stream.avail_in > 0 && stream.avail_out == 0 {
76-
let used = output.len() - stream.avail_out as usize;
79+
let used = output.capacity() - stream.avail_out as usize;
80+
81+
// Safety: we've written this many (initialized!) bytes to the output.
82+
unsafe { output.set_len(used) };
83+
7784
// The dest buffer is full, resize it
78-
let add_space: u32 = Ord::max(4096, output.len().try_into().unwrap());
79-
output.resize(output.len() + add_space as usize, 0);
85+
let add_space: u32 = Ord::max(4096, output.capacity().try_into().unwrap());
86+
output.reserve(add_space as usize);
8087

8188
// If resize() reallocates, it may have moved in memory
8289
stream.next_out = output.as_mut_ptr().cast::<i8>().wrapping_add(used);
@@ -108,6 +115,12 @@ fuzz_target!(|fuzz_data: &[u8]| -> Corpus {
108115
}
109116
}
110117

118+
let total = u64::from(stream.total_out_hi32) << 32 | u64::from(stream.total_out_lo32);
119+
unsafe { output.set_len(usize::try_from(total).unwrap()) };
120+
121+
// Just check that this byte is in fact initialized.
122+
std::hint::black_box(output.last() == Some(&0));
123+
111124
unsafe {
112125
// clean up state, should always succeed
113126
let err = BZ2_bzDecompressEnd(&mut stream);

test-libbz2-rs-sys/src/lib.rs

Lines changed: 24 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,8 @@ pub unsafe fn decompress_c_with_capacity(
260260
opaque: std::ptr::null_mut::<libc::c_void>(),
261261
};
262262

263-
let mut dest = vec![0u8; capacity];
263+
// Deliberately use uninitialized memory for the output.
264+
let mut dest = Vec::<u8>::with_capacity(capacity);
264265

265266
strm.bzalloc = None;
266267
strm.bzfree = None;
@@ -271,7 +272,7 @@ pub unsafe fn decompress_c_with_capacity(
271272
return (ret, vec![]);
272273
}
273274
strm.avail_in = source_len;
274-
strm.avail_out = dest.len() as _;
275+
strm.avail_out = dest.capacity() as _;
275276
strm.next_in = source as *mut libc::c_char;
276277
strm.next_out = dest.as_mut_ptr().cast::<core::ffi::c_char>();
277278

@@ -282,10 +283,14 @@ pub unsafe fn decompress_c_with_capacity(
282283
BZ2_bzDecompressEnd(&mut strm);
283284
break BZ_UNEXPECTED_EOF;
284285
} else {
285-
let used = dest.len() - strm.avail_out as usize;
286+
let used = dest.capacity() - strm.avail_out as usize;
287+
288+
// We've written this many (initialized!) bytes to the output.
289+
dest.set_len(used);
290+
286291
// The dest buffer is full.
287-
let add_space: u32 = Ord::max(1024, dest.len().try_into().unwrap());
288-
dest.resize(dest.len() + add_space as usize, 0);
292+
let add_space: u32 = Ord::max(1024, dest.capacity().try_into().unwrap());
293+
dest.reserve(add_space as usize);
289294

290295
// If resize() reallocates, it may have moved in memory.
291296
strm.next_out = dest.as_mut_ptr().cast::<i8>().wrapping_add(used);
@@ -305,11 +310,8 @@ pub unsafe fn decompress_c_with_capacity(
305310
}
306311
};
307312

308-
dest.truncate(
309-
((u64::from(strm.total_out_hi32) << 32) + u64::from(strm.total_out_lo32))
310-
.try_into()
311-
.unwrap(),
312-
);
313+
let total = (u64::from(strm.total_out_hi32) << 32) + u64::from(strm.total_out_lo32);
314+
dest.set_len(usize::try_from(total).unwrap());
313315

314316
(ret, dest)
315317
}
@@ -341,7 +343,8 @@ pub unsafe fn decompress_rs_with_capacity(
341343
opaque: std::ptr::null_mut::<libc::c_void>(),
342344
};
343345

344-
let mut dest = vec![0u8; capacity];
346+
// Deliberately use uninitialized memory for the output.
347+
let mut dest = Vec::<u8>::with_capacity(capacity);
345348

346349
strm.bzalloc = None;
347350
strm.bzfree = None;
@@ -352,7 +355,7 @@ pub unsafe fn decompress_rs_with_capacity(
352355
return (ret, vec![]);
353356
}
354357
strm.avail_in = source_len;
355-
strm.avail_out = dest.len() as _;
358+
strm.avail_out = dest.capacity() as _;
356359
strm.next_in = source as *mut libc::c_char;
357360
strm.next_out = dest.as_mut_ptr().cast::<core::ffi::c_char>();
358361

@@ -363,10 +366,14 @@ pub unsafe fn decompress_rs_with_capacity(
363366
BZ2_bzDecompressEnd(&mut strm);
364367
break BZ_UNEXPECTED_EOF;
365368
} else {
366-
let used = dest.len() - strm.avail_out as usize;
369+
let used = dest.capacity() - strm.avail_out as usize;
370+
371+
// We've written this many (initialized!) bytes to the output.
372+
dest.set_len(used);
373+
367374
// The dest buffer is full.
368-
let add_space: u32 = Ord::max(1024, dest.len().try_into().unwrap());
369-
dest.resize(dest.len() + add_space as usize, 0);
375+
let add_space: u32 = Ord::max(1024, dest.capacity().try_into().unwrap());
376+
dest.reserve(add_space as usize);
370377

371378
// If resize() reallocates, it may have moved in memory.
372379
strm.next_out = dest.as_mut_ptr().cast::<i8>().wrapping_add(used);
@@ -386,11 +393,8 @@ pub unsafe fn decompress_rs_with_capacity(
386393
}
387394
};
388395

389-
dest.truncate(
390-
((u64::from(strm.total_out_hi32) << 32) + u64::from(strm.total_out_lo32))
391-
.try_into()
392-
.unwrap(),
393-
);
396+
let total = (u64::from(strm.total_out_hi32) << 32) + u64::from(strm.total_out_lo32);
397+
dest.set_len(usize::try_from(total).unwrap());
394398

395399
(ret, dest)
396400
}

0 commit comments

Comments
 (0)