Skip to content

Commit

Permalink
fix: checked-decode
Browse files Browse the repository at this point in the history
fixes checked decode checks
revert #69 as this leads to out of bounds writes
  • Loading branch information
PSeitz committed Apr 30, 2023
1 parent 894b13c commit 5f5ca4d
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 6 deletions.
2 changes: 1 addition & 1 deletion src/block/decompress.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ unsafe fn duplicate_overlapping(
// Note that we copy 4 bytes, instead of one.
// Without that the compiler will unroll/auto-vectorize the copy with a lot of branches.
// This is not what we want, as large overlapping copies are not that common.
core::ptr::copy(start, *output_ptr, 4);
core::ptr::copy(start, *output_ptr, 1);
start = start.add(1);
*output_ptr = output_ptr.add(1);
}
Expand Down
11 changes: 9 additions & 2 deletions src/block/decompress_safe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,10 @@ pub(crate) fn decompress_internal<const USE_DICT: bool, S: Sink>(
// In this branch we know that match_length is at most 18 (14 + MINMATCH).
// But the blocks can overlap, so make sure they are at least 18 bytes apart
// to enable an optimized copy of 18 bytes.
let start = output.pos() - offset;
let (start, did_overflow) = output.pos().overflowing_sub(offset);
if did_overflow {
return Err(DecompressError::OffsetOutOfBounds);
}
if offset >= match_length {
output.extend_from_within(start, 18, match_length);
} else {
Expand Down Expand Up @@ -261,7 +264,11 @@ fn duplicate_slice(
if match_length > offset {
duplicate_overlapping_slice(output, offset, match_length)?;
} else {
let start = output.pos() - offset;
let (start, did_overflow) = output.pos().overflowing_sub(offset);
if did_overflow {
return Err(DecompressError::OffsetOutOfBounds);
}

match match_length {
0..=32 if output.pos() + 32 <= output.capacity() => {
output.extend_from_within(start, 32, match_length)
Expand Down
18 changes: 15 additions & 3 deletions tests/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -322,25 +322,37 @@ fn print_compression_ration(input: &'static [u8], name: &str) {
mod checked_decode {
use super::*;

#[cfg_attr(not(feature = "checked_decode"), ignore)]
#[cfg_attr(not(feature = "checked-decode"), ignore)]
#[test]
fn error_case_1() {
let _err = decompress_size_prepended(&[122, 1, 0, 1, 0, 10, 1, 0]);
}
#[cfg_attr(not(feature = "checked_decode"), ignore)]
#[cfg_attr(not(feature = "checked-decode"), ignore)]
#[test]
fn error_case_2() {
let _err = decompress_size_prepended(&[
44, 251, 49, 0, 0, 0, 8, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 16, 0, 0, 0, 0, 0, 0, 0, 0,
]);
}
#[cfg_attr(not(feature = "checked_decode"), ignore)]
#[cfg_attr(not(feature = "checked-decode"), ignore)]
#[test]
fn error_case_3() {
let _err = decompress_size_prepended(&[
7, 0, 0, 0, 0, 0, 0, 11, 0, 0, 7, 16, 0, 0, 0, 0, 0, 0, 0, 0, 0, 4, 1, 0, 0,
]);
}

#[cfg_attr(not(feature = "checked-decode"), ignore)]
#[test]
fn error_case_4() {
let _err = decompress_size_prepended(&[0, 61, 0, 0, 0, 7, 0]);
}

#[cfg_attr(not(feature = "checked-decode"), ignore)]
#[test]
fn error_case_5() {
let _err = decompress_size_prepended(&[8, 0, 0, 0, 4, 0, 0, 0]);
}
}

#[test]
Expand Down

0 comments on commit 5f5ca4d

Please sign in to comment.