mirror of
https://github.com/storytold/lightcraft.git
synced 2026-10-24 18:18:12 -06:00
raw: check maker-note crop bounds with checked_add
A PEF or ORF maker-note crop with an origin of `u64::MAX` and a width of
1 made `x + width` wrap to 0, so the guard read it as inside the image and
`RawImage::check`'s identical comparison also wrapped: `validate()` passed
and `normalized()` then indexed past the sample buffer. Debug builds
panicked at the guard itself ("attempt to add with overflow").
The active-area check and both vendor crop guards (PEF AOC, ORF
ImageProcessing) now compare with `checked_add`, so a wrapping rectangle
is a `Corrupt` error at validate time. Exact fits still pass.
Regression test `an_active_area_that_wraps_past_the_image_is_corrupt`
builds such an image and asserts `validate()` and `normalized()` return
`Err` (fails before the fix with the overflow panic above).
This commit is contained in:
+35
-1
@@ -728,7 +728,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(())
|
||||
@@ -878,6 +881,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] };
|
||||
|
||||
Vendored
+6
-1
@@ -285,7 +285,12 @@ pub(crate) fn decode(bytes: &[u8], mode: Mode) -> Result<RawImage> {
|
||||
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));
|
||||
|
||||
Vendored
+6
-1
@@ -243,7 +243,12 @@ pub(crate) fn decode(bytes: &[u8], mode: Mode) -> Result<RawImage> {
|
||||
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)
|
||||
|
||||
Reference in New Issue
Block a user