Skip to content

Commit c680a6b

Browse files
committed
parse: Fix addition overflow in add_consumed for chained extensions
The fuzzer found that chained extension headers (GNU long name/link, PAX) with large declared sizes can cause an arithmetic overflow in add_consumed() when the recursive parse_header calls unwind. Each layer adds total_size (HEADER_SIZE + padded_size) to the consumed or min_bytes field, and with Limits::permissive() allowing metadata up to u32::MAX, two such additions can overflow usize. Fix by using saturating_add instead of plain +. A saturated value (usize::MAX) will always exceed the input length, so callers correctly treat it as insufficient data. Crash: parse.rs:413 panic_const_add_overflow in GlobalExtensions variant, found by extended fuzzing on commit 518dcfb. Assisted-by: OpenCode (Claude Opus)
1 parent 4fee1e7 commit c680a6b

1 file changed

Lines changed: 65 additions & 5 deletions

File tree

src/parse.rs

Lines changed: 65 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -390,10 +390,10 @@ impl<'a> ParseEvent<'a> {
390390
fn add_consumed(self, n: usize) -> Self {
391391
match self {
392392
ParseEvent::NeedData { min_bytes } => ParseEvent::NeedData {
393-
min_bytes: min_bytes + n,
393+
min_bytes: min_bytes.saturating_add(n),
394394
},
395395
ParseEvent::Entry { consumed, entry } => ParseEvent::Entry {
396-
consumed: consumed + n,
396+
consumed: consumed.saturating_add(n),
397397
entry,
398398
},
399399
ParseEvent::SparseEntry {
@@ -402,17 +402,17 @@ impl<'a> ParseEvent<'a> {
402402
sparse_map,
403403
real_size,
404404
} => ParseEvent::SparseEntry {
405-
consumed: consumed + n,
405+
consumed: consumed.saturating_add(n),
406406
entry,
407407
sparse_map,
408408
real_size,
409409
},
410410
ParseEvent::GlobalExtensions { consumed, pax_data } => ParseEvent::GlobalExtensions {
411-
consumed: consumed + n,
411+
consumed: consumed.saturating_add(n),
412412
pax_data,
413413
},
414414
ParseEvent::End { consumed } => ParseEvent::End {
415-
consumed: consumed + n,
415+
consumed: consumed.saturating_add(n),
416416
},
417417
}
418418
}
@@ -4007,4 +4007,64 @@ mod tests {
40074007
}
40084008
}
40094009
}
4010+
4011+
/// Regression test: `add_consumed` must not overflow when chained
4012+
/// extension headers declare very large sizes.
4013+
///
4014+
/// With `Limits::permissive()` (`max_metadata_size = u32::MAX`),
4015+
/// extension headers can declare sizes close to `u32::MAX`. When
4016+
/// `handle_extension` recurses and the inner call returns `NeedData`,
4017+
/// `add_consumed(total_size)` is applied on unwind at each level.
4018+
/// Before the fix, `min_bytes + n` used plain `+` and could overflow
4019+
/// `usize` (especially on 32-bit targets). The fix uses
4020+
/// `saturating_add`. This test verifies the parser returns `NeedData`
4021+
/// (or an error) without panicking.
4022+
#[test]
4023+
fn test_add_consumed_no_overflow() {
4024+
// First extension: a complete GNU long name ('L') with a small
4025+
// payload so the parser can fully consume it and recurse.
4026+
let long_name = b"a]long/path".to_vec();
4027+
let gnu_entry = make_gnu_long_name(&long_name);
4028+
let first_entry_size = gnu_entry.len(); // 1024 bytes (header + padded name)
4029+
4030+
// Second extension: a PAX ('x') header that declares a size close
4031+
// to u32::MAX. We only provide the header—not the content—so the
4032+
// recursive call in handle_extension will return NeedData with
4033+
// min_bytes ≈ pax_size + 512. On unwind, add_consumed adds
4034+
// first_entry_size, giving min_bytes ≈ pax_size + 512 + 1024.
4035+
// On 32-bit this would overflow without saturating_add.
4036+
let pax_size: u64 = u32::MAX as u64 - long_name.len() as u64 - 512;
4037+
let pax_header = make_header(b"PaxHeaders/file", pax_size, b'x');
4038+
4039+
// Build input: complete GNU long name entry + PAX header only (no
4040+
// PAX content).
4041+
let mut input = Vec::with_capacity(first_entry_size + HEADER_SIZE);
4042+
input.extend_from_slice(&gnu_entry);
4043+
input.extend_from_slice(&pax_header);
4044+
4045+
let mut parser = Parser::new(Limits::permissive());
4046+
let result = parser.parse(&input);
4047+
4048+
// The parser must not panic. It should return NeedData (because the
4049+
// PAX content is missing) or an error—both are acceptable.
4050+
match result {
4051+
Ok(ParseEvent::NeedData { min_bytes }) => {
4052+
// min_bytes must be at least the PAX entry's total_size
4053+
// (header + padded content), and must not have wrapped to
4054+
// a small value due to overflow.
4055+
assert!(
4056+
min_bytes > HEADER_SIZE,
4057+
"min_bytes should be large, got {min_bytes}"
4058+
);
4059+
}
4060+
Err(_) => {
4061+
// An error (e.g. metadata too large) is also acceptable;
4062+
// the important thing is no panic from arithmetic overflow.
4063+
}
4064+
other => panic!(
4065+
"Expected NeedData or Err for truncated extension chain, got {:?}",
4066+
other
4067+
),
4068+
}
4069+
}
40104070
}

0 commit comments

Comments
 (0)