From b62b51378e380b65d31e2a57ae283f2e1ecc342c Mon Sep 17 00:00:00 2001 From: Michael Anderson Date: Tue, 13 Oct 2026 07:29:01 -0500 Subject: [PATCH] Never crash: Fix i32 overflow in the NEF and PEF Huffman predictors (#1515) Use checked adds and return a corrupt data error instead of panicking. --- crates/raw/src/vendor/nefc.rs | 38 ++++++++++++++++++++++++++++--- crates/raw/src/vendor/pef.rs | 42 ++++++++++++++++++++++++++++++----- 2 files changed, 72 insertions(+), 8 deletions(-) diff --git a/crates/raw/src/vendor/nefc.rs b/crates/raw/src/vendor/nefc.rs index c63f0fdc0..fd8e964b7 100644 --- a/crates/raw/src/vendor/nefc.rs +++ b/crates/raw/src/vendor/nefc.rs @@ -279,14 +279,17 @@ pub(crate) fn decode_rows(src: &[u8], w: usize, h: usize, rows: usize, bits: u32 } else { diff(&mut stream, &huff).ok_or_else(|| RawError::Corrupt(format!("NEF: invalid Huffman code at row {y}")))? }; + // a run of large differences in a corrupt stream can drive a predictor past i32 (issue #1515) + let overflow = || RawError::Corrupt(format!("NEF: predictor out of range at row {y}")); let p = if x < 2 { let s = seeds.get_mut(x).ok_or(RawError::Limit("NEF column"))?; - *s += d; + *s = s.checked_add(d).ok_or_else(overflow)?; hpred[x & 1] = *s; *s } else { - hpred[x & 1] += d; - hpred[x & 1] + let p = hpred[x & 1].checked_add(d).ok_or_else(overflow)?; + hpred[x & 1] = p; + p }; let v = p.clamp(0, max); clipped += (v != p && (table.split == 0 || y < table.split)) as usize; @@ -728,6 +731,35 @@ mod tests { assert!(decode_rows(&src[..w * h / 4 - 1], w, h, 1, 14, &t).is_err(), "a strip too short for the image is corrupt in any mode"); } + /// A long run of the largest difference drove a predictor past i32: a panic in builds with overflow checks, + /// wrapped values otherwise (issue #1515). Both the row and the column predictors now stop with an error. + #[test] + fn predictor_overflow_is_an_error_not_a_panic() { + let codes = LOSSLESS_14.codes(); + let k = (0..codes.len()).rev().find(|&k| codes[k].1 > 0).unwrap(); + let d = (1u32 << k) - 1; + let stream = |n: usize| { + let mut wr = Writer::default(); + let (code, len) = codes[k]; + for _ in 0..n { + wr.put(code as u32 >> (12 - len), len as u32); + wr.put(d, k as u32); + } + wr.finish() + }; + // differences one predictor needs to pass i32::MAX + let steps = (i32::MAX as u32 / d) as usize + 2; + let table = DecodeTable { encoding: Encoding::Lossless, seeds: [[0; 2]; 2], split: 0 }; + // along a row: every other sample adds to the same predictor + let w = 2 * steps + 2; + let err = decode(&stream(w), w, 1, 14, &table).unwrap_err(); + assert!(matches!(err, RawError::Corrupt(ref m) if m.contains("predictor")), "{err}"); + // down two columns: each row adds to the seed of its parity + let h = 2 * steps + 2; + let err = decode(&stream(2 * h), 2, h, 14, &table).unwrap_err(); + assert!(matches!(err, RawError::Corrupt(ref m) if m.contains("predictor")), "{err}"); + } + #[test] fn truncated_and_corrupted_streams_error_never_panic() { let (w, h) = (64, 16); diff --git a/crates/raw/src/vendor/pef.rs b/crates/raw/src/vendor/pef.rs index abb928e68..4020829e4 100644 --- a/crates/raw/src/vendor/pef.rs +++ b/crates/raw/src/vendor/pef.rs @@ -150,13 +150,17 @@ pub(crate) fn decode_huffman(src: &[u8], h: &Huffman, w: usize, hgt: usize, bits let row = &mut out[y * w..(y + 1) * w]; for (x, o) in row.iter_mut().enumerate() { let d = diff(&mut bits, h).ok_or_else(|| RawError::Corrupt(format!("PEF: invalid Huffman code at row {y}")))?; + // a run of large differences in a corrupt stream can drive a predictor past i32 (issue #1515) + let overflow = || RawError::Corrupt(format!("PEF: predictor out of range at row {y}")); let p = if x < 2 { - vpred[y & 1][x] += d; - hpred[x] = vpred[y & 1][x]; - hpred[x] + let v = vpred[y & 1][x].checked_add(d).ok_or_else(overflow)?; + vpred[y & 1][x] = v; + hpred[x] = v; + v } else { - hpred[x & 1] += d; - hpred[x & 1] + let v = hpred[x & 1].checked_add(d).ok_or_else(overflow)?; + hpred[x & 1] = v; + v }; *o = p.clamp(0, max) as u16; } @@ -390,6 +394,34 @@ mod tests { assert!(table(&tb[..20], ByteOrder::Big).is_err()); } + /// A long run of the largest difference drove a predictor past i32: a panic in builds with overflow checks, + /// wrapped values otherwise (issue #1515). Both the row and the column predictors now stop with an error. + #[test] + fn predictor_overflow_is_an_error_not_a_panic() { + let (tb, codes) = test_table(); + let h = table(&tb, ByteOrder::Big).unwrap(); + let (c, l) = codes[14]; + let d = (1u32 << 14) - 1; + let stream = |n: usize| { + let mut bits: Vec = Vec::with_capacity(n * (l as usize + 14)); + for _ in 0..n { + (0..l).rev().for_each(|i| bits.push((c >> (12 - l)) >> i & 1 == 1)); + (0..14).rev().for_each(|i| bits.push(d >> i & 1 == 1)); + } + bits.chunks(8).map(|c| c.iter().enumerate().fold(0u8, |a, (i, &b)| a | (b as u8) << (7 - i))).collect::>() + }; + // differences one predictor needs to pass i32::MAX + let steps = (i32::MAX as u32 / d) as usize + 2; + // along a row: every other sample adds to the same predictor + let w = 2 * steps + 2; + let err = decode_huffman(&stream(w), &h, w, 1, 14).unwrap_err(); + assert!(matches!(err, RawError::Corrupt(ref m) if m.contains("predictor")), "{err}"); + // down two columns: each row adds to the predictor of its parity + let rows = 2 * steps + 2; + let err = decode_huffman(&stream(2 * rows), &h, 2, rows, 14).unwrap_err(); + assert!(matches!(err, RawError::Corrupt(ref m) if m.contains("predictor")), "{err}"); + } + /// A big-endian one-strip PEF-style TIFF of `w × h` 12-bit samples with the given compression tag and strip /// byte count, around `payload`. fn tiny_pef(w: u16, h: u16, compression: u16, strip_len: u32, payload: &[u8]) -> Vec {