Skip to content

Commit

Permalink
fix(inflate): Guard against edge case with invalid match distance wra…
Browse files Browse the repository at this point in the history
…pping around too far when using wrapping buffer

fixes #161

Might still need a better check here for wrapping buffers as the code might in some edge cases result in wrong error/garbled data instead of correct error with invalid match distances that refer past the start
  • Loading branch information
oyvindln committed Feb 11, 2025
1 parent 7014124 commit 4037fee
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 5 deletions.
2 changes: 1 addition & 1 deletion miniz_oxide/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ compiler_builtins = { version = '0.1.2', optional = true }
[dev-dependencies]
## Messes with minimum rust version and drags in deps just for running tests
## so just comment out for now and enable manually when needed for enabling benches
# criterion = "0.5"
criterion = "0.5"

[[bench]]
name = "benchmark"
Expand Down
12 changes: 8 additions & 4 deletions miniz_oxide/src/inflate/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1000,6 +1000,9 @@ fn apply_match(
transfer(out_slice, source_pos, out_pos, match_len, out_buf_size_mask);
} else if match_len <= dist && source_pos + match_len < out_slice.len() {
// Destination and source segments does not intersect and source does not wrap.
// TODO: An invalid before start of data wrapping match reached here before
// it was fixed (it wrapped around and ended overlapping again)- need
// to check that we are not wrapping here.
if source_pos < out_pos {
let (from_slice, to_slice) = out_slice.split_at_mut(out_pos);
to_slice[..match_len].copy_from_slice(&from_slice[source_pos..source_pos + match_len]);
Expand Down Expand Up @@ -1140,8 +1143,9 @@ fn decompress_fast(
}

let position = out_buf.position();
if l.dist as usize > out_buf.position()
&& (flags & TINFL_FLAG_USING_NON_WRAPPING_OUTPUT_BUF != 0)
if (l.dist as usize > out_buf.position()
&& (flags & TINFL_FLAG_USING_NON_WRAPPING_OUTPUT_BUF != 0))
|| (l.dist as usize > out_buf.get_ref().len())
{
// We encountered a distance that refers a position before
// the start of the decoded data, so we can't continue.
Expand Down Expand Up @@ -1665,8 +1669,8 @@ pub fn decompress(
}),

HuffDecodeOuterLoop2 => generate_state!(state, 'state_machine, {
if l.dist as usize > out_buf.position() &&
(flags & TINFL_FLAG_USING_NON_WRAPPING_OUTPUT_BUF != 0)
if (l.dist as usize > out_buf.position() &&
(flags & TINFL_FLAG_USING_NON_WRAPPING_OUTPUT_BUF != 0)) || (l.dist as usize > out_buf.get_ref().len())
{
// We encountered a distance that refers a position before
// the start of the decoded data, so we can't continue.
Expand Down
21 changes: 21 additions & 0 deletions miniz_oxide/tests/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,27 @@ fn decompress_empty_dynamic() {
assert!(res.is_err());
}

fn decode_hex(s: &str) -> Vec<u8> {
(0..s.len())
.step_by(2)
.map(|i| u8::from_str_radix(&s[i..i + 2], 16).unwrap())
.collect::<Vec<_>>()
}

#[test]
fn issue_161_index_out_of_range_apply_match() {
// This data contains an match that has a distance before the start of the data.
// and resulted in an edge cause causing a panic instead of returning with an error when using.
// a smaller wrapping buffer.
let content_hex = "fa99fff4f37fef5bbff9bb6ccb9ab4e47f66d9875cebf9ffe6eb6fbdf6e24b773f72ebe5175f62ff26bf78eec57bafdd78ee6b5f7efeee2b2f5b1d2bfe5100";
let content = decode_hex(&content_hex);

let mut decompressor = miniz_oxide::inflate::core::DecompressorOxide::new();

let mut buf2 = vec![0; 2048];
let _ = miniz_oxide::inflate::core::decompress(&mut decompressor, &content, &mut buf2, 0, 0);
}

/*
#[test]
fn partial_decompression_imap_issue_158() {
Expand Down

0 comments on commit 4037fee

Please sign in to comment.