diff --git a/crates/raw/src/lib.rs b/crates/raw/src/lib.rs index 3e5093d73..3a7f04a95 100644 --- a/crates/raw/src/lib.rs +++ b/crates/raw/src/lib.rs @@ -743,7 +743,10 @@ impl RawImage { return Err(RawError::Unsupported(format!("CFA pattern {}", c.name()))); } let a = self.active_area; - if a.width == 0 || a.height == 0 || a.x + a.width > self.width || a.y + a.height > self.height { + // checked: a crop origin from a maker note can be `u64::MAX`, and `x + width` would wrap + // back inside the image here and again in `normalized`'s index arithmetic. + let fits = |x: usize, len: usize, limit: usize| x.checked_add(len).is_some_and(|end| end <= limit); + if a.width == 0 || a.height == 0 || !fits(a.x, a.width, self.width) || !fits(a.y, a.height, self.height) { return Err(RawError::Corrupt("active area outside image".into())); } Ok(()) @@ -893,6 +896,37 @@ mod tests { assert_eq!(whole.develop_crop(8, 6), Rect::new(6, 4, 2, 2)); } + /// A maker-note crop (`PEF` AOC / `ORF` ImageProcessing) with an origin of `u64::MAX` used to + /// pass the bounds check (`x + width` wraps to 0) and then index out of bounds in + /// `normalized`. It must be a `Corrupt` error at validate time instead. + #[test] + fn an_active_area_that_wraps_past_the_image_is_corrupt() { + let (w, h) = (12usize, 10usize); + let mut raw = RawImage { + format: RawFormat::Dng, + width: w, + height: h, + cpp: 1, + data: RawData::U16(vec![100; w * h]), + cfa: Cfa::bayer("RGGB"), + bits: 12, + black: BlackLevel::uniform(64.0), + white: vec![1023.0], + active_area: Rect::new(usize::MAX, 0, 1, 1), + crop: Rect::default(), + orientation: Orientation::Normal, + color: ColorData::default(), + wb_multipliers: None, + linearized: false, + opcodes: OpcodeLists::default(), + metadata: Metadata::default(), + }; + assert!(raw.validate().is_err(), "`x + width` wrapped: the area is outside the image"); + assert!(raw.normalized().is_err()); + raw.active_area = Rect::new(0, 0, w + 1, h); + assert!(raw.validate().is_err(), "one past the last column is outside too"); + } + #[test] fn black_levels() { let b = BlackLevel { repeat_rows: 2, repeat_cols: 2, values: vec![1.0, 2.0, 3.0, 4.0], delta_h: vec![0.5, 0.0], delta_v: vec![0.0, 10.0] }; diff --git a/crates/raw/src/vendor/orf.rs b/crates/raw/src/vendor/orf.rs index ea7f30eb7..31dd2d21b 100644 --- a/crates/raw/src/vendor/orf.rs +++ b/crates/raw/src/vendor/orf.rs @@ -285,7 +285,12 @@ pub(crate) fn decode(bytes: &[u8], mode: Mode) -> Result { let mn = maker_note(bytes, &tiff); let ip = mn.as_ref().and_then(|m| sub_ifd(bytes, m, IMAGE_PROCESSING)); let active = match ip.as_ref().map(|i| CROP.map(|tag| i.u64(tag).map(|v| v as usize))) { - Some([Some(x), Some(y), Some(cw), Some(ch)]) if cw > 0 && ch > 0 && x + cw <= w && y + ch <= h => Rect::new(x, y, cw, ch), + // checked: the tags are file data; `x + cw` on a hostile value would wrap past the bounds + Some([Some(x), Some(y), Some(cw), Some(ch)]) + if cw > 0 && ch > 0 && x.checked_add(cw).is_some_and(|end| end <= w) && y.checked_add(ch).is_some_and(|end| end <= h) => + { + Rect::new(x, y, cw, ch) + } _ => Rect::new(0, 0, w, h), }; let cfa = stated.unwrap_or_else(|| cfa_from_data(samples, w, active)); diff --git a/crates/raw/src/vendor/pef.rs b/crates/raw/src/vendor/pef.rs index 49486be74..d8e84db99 100644 --- a/crates/raw/src/vendor/pef.rs +++ b/crates/raw/src/vendor/pef.rs @@ -249,7 +249,12 @@ pub(crate) fn decode(bytes: &[u8], mode: Mode) -> Result { tiff.exif().and_then(|e| e.get(t::MAKER_NOTE)).and_then(|e| makernote::parse_makernote(bytes, e.offset, e.count() as u64, tiff.order, &make)); let pair = |tag: u16| mn.as_ref().and_then(|m| m.ifd.u64s(tag)).filter(|v| v.len() == 2).map(|v| (v[0] as usize, v[1] as usize)); let tagged = match (pair(CROP_ORIGIN), pair(CROP_SIZE)) { - (Some((x, y)), Some((cw, ch))) if cw > 0 && ch > 0 && x + cw <= w && y + ch <= hgt => Some(Rect::new(x, y, cw, ch)), + // checked: the tags are file data; `x + cw` on a hostile value would wrap past the bounds + (Some((x, y)), Some((cw, ch))) + if cw > 0 && ch > 0 && x.checked_add(cw).is_some_and(|end| end <= w) && y.checked_add(ch).is_some_and(|end| end <= hgt) => + { + Some(Rect::new(x, y, cw, ch)) + } _ => None, }; // without crop tags the image area is found from the samples (dark borders)