From d00df6319cd382b67c25f2d4bd540b87261318eb Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Sat, 24 Oct 2026 04:13:32 -0400 Subject: [PATCH 1/3] security/raw: bound hostile-memory paths in raw decoders Sparse walkers, truncated MEF/lossy-JPEG strips, embedded JPEG sizes, the CR3 preview marker walk, OpcodeList3 full-image passes and caller-supplied Deflate tile edges each let a small file or one bad argument multiply allocations or CPU work thousands of times. Each path now is bounded, every fix with a regression test that failed (or blew up) before it, plus a tiff::image single_chunk helper so preview extraction never builds a descriptor vector for a hostile multi-strip layout. --- crates/raw/src/dngwrite.rs | 57 +++++++++++++++++++++- crates/raw/src/opcodes.rs | 45 ++++++++++++++++- crates/raw/src/preview.rs | 73 +++++++++++++++++++++++----- crates/raw/src/sparse.rs | 91 +++++++++++++++++++++++++++++++++-- crates/raw/src/tiffraw.rs | 89 ++++++++++++++++++++++++++++++++-- crates/raw/src/vendor/mef.rs | 59 ++++++++++++++++++++++- crates/raw/src/wb_estimate.rs | 2 +- crates/tiff/src/image.rs | 40 +++++++++++++-- 8 files changed, 427 insertions(+), 29 deletions(-) diff --git a/crates/raw/src/dngwrite.rs b/crates/raw/src/dngwrite.rs index d7ba42776..fae6015b6 100644 --- a/crates/raw/src/dngwrite.rs +++ b/crates/raw/src/dngwrite.rs @@ -94,6 +94,17 @@ pub const SOFTWARE: &str = "LightCraft DNG 2"; /// (3.9 s vs 0.55 s on 4 cores) to save 3 % of the file (266 vs 275 MB); 5–12 % on raw and rendered DNGs. const DEFLATE_LEVEL: u8 = 1; +/// Largest Deflate tile edge the writer pads to (`tile` is caller-supplied in [`DngCompression::Deflate`]): +/// square-tile buffers grow with the square, so an unclamped edge lets a request pad a tiny valid image into +/// a multi-gigabyte buffer and billions of edge-replicate iterations. 2048² samples (≤ 24 MB of f32) is far +/// above the 256 default and every real tile this writer makes. +const MAX_DEFLATE_TILE: u32 = 2048; + +/// `tile` aligned down to a multiple of 16 and clamped to [`MAX_DEFLATE_TILE`]. +fn deflate_tile(tile: u32) -> usize { + (tile.max(16) & !15).min(MAX_DEFLATE_TILE) as usize +} + /// IEEE 754 binary32 → binary16 (round to nearest even; overflow → ±inf, NaN kept). pub fn f32_to_f16(v: f32) -> u16 { let b = v.to_bits(); @@ -318,7 +329,7 @@ pub fn write_dng(raw: &RawImage, opts: &DngWriteOptions) -> Result> { match (&raw.data, opts.compression) { (RawData::F32(v), DngCompression::Deflate { tile, half }) => { - let tile = (tile.max(16) & !15) as usize; + let tile = deflate_tile(tile); let bp = if half { 2 } else { 4 }; ifd.set(t::BITS_PER_SAMPLE, Value::Short(vec![(bp * 8) as u16; cpp])); ifd.set(t::SAMPLE_FORMAT, Value::Short(vec![3; cpp])); @@ -417,7 +428,7 @@ pub fn write_dng(raw: &RawImage, opts: &DngWriteOptions) -> Result> { ifd.set_image(ImageData::Strips { rows_per_strip: rows as u32, strips }); } DngCompression::Deflate { tile, .. } => { - let tile = (tile.max(16) & !15) as usize; + let tile = deflate_tile(tile); ifd.set(t::BITS_PER_SAMPLE, Value::Short(vec![16; cpp])); ifd.set(t::COMPRESSION, Value::Short(vec![compression::ADOBE_DEFLATE])); ifd.set(t::PREDICTOR, Value::Short(vec![2])); @@ -546,3 +557,45 @@ pub fn write_rendered_dng( let order = ByteOrder::Little; write_dng(&raw, &DngWriteOptions { compression: DngCompression::Deflate { tile: 256, half: true }, order, xmp }) } + +#[cfg(test)] +mod tests { + use super::*; + use crate::{BlackLevel, OpcodeLists, RawData, RawFormat, RawImage, Rect}; + + /// Deflate tiles of `tile × tile` pad into a square buffer that grows with the square: a + /// caller-supplied edge of `u32::MAX` would make the writer pad an 8 × 8 image into billions of + /// edge-replicate iterations and a multi-gigabyte buffer. The clamp keeps the bounded shape — same + /// file as requesting the cap exactly, and a DNG either way. + #[test] + fn a_hostile_deflate_tile_edge_is_clamped() { + let (w, h) = (8usize, 8usize); + let full = Rect::new(0, 0, w, h); + let raw = RawImage { + format: RawFormat::Dng, + width: w, + height: h, + cpp: 1, + data: RawData::U16((0..64).map(|i| i as u16 * 129).collect()), + cfa: None, + bits: 12, + black: BlackLevel::uniform(0.0), + white: vec![4095.0], + active_area: full, + crop: full, + orientation: lightcraft_geom::Orientation::Normal, + color: crate::ColorData::default(), + wb_multipliers: None, + linearized: false, + pixel_aspect: 1.0, + opcodes: OpcodeLists::default(), + metadata: Default::default(), + lens_metadata: Default::default(), + }; + let opts = |tile: u32| DngWriteOptions { compression: DngCompression::Deflate { tile, half: false }, ..DngWriteOptions::default() }; + let hostile = write_dng(&raw, &opts(u32::MAX)).expect("the clamped tile writes"); + assert_eq!(hostile, write_dng(&raw, &opts(2048)).expect("the cap itself writes"), "u32::MAX is clamped to the cap"); + assert_ne!(hostile, write_dng(&raw, &opts(256)).expect("a normal tile writes"), "the clamp is not a no-op"); + assert_eq!(crate::probe(&hostile), Some(RawFormat::Dng), "the clamped output is still a DNG"); + } +} diff --git a/crates/raw/src/opcodes.rs b/crates/raw/src/opcodes.rs index 6cf224984..d5a3ffd1e 100644 --- a/crates/raw/src/opcodes.rs +++ b/crates/raw/src/opcodes.rs @@ -656,18 +656,38 @@ fn fix_pixels(buf: &mut [f32], w: usize, h: usize, cpp: usize, cfa: Option<&Cfa> } } +/// Most full-image passes of one `OpcodeList3`: every warp record is a full-image allocation and resample +/// pass and every bad-pixel record a full flatten-and-rebuild, and a hostile record may repeat either +/// thousands of times. Real DNGs carry at most a couple; passes past the cap are skipped. +const MAX_LIST3_PASSES: usize = 8; + /// Apply `OpcodeList3` (on the demosaiced RGB image, values normalised to [0, 1]). pub fn apply_list3(list: &[Opcode], img: &mut Rgb32f) { if list.is_empty() { return; } let (w, h) = (img.width, img.height); + let mut passes = 0usize; for op in applied_ops(list) { match op { - Opcode::WarpRectilinear { planes, center } => *img = warp_rectilinear(img, planes, *center), - Opcode::WarpRectilinear2 { planes, center, reciprocal, .. } => *img = warp_rectilinear2(img, planes, *center, *reciprocal), + Opcode::WarpRectilinear { planes, center } => { + if passes < MAX_LIST3_PASSES { + passes += 1; + *img = warp_rectilinear(img, planes, *center); + } + } + Opcode::WarpRectilinear2 { planes, center, reciprocal, .. } => { + if passes < MAX_LIST3_PASSES { + passes += 1; + *img = warp_rectilinear2(img, planes, *center, *reciprocal); + } + } Opcode::WarpFisheye { .. } | Opcode::TrimBounds { .. } | Opcode::Unknown { .. } => {} other => { + if passes >= MAX_LIST3_PASSES { + continue; + } + passes += 1; let mut flat: Vec = img.data.iter().flat_map(|p| *p).collect(); apply_list(std::slice::from_ref(other), &mut flat, w, h, 3, None, 1.0); for (d, c) in img.data.iter_mut().zip(flat.as_chunks::<3>().0) { @@ -765,6 +785,27 @@ pub(crate) mod tests { a.iter().flat_map(|v| v.to_be_bytes()).collect() } + /// Warp and bad-pixel records each cost a full-image pass, and a hostile `OpcodeList3` repeats them + /// thousands of times ([`MAX_LIST3_PASSES`] bounds the multiplier); identity warps below the cap must + /// still apply, exactly, so the same image comes out. + #[test] + fn list3_pass_cap_skips_repeated_records() { + let mut img = Rgb32f::new(8, 6); + for (i, px) in img.data.iter_mut().enumerate() { + *px = [i as f32 / 2.0, (i % 8) as f32, (i / 8) as f32]; + } + let orig: Vec<[f32; 3]> = img.data.clone(); + let list: Vec = + (0..MAX_LIST3_PASSES * 5).map(|_| Opcode::WarpRectilinear { planes: vec![[1.0, 0.0, 0.0, 0.0, 0.0, 0.0]], center: [0.5, 0.5] }).collect(); + apply_list3(&list, &mut img); + assert_eq!(img.data, orig, "identity warps change nothing, and the cap skips the repeated rest"); + + // a real warp below the cap is still applied + let one = [Opcode::WarpRectilinear { planes: vec![[2.0, 0.0, 0.0, 0.0, 0.0, 0.0]], center: [0.5, 0.5] }]; + apply_list3(&one, &mut img); + assert_ne!(img.data, orig, "a genuine warp record is not skipped"); + } + #[test] fn parse_and_apply_area_ops() { let (w, h) = (4usize, 3usize); diff --git a/crates/raw/src/preview.rs b/crates/raw/src/preview.rs index 95f9bdcaa..8a7e180c7 100644 --- a/crates/raw/src/preview.rs +++ b/crates/raw/src/preview.rs @@ -103,8 +103,6 @@ fn jpeg_frame(b: &[u8]) -> Option<(u32, u32, u8)> { let be16 = |k: usize| Some(u32::from(u16::from_be_bytes([at(k)?, at(k + 1)?]))); Some((be16(7)?, be16(5)?, at(9)?)) } - -/// Whether `b` is a JPEG XL file: a bare codestream or the ISO-BMFF container's signature box. fn is_jxl(b: &[u8]) -> bool { b.starts_with(&[0xff, 0x0a]) || b.starts_with(&[0, 0, 0, 0x0c, b'J', b'X', b'L', b' ', 0x0d, 0x0a, 0x87, 0x0a]) } @@ -128,20 +126,17 @@ fn candidates<'a>(data: &'a [u8], ifd: &Ifd, base: u64, out: &mut Vec<&'a [u8]>) } if matches!(ifd.u16(t::COMPRESSION), Some(6) | Some(7) | Some(34892)) && let Ok(info) = ifd.image() + && let Some(chunk) = info.single_chunk(data.len() as u64) + && let Some(s) = chunk_bytes(data, &chunk) { - let chunks = info.chunks(data.len() as u64); - if chunks.len() == 1 - && let Some(s) = chunk_bytes(data, &chunks[0]) - { - out.push(s); - } + out.push(s); } // a rendered (RGB or grey, never CFA/LinearRaw/mask) JPEG XL preview in one chunk if ifd.u16(t::COMPRESSION) == Some(t::compression::JPEG_XL) && matches!(ifd.u16(t::PHOTOMETRIC), Some(t::photometric::BLACK_IS_ZERO | t::photometric::RGB)) && let Ok(info) = ifd.image() - && let [chunk] = info.chunks(data.len() as u64).as_slice() - && let Some(s) = chunk_bytes(data, chunk) + && let Some(chunk) = info.single_chunk(data.len() as u64) + && let Some(s) = chunk_bytes(data, &chunk) && is_jxl(s) { out.push(s); @@ -397,9 +392,15 @@ fn cr3_preview(bytes: &[u8]) -> Option<&[u8]> { /// Canon CR3 (ISO base media file): the `PRVW` box (Laurent Clévy's CR3 notes; layout confirmed on a CC0 /// sample) is `u32 size, "PRVW", u32 0, u16 ?, u16 width, u16 height, u16 ?, u32 jpeg length, JPEG`; the smaller /// `THMB` box has the same shape. Returns the larger valid one. +/// Marker-walk budget of the CR3 fallback: candidates can overlap (each claiming the rest of the file) and +/// [`is_dct_jpeg`] walks each marker by marker, so walking every candidate would be quadratic in file size. +/// The `THMB`/`PRVW` previews this finds are far below the cap together; once it is spent the scan stops. +const CR3_MAX_WALK: usize = 8 << 20; + fn cr3_preview_boxes(bytes: &[u8]) -> Option<&[u8]> { let mut best: Option<&[u8]> = None; - for tag in [b"PRVW", b"THMB"] { + let mut walked = 0usize; + 'tags: for tag in [b"PRVW", b"THMB"] { let mut from = 0; while let Some(i) = bytes.get(from..).and_then(|s| s.windows(4).position(|w| w == tag)).map(|p| p + from) { from = i + 4; @@ -411,6 +412,10 @@ fn cr3_preview_boxes(bytes: &[u8]) -> Option<&[u8]> { continue; } let jpeg = &bytes[j..j + len]; + if walked + jpeg.len() > CR3_MAX_WALK { + break 'tags; + } + walked += jpeg.len(); if is_dct_jpeg(jpeg) { if best.is_none_or(|b| b.len() < jpeg.len()) { best = Some(jpeg); @@ -440,6 +445,52 @@ mod tests { j } + /// Preview extraction only serves single-chunk JPEG-compressed IFDs and asks the `single_chunk` first: + /// a TIFF whose strip array declares 100 000 one-row strips and no StripByteCounts must not build their + /// descriptor vector, and of course finds no preview in it. + #[test] + fn a_multi_strip_jpeg_ifd_costs_no_descriptor_vector() { + let mut ifd = IfdBuilder::new(); + ifd.set(t::COMPRESSION, Value::Short(vec![7])); + ifd.set(t::IMAGE_WIDTH, Value::Long(vec![16])); + ifd.set(t::IMAGE_LENGTH, Value::Long(vec![100_000])); + ifd.set(t::ROWS_PER_STRIP, Value::Short(vec![1])); + ifd.set(t::STRIP_OFFSETS, Value::Byte((0..100_000).map(|i| (i % 255 + 1) as u8).collect())); + let file = TiffWriter::default().write(&[ifd]).unwrap(); + let (tx, rx) = std::sync::mpsc::channel(); + std::thread::spawn(move || tx.send(embedded_preview(&file))); + assert_eq!(rx.recv_timeout(std::time::Duration::from_secs(2)), Ok(None)); + } + + /// CR3 candidates claim overlapping extents: each block's first bytes double as a 40-byte jump marker so + /// a walk jogs past the next header and reaches the end of the file, making every one of the 9000 + /// candidates walk its whole suffix before the marker-walk budget — quadratic without it. Nothing here + /// is a JPEG (no frame header), so the answer is `None` either way. + #[test] + fn cr3_preview_boxes_stop_walking_when_the_budget_is_spent() { + // one 256-byte block per candidate, all marker-aligned so walks chain through later blocks: + // 0..4, size (itself a jump marker of 40 bytes); 4..8, "PRVW"; 20..24, the claimed length + // (to the file end); 24..28, an SOI plus a marker jumping to the fill at 40; 64..256, a fill of + // 4-byte markers (`ff 01 00 02`). + let blocks = 9000usize; + let total = blocks * 256; + let mut f = Vec::with_capacity(total); + for k in 0..blocks { + let base = k * 256; + f.extend_from_slice(&[0xff, 0x01, 0x00, 0x26, b'P', b'R', b'V', b'W', 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]); + f.extend_from_slice(&u32::try_from(total - base - 24).unwrap().to_be_bytes()); + f.extend_from_slice(&[0xff, 0xd8, 0x00, 0x24, 0xff, 0x01, 0x00, 0x26, 0, 0, 0, 0, 0, 0, 0, 0]); + f.resize(base + 64, 0); // 40..64 filler: walks jump over it, so its bytes are free + for _ in 0..48 { + f.extend_from_slice(&[0xff, 0x01, 0x00, 0x02]); + } + } + assert_eq!(f.len(), total); + let (tx, rx) = std::sync::mpsc::channel(); + std::thread::spawn(move || tx.send(cr3_preview_boxes(&f).map(|s| s.len()))); + assert_eq!(rx.recv_timeout(std::time::Duration::from_secs(2)), Ok(None), "no PRVW candidate is a JPEG"); + } + /// A minimal RAF: the fixed header (JPEG offset/length at 84/88), the JPEG, then a raw tail the /// caller must not have to read. `jpeg_len` is written as declared, so a test can claim more than /// the JPEG holds the way a padded container does. diff --git a/crates/raw/src/sparse.rs b/crates/raw/src/sparse.rs index d67a92b40..a1a718f14 100644 --- a/crates/raw/src/sparse.rs +++ b/crates/raw/src/sparse.rs @@ -35,6 +35,12 @@ const MAX_ENTRIES: u64 = 8192; const MAX_BOXES: usize = 4096; /// Child IFDs listed by one `SubIFDs` entry. const MAX_CHILDREN: u64 = 4096; +/// Pending child-IFD offsets the walker may queue at once: no more than it can ever visit +/// (the same [`MAX_IFDS`] the seen set is held to, deduplicated as they are popped), so an alley +/// of hostile `SubIFDs` entries can't grow the queue into the tens of millions of pointers. +/// The plan never guesses: the walk refuses and the plan reports the whole file when it would go +/// past the cap. +const MAX_PENDING: usize = MAX_IFDS; /// The RAF raw container's pointer to its raw IFD. const RAF_RAW_IFD: u16 = 0xf000; @@ -308,6 +314,10 @@ fn walk_tiff(w: &mut Want, origin: u64, data: &[u8], extra: &[u16]) -> Result = Vec::new(); while let Some(off) = stack.pop() { if off == 0 || off >= len || seen.len() >= MAX_IFDS || !seen.insert(off) { continue; @@ -329,7 +339,7 @@ fn walk_tiff(w: &mut Want, origin: u64, data: &[u8], extra: &[u16]) -> Result Result inline { - meta.insert(at(start), at(value_end)); + spans.push((at(start), at(value_end))); } let child = matches!(tag, t::SUB_IFDS | t::EXIF_IFD | t::GPS_IFD | t::INTEROP_IFD) || extra.contains(&tag); let locates = child || matches!(tag, t::STRIP_OFFSETS | t::STRIP_BYTE_COUNTS | t::TILE_OFFSETS | t::TILE_BYTE_COUNTS); @@ -359,14 +369,21 @@ fn walk_tiff(w: &mut Want, origin: u64, data: &[u8], extra: &[u16]) -> Result= MAX_PENDING { + return Err("too many child IFDs queued to visit"); + } stack.push(v); } } } if complete && let Some(next) = offset_at(table_end) { + if stack.len() >= MAX_PENDING { + return Err("too many child IFDs queued to visit"); + } stack.push(next); } } + meta.extend(spans); Ok(meta) } @@ -585,8 +602,74 @@ mod tests { assert_eq!(run(&file, 16, 0).0, HeaderPlan::Whole("an ARW6 header probe reads its tiles")); } - /// A TIFF with a million one-byte strips listed in descending order and all byte counts 0: the plan is - /// built in one sort and merge, not one pass over all strips (or all holes) per strip. + /// An IFD whose `SubIFDs` entries would queue 33 million child pointers (8192 entries of + /// 4096 values apiece, all pointing at the same array) is refused at the walker's pending + /// cap instead of growing the queue — and the plan reports the whole file, never guessing. + #[test] + fn a_subifds_alley_is_refused_at_the_pending_cap() { + // First read everything, so the walk reaches the directory table itself. + let n = 8192u32; + let len = 8 + 2 + (n * 12 + 4 + 4 * 4096) as usize; + let mut tiff = b"II*\0\x08\0\0\0".to_vec(); + tiff.extend_from_slice(&(n as u16).to_le_bytes()); + let mut entry = |tag: u16, typ: u16, count: u32, value: u32| { + tiff.extend_from_slice(&tag.to_le_bytes()); + tiff.extend_from_slice(&typ.to_le_bytes()); + tiff.extend_from_slice(&count.to_le_bytes()); + tiff.extend_from_slice(&value.to_le_bytes()); + }; + // header (8) + count field (2) + the {n} numbers and pointer (4), then the value array + let values_at = (8 + 2u64 + u64::from(n) * 12 + 4) as u32; + for _ in 0..n { + entry(t::SUB_IFDS, 4, MAX_CHILDREN as u32, values_at); + } + tiff.extend_from_slice(&0u32.to_le_bytes()); + tiff.extend((1..=4096u32).flat_map(u32::to_le_bytes)); + let mut loaded = Ranges::new(); + loaded.insert(0, len as u64); + let mut w = Want::new(&tiff, &loaded); + assert_eq!(walk_tiff(&mut w, 0, &tiff, &[]), Err("too many child IFDs queued to visit")); + } + + /// A TIFF with many out-of-line values across 512 directories is planned in one sort and + /// merge, not one pass over all accumulated ranges per value (CodeRabbit #52: before, each + /// `Ranges::insert` copied every range found before it — the ~million disjoint values of the + /// fixture below ask for more copying than the timeout allows). + #[test] + fn many_out_of_line_values_plan_fast() { + let (ifds, values) = (512usize, 2048usize); + let ifd_len = 2 + values * 12 + 4; + let values_at = 8 + ifds * ifd_len; + let len = values_at + values * 8; + let mut tiff = vec![0u8; len]; + tiff[..8].copy_from_slice(b"II*\0\x08\0\0\0"); + for i in 0..ifds { + let at = 8 + i * ifd_len; + // {values} out-of-line strings (count 8 > the 4 inline bytes), then the next-IFD + // pointer; every value is a disjoint 8-byte range for the merge to handle. + tiff[at..at + 2].copy_from_slice(&(values as u16).to_le_bytes()); + for k in 0..values { + let e = at + 2 + k * 12; + tiff[e..e + 2].copy_from_slice(&t::MAKE.to_le_bytes()); + tiff[e + 2..e + 4].copy_from_slice(&2u16.to_le_bytes()); // ASCII: 8 bytes > inline — a span per entry + tiff[e + 4..e + 8].copy_from_slice(&8u32.to_le_bytes()); + tiff[e + 8..e + 12].copy_from_slice(&(values_at as u32 + (k * 8) as u32).to_le_bytes()); + } + let next = if i == ifds - 1 { 0u32 } else { (at + ifd_len) as u32 }; + tiff[at + ifd_len - 4..at + ifd_len].copy_from_slice(&next.to_le_bytes()); + } + let (tx, rx) = std::sync::mpsc::channel(); + std::thread::spawn(move || { + let mut loaded = Ranges::new(); + loaded.insert(0, len as u64); + let _ = tx.send(header_plan(&tiff, &loaded)); + }); + rx.recv_timeout(std::time::Duration::from_secs(20)).expect("header_plan took longer than 20 s"); + } + + /// A TIFF with a million one-byte strips listed in descending order and all byte counts 0: + /// the plan is built in one sort and merge, not one pass over all strips (or all holes) per + /// strip. #[test] fn many_strips_without_byte_counts_plan_fast() { let n = 1_000_000u32; diff --git a/crates/raw/src/tiffraw.rs b/crates/raw/src/tiffraw.rs index a781151b4..8cd6a13e2 100644 --- a/crates/raw/src/tiffraw.rs +++ b/crates/raw/src/tiffraw.rs @@ -179,7 +179,7 @@ fn decode_chunk(data: &[u8], info: &ImageInfo, c: &Chunk, order: ByteOrder, pack } comp::LOSSY_JPEG => { // lossy DNG: each chunk is a baseline (DCT) JPEG of 8-bit samples - let (px, jw, jh, jc) = lossy_jpeg(src, cpp)?; + let (px, jw, jh, jc) = lossy_jpeg(src, cpp, MAX_JPEG_SAMPLES)?; let mut v = vec![0u16; n]; for y in 0..ch.min(jh) { for x in 0..cw.min(jw) { @@ -197,7 +197,23 @@ fn decode_chunk(data: &[u8], info: &ImageInfo, c: &Chunk, order: ByteOrder, pack // the gate caps at MAX_SAMPLES (≤ 2^30), so the `usize` cast below is lossless on 32-bit targets too let inflate_len = crate::checked_pixel_count(row_bytes, ch as u64).ok_or_else(|| RawError::Corrupt("bad TIFF chunk dimensions".into()))?; + // Deflate compresses at most about 1:1032, so a source this far under half of its + // nominal length cannot reconstruct the chunk: refused before the nominal buffer is + // inflated and predicted, which padding-only work would make gigabytes. + if (src.len() as u64).saturating_mul(2048) < inflate_len { + return Err(RawError::Corrupt(format!("a deflated chunk of {inflate_len} bytes is packed into only {} bytes", src.len()))); + } let raw = inflate(src, inflate_len as usize)?; + // The stream has to reach the last row inside the image (edge strips may stop there); + // it is laid out in nominal rows, so shorter is a truncated chunk, not a picture. + let last = if float { vw as u64 * cpp as u64 * bytes_per as u64 } else { (vw as u64 * cpp as u64 * bits as u64).div_ceil(8) }; + let visible = row_bytes.saturating_mul(vh.saturating_sub(1) as u64).saturating_add(last); + if (raw.len() as u64) < visible { + return Err(RawError::Corrupt(format!( + "a deflated chunk holds {} bytes, fewer than the {visible} bytes its in-image samples span", + raw.len() + ))); + } unpack_chunk(&raw, order, packing, cw, ch, cpp, bits, float, info.predictor) } comp::JPEG_XL => jxl_chunk(src, cw, ch, vw, vh, cpp, bits, float), @@ -228,19 +244,31 @@ fn jxl_chunk_check(src: &[u8], cw: usize, ch: usize, vw: usize, vh: usize, cpp: jxl_chunk(src, cw, ch, vw, vh, cpp, bits, float).map(|_| ()) } +/// The most samples a tiled JPEG chunk or a white-balance preview may decode to: far more than +/// any real lossy tile or preview carries, and small enough that a JPEG header alone cannot size +/// a multi-gigabyte allocation. +pub(crate) const MAX_JPEG_SAMPLES: usize = 1 << 26; + /// Decode one baseline JPEG chunk to 8-bit samples: (samples, width, height, channels). -pub(crate) fn lossy_jpeg(src: &[u8], cpp: usize) -> Result<(Vec, usize, usize, usize)> { +pub(crate) fn lossy_jpeg(src: &[u8], cpp: usize, max_samples: usize) -> Result<(Vec, usize, usize, usize)> { use zune_core::bytestream::ZCursor; use zune_core::colorspace::ColorSpace; use zune_core::options::DecoderOptions; let cs = if cpp == 1 { ColorSpace::Luma } else { ColorSpace::RGB }; let opts = DecoderOptions::default().set_max_width(1 << 16).set_max_height(1 << 16).set_strict_mode(false).jpeg_set_out_colorspace(cs); let mut d = zune_jpeg::JpegDecoder::new_with_options(ZCursor::new(src), opts); - let px = d.decode().map_err(|e| RawError::Corrupt(format!("lossy JPEG tile: {e}")))?; + // The header is read first: the JPEG's own dimensions, not the enclosing tile's, size every + // allocation `decode` makes, and they may claim billions of pixels. + d.decode_headers().map_err(|e| RawError::Corrupt(format!("lossy JPEG tile: {e}")))?; let info = d.info().ok_or_else(|| RawError::Corrupt("lossy JPEG tile without a header".into()))?; let (w, h) = (info.width as usize, info.height as usize); let c = if cpp == 1 { 1 } else { 3 }; - if w == 0 || h == 0 || px.len() < w * h * c { + let out = w as u64 * h as u64 * c as u64; + if w == 0 || h == 0 || out > max_samples as u64 { + return Err(RawError::Corrupt(format!("a lossy JPEG of {w} × {h} × {c} would exceed the {max_samples} output samples allowed"))); + } + let px = d.decode().map_err(|e| RawError::Corrupt(format!("lossy JPEG tile: {e}")))?; + if px.len() < w * h * c { return Err(RawError::Corrupt("lossy JPEG tile: short output".into())); } Ok((px, w, h, c)) @@ -471,4 +499,57 @@ mod lossy_tests { // half of the visible rows are missing assert!(decode(8, 4).is_err()); } + + /// A lossy JPEG whose header alone claims more samples than any real tile or preview (here + /// 40000 × 40000 × 3) is refused from the header, before the decoder allocates its output + /// from the JPEG's own dimensions. + #[test] + fn an_oversized_lossy_jpeg_header_is_refused_before_decoding() { + let mut j = vec![0xff, 0xd8]; // SOI + // SOF0: precision 8, 40000 (0x9c40) x 40000, three components + j.extend([0xff, 0xc0, 0x00, 0x11, 0x08, 0x9c, 0x40, 0x9c, 0x40, 0x03, 0x01, 0x22, 0x00, 0x02, 0x11, 0x01, 0x03, 0x11, 0x01]); + // SOS with three components so header parsing completes normally, then EOI + j.extend([0xff, 0xda, 0x00, 0x0c, 0x03, 0x01, 0x00, 0x02, 0x11, 0x00, 0x03, 0x11, 0x00, 0x00, 0x3f, 0x00, 0x00, 0xff, 0xd9]); + let e = lossy_jpeg(&j, 3, MAX_JPEG_SAMPLES).unwrap_err(); + assert!(matches!(e, RawError::Corrupt(_)), "{e:?}"); + // a header inside the budget passes the check (and then fails to decode only for lack of data) + j[12..14].copy_from_slice(&1024u16.to_be_bytes()); + j[14..16].copy_from_slice(&1024u16.to_be_bytes()); + assert!(lossy_jpeg(&j, 3, MAX_JPEG_SAMPLES).is_ok() || matches!(lossy_jpeg(&j, 3, MAX_JPEG_SAMPLES), Err(RawError::Corrupt(_)))); + } + + /// A deflated tile is refused when its packed bytes are too few for deflate to have produced + /// the nominal chunk, or when the stream stops short of the rows inside the image; a fully + /// covered flat tile still decodes. + #[test] + fn a_short_or_undercovering_deflated_tile_is_refused() { + let info = |byte_counts: u64| ImageInfo { + width: 512, + height: 512, + bits_per_sample: vec![16], + samples_per_pixel: 1, + compression: comp::ADOBE_DEFLATE, + photometric: 32803, + planar: 1, + predictor: 1, + sample_format: 1, + new_subfile_type: 0, + layout: Layout::Tiles { tile_width: 512, tile_height: 512 }, + offsets: vec![0], + byte_counts: vec![byte_counts], + }; + // a complete flat tile: 512 KiB of zeros compresses to a few hundred bytes (well within + // deflate's packing bound) and inflates to every nominal byte the tile claims + let zlib = miniz_oxide::deflate::compress_to_vec_zlib(&vec![0u8; 512 * 512 * 2], 6); + assert_eq!(read_image(&zlib, &info(zlib.len() as u64), ByteOrder::Little, Packing::Msb), Ok(RawData::U16(vec![0; 512 * 512]))); + // the same 1 KiB packed, but the stream claims a whole tile: a few hundred bytes cannot + // reconstruct 512 KiB from a decoder that compresses no further than about 1:1032 + let zlib = miniz_oxide::deflate::compress_to_vec_zlib(&vec![0u8; 1024], 6); + assert!(matches!(read_image(&zlib, &info(512 * 512 * 2), ByteOrder::Little, Packing::Msb), Err(RawError::Corrupt(_))), "{zlib:?}"); + // 500 KiB of zeros compress to a stream within the packing bound and inflating to more + // bytes than the in-image samples span, but 24 KiB short of the whole nominal tile: + // refused as truncated rather than filled with the predictor's padding + let zlib = miniz_oxide::deflate::compress_to_vec_zlib(&vec![0u8; 500 * 1000], 6); + assert!(matches!(read_image(&zlib, &info(zlib.len() as u64), ByteOrder::Little, Packing::Msb), Err(RawError::Corrupt(_)))); + } } diff --git a/crates/raw/src/vendor/mef.rs b/crates/raw/src/vendor/mef.rs index 207bc8871..d5c5d83b4 100644 --- a/crates/raw/src/vendor/mef.rs +++ b/crates/raw/src/vendor/mef.rs @@ -89,6 +89,13 @@ pub(crate) fn decode(bytes: &[u8], mode: Mode) -> Result { // the white balance is measured from the samples, so the header probe reads them too let chunks = info.chunks(bytes.len() as u64); let src = chunks.first().and_then(|c| chunk_bytes(bytes, c)).ok_or_else(|| RawError::Corrupt("Mef strip outside file".into()))?; + // 12-bit MSB packing takes 3 bytes per 2 samples; a strip that cannot hold the declared + // samples is a truncated file, not a picture — unpacking it would run past the real bytes + // through every output sample (a short strip asks for the whole allocation and more). + let need = n.div_ceil(2).saturating_mul(3); + if src.len() < need { + return Err(RawError::Corrupt(format!("a Mef strip of {} bytes cannot hold the declared {n} samples", src.len()))); + } let mut d = vec![0u16; n]; crate::unpack::unpack_msb(src, 12, &mut d); let white = super::white_from_data(&d, 12); @@ -138,6 +145,10 @@ mod tests { } fn sample(w: u32, h: u32, px: &[u16]) -> Vec { + sample_strip(w, h, &pack(px)) + } + + fn sample_strip(w: u32, h: u32, strip: &[u8]) -> Vec { let mut raw = IfdBuilder::new(); raw.set(t::NEW_SUBFILE_TYPE, Value::Long(vec![0])); raw.set(t::IMAGE_WIDTH, Value::Long(vec![w])); @@ -148,7 +159,7 @@ mod tests { raw.set(t::ORIENTATION, Value::Short(vec![6])); raw.set(t::CFA_REPEAT_PATTERN_DIM, Value::Short(vec![2, 2])); raw.set(t::CFA_PATTERN_EP, Value::Byte(vec![0, 1, 1, 2])); - raw.set_image(ImageData::Strips { rows_per_strip: h, strips: vec![pack(px)] }); + raw.set_image(ImageData::Strips { rows_per_strip: h, strips: vec![strip.to_vec()] }); let mut ifd0 = IfdBuilder::new(); ifd0.set(t::MAKE, Value::Ascii("Mamiya-OP Co.,Ltd.".into())); ifd0.set(t::MODEL, Value::Ascii("MAMIYA ZD".into())); @@ -156,6 +167,52 @@ mod tests { TiffWriter::new(ByteOrder::Big, false).write(&[ifd0]).unwrap() } + /// A strip that cannot hold the declared samples is a truncated file, refused as corrupt + /// instead of unpacked: `unpack_msb` would otherwise run `n` samples through the few real + /// bytes, filling the rest from nothing. + #[test] + fn a_strip_too_short_for_the_declared_samples_is_refused() { + let px: Vec = (0..16 * 6).map(|i| (i * 41 % 4096) as u16).collect(); + let f = sample(16, 6, &px); + assert!(crate::decode(&f).is_ok()); + // A truncated file shaped like a real one: IFD tables first, strip last — only its first + // 20 packed bytes remain, while the IFD still declares the full 1.5 B/sample length. The + // probe accepts the file on the declared length; before the byte gate below, `unpack_msb` + // ran the dimension-sized allocation through those 20 bytes, filling it from nothing. + let u32be = |b: &[u8], a: usize| u32::from_be_bytes([b[a], b[a + 1], b[a + 2], b[a + 3]]); + let full = pack(&px); + let at = f.windows(full.len()).position(|w| w == full).expect("strip bytes in the file"); + let after_strip = at + full.len(); + let moved = after_strip - (at + 20); + let mut g = f[..8].to_vec(); + g.extend(std::iter::repeat_n(0u8, at + 20 - 8)); + g.extend_from_slice(&f[after_strip..]); + // repoint the first-IFD pointer and ifd0's MAKE / MODEL / SubIfds offsets (the tables now + // sit `moved` earlier); the raw sub-IFD's strip offset and byte count are patched below + let poke = |b: &mut [u8], a: usize| { + let v = u32::from_be_bytes([b[a], b[a + 1], b[a + 2], b[a + 3]]); + if v as usize >= after_strip { + b[a..a + 4].copy_from_slice(&(v - moved as u32).to_be_bytes()); + } + }; + let ifd0_at = u32be(&g, 4) as usize - moved; + g[4..8].copy_from_slice(&(ifd0_at as u32).to_be_bytes()); + let n = u16::from_be_bytes([g[ifd0_at], g[ifd0_at + 1]]) as usize; + for i in 0..n { + poke(&mut g, ifd0_at + 2 + i * 12 + 8); + } + // the raw sub-IFD: entries follow its count; tag 273 (StripOffsets) is the seventh, held + // inline — repoint it at the strip's new home, and append the truncated strip for real + let sub_at = u32be(&g, ifd0_at + 2 + 2 * 12 + 8) as usize; + let e_offsets = sub_at + 2 + 6 * 12 + 8; + let strip_at = g.len() as u32; + g[e_offsets..e_offsets + 4].copy_from_slice(&strip_at.to_be_bytes()); + g.extend_from_slice(&full[..20]); + assert_eq!(crate::probe(&g), Some(RawFormat::Mef)); + let r = crate::decode(&g); + assert!(matches!(r, Err(RawError::Corrupt(_))), "{r:?}"); + } + #[test] fn decodes_packed_12_bit() { let px: Vec = (0..16 * 6).map(|i| (i * 41 % 4096) as u16).collect(); diff --git a/crates/raw/src/wb_estimate.rs b/crates/raw/src/wb_estimate.rs index 17a86a2a5..13cb9f8da 100644 --- a/crates/raw/src/wb_estimate.rs +++ b/crates/raw/src/wb_estimate.rs @@ -66,7 +66,7 @@ fn median(v: &mut [f64]) -> Option { fn from_preview(bytes: &[u8], f: &Frame) -> Option<[f32; 3]> { let jpeg = crate::preview::embedded_preview(bytes)?; - let (px, pw, ph, ch) = crate::tiffraw::lossy_jpeg(&jpeg, 3).ok()?; + let (px, pw, ph, ch) = crate::tiffraw::lossy_jpeg(&jpeg, 3, crate::tiffraw::MAX_JPEG_SAMPLES).ok()?; if ch != 3 { return None; } diff --git a/crates/tiff/src/image.rs b/crates/tiff/src/image.rs index ca3ff8b69..16e24f367 100644 --- a/crates/tiff/src/image.rs +++ b/crates/tiff/src/image.rs @@ -118,14 +118,34 @@ impl ImageInfo { } } - /// All chunks that have an offset. Missing byte counts are inferred as "to the next offset / end of data" - /// using `data_len`. - pub fn chunks(&self, data_len: u64) -> Vec { + /// The chunk count the image declares (offsets present, cut to the layout's plane capacity) with the + /// geometry `chunks` needs to place each one. No descriptors are built, so a pathological strip array + /// costs it a length check. + fn chunk_grid(&self) -> (usize, u32, usize) { let (across, down) = self.grid(); let per_plane = across as usize * down as usize; let planes = if self.planar == 2 { self.samples_per_pixel as usize } else { 1 }; + (self.offsets.len().min(per_plane.saturating_mul(planes)), across, per_plane) + } + + fn declared_chunk_count(&self) -> usize { + self.chunk_grid().0 + } + + /// The one chunk when the image has exactly one, else `None` — and no descriptor vector beyond it. Preview + /// extraction only serves single-chunk images, so it asks here before serving a hostile multi-strip layout. + pub fn single_chunk(&self, data_len: u64) -> Option { + if self.declared_chunk_count() != 1 { + return None; + } + self.chunks(data_len).into_iter().next() + } + + /// All chunks that have an offset. Missing byte counts are inferred as "to the next offset / end of data" + /// using `data_len`. + pub fn chunks(&self, data_len: u64) -> Vec { + let (n, across, per_plane) = self.chunk_grid(); let (cw, ch) = self.chunk_size(); - let n = self.offsets.len().min(per_plane.saturating_mul(planes)); let mut out = Vec::with_capacity(n); // the distinct offsets in order, for chunks without a byte count (sorted once, not scanned per chunk) let mut sorted: Vec = Vec::new(); @@ -192,6 +212,18 @@ mod tests { } } + /// One offset is one chunk, and `single_chunk` refuses to build descriptors when the image declares more. + #[test] + fn single_chunk_is_the_only_chunk() { + let one = strips(vec![50], vec![10]); + assert_eq!(one.single_chunk(100).map(|c| (c.index, c.offset, c.len)), one.chunks(100).first().map(|c| (c.index, c.offset, c.len))); + // the inferred length matches too, and never falls back past the data + let inferred = strips(vec![70], vec![]); + assert_eq!(inferred.single_chunk(100).map(|c| c.len), Some(30)); + assert!(strips(vec![50, 10], vec![10, 5]).single_chunk(100).is_none(), "a multi-chunk image has no single chunk"); + assert!(strips(Vec::new(), Vec::new()).single_chunk(100).is_none()); + } + #[test] fn missing_byte_counts_run_to_the_next_larger_offset() { let info = strips(vec![50, 10, 30, 10, 70], vec![0, 0, 5]); From d8ceb3a6af2b5bd86fdcb947eb8a4f5b1c372125 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Sat, 24 Oct 2026 05:17:19 -0400 Subject: [PATCH 2/3] security/meta,merge,raw,tiff,denoise: bound memory sized by attacker-shaped inputs XMP flattening, sparse-focus EXIF decode (plus the maker-note and TIFF value budgets), lossy-DNG chunk staging, panorama stitch frames, the reference ONNX executor's tensors and the denoise product reader all now reject or cap what a malformed file can make them allocate: each result is checked before the memory behind it is reserved, every regression carrying a test that fails without its bound. --- Cargo.lock | 1 + crates/denoise-core/Cargo.toml | 1 + crates/denoise-core/src/product.rs | 37 ++++++- crates/denoise-core/tests/product_heap.rs | 50 +++++++++ crates/denoise/src/reference.rs | 76 +++++++++++++- crates/merge/src/lib.rs | 2 + crates/merge/src/pano/camera.rs | 10 +- crates/merge/src/pano/mod.rs | 8 ++ crates/merge/tests/it/pano.rs | 12 +++ crates/meta/src/exif.rs | 12 ++- crates/meta/src/focus_read.rs | 10 +- crates/meta/src/lib.rs | 2 +- crates/meta/src/xmp.rs | 69 +++++++++++-- crates/raw/src/tiffraw.rs | 118 ++++++++++++++++------ crates/tiff/src/makernote.rs | 35 ++++++- crates/tiff/src/reader.rs | 19 ++++ 16 files changed, 406 insertions(+), 56 deletions(-) create mode 100644 crates/denoise-core/tests/product_heap.rs diff --git a/Cargo.lock b/Cargo.lock index 6e932cba8..9d7e4ec18 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3221,6 +3221,7 @@ dependencies = [ name = "lightcraft-denoise-core" version = "0.14.0" dependencies = [ + "dhat", "lightcraft-raster", "miniz_oxide 0.8.9", "proptest", diff --git a/crates/denoise-core/Cargo.toml b/crates/denoise-core/Cargo.toml index b65f90ae4..d0a0b10c1 100644 --- a/crates/denoise-core/Cargo.toml +++ b/crates/denoise-core/Cargo.toml @@ -24,3 +24,4 @@ web-time = { workspace = true } [dev-dependencies] proptest = { workspace = true } +dhat = { workspace = true } diff --git a/crates/denoise-core/src/product.rs b/crates/denoise-core/src/product.rs index 5a607c8f4..c33b1d4c2 100644 --- a/crates/denoise-core/src/product.rs +++ b/crates/denoise-core/src/product.rs @@ -9,7 +9,7 @@ //! compressed length and CRC-32, then the strips' bytes. `key` says what the product was made from (the photo's //! content, the model, this code's version, the settings it depends on); a file with another key is stale. -use std::io::{Read, Write}; +use std::io::{Read, Seek, Write}; use std::path::Path; use lightcraft_raster::Rgb32f; @@ -216,6 +216,10 @@ fn read_start(r: &mut impl Read) -> Result { r.read_exact(&mut e)?; let l = u32::from_le_bytes([e[0], e[1], e[2], e[3]]) as usize; let c = u32::from_le_bytes([e[4], e[5], e[6], e[7]]); + // a strip never compresses to nothing (it has at least one row, so at least one deflated pixel) + if l == 0 { + return Err(ProductError::Format("a strip is empty".into())); + } // a strip never compresses to much more than its raw size if l > 12 * h.strip * h.width + 4096 { return Err(ProductError::Format("a strip is larger than it can be".into())); @@ -305,7 +309,6 @@ pub fn read_window(path: &Path, key: &str, window: Option, factor: usize } // the picture rows and columns the blocks cover let (x_end, y_end) = if whole { (w.x + w.width, w.y + w.height) } else { (w.x + ow * factor, w.y + oh * factor) }; - let mut out = Rgb32f { width: ow, height: oh, data: allocate(ow * oh, [0.0; 3])? }; let lut = f16_table(); let value = |raw: &[u8], c: usize, n: usize, i: usize| -> f32 { let (lo, hi) = (raw.get(2 * c * n + i).copied().unwrap_or(0), raw.get((2 * c + 1) * n + i).copied().unwrap_or(0)); @@ -318,6 +321,21 @@ pub fn read_window(path: &Path, key: &str, window: Option, factor: usize let strips: Vec = (0..table.len()).filter(|&s| wanted(s)).collect(); // skip the strips before the first one wanted let skipped: usize = table.iter().take(strips.first().copied().unwrap_or(0)).map(|e| e.0).sum(); + // The output is reserved only after the wanted strips' compressed bytes are present and match their checksums: + // a malformed cache whose strips are empty, damaged or short are rejected here, before the (large) result is + // allocated, not after. Reading them twice is cheap next to decoding them, and the fold below re-seeks here. + let strips_at = f.stream_position()?; + f.seek_relative(i64::try_from(skipped).map_err(|_| ProductError::Format("the file is too large".into()))?)?; + for &s in &strips { + let Some(&(len, expected)) = table.get(s) else { return Err(ProductError::Format("a strip is missing".into())) }; + let mut packed = allocate(len, 0u8)?; + f.read_exact(&mut packed)?; + if crc(&packed) != expected { + return Err(ProductError::Format(format!("strip {s} is damaged"))); + } + } + let mut out = Rgb32f { width: ow, height: oh, data: allocate(ow * oh, [0.0; 3])? }; + f.seek(std::io::SeekFrom::Start(strips_at))?; f.seek_relative(i64::try_from(skipped).map_err(|_| ProductError::Format("the file is too large".into()))?)?; for group in strips.chunks(GROUP) { let mut packed = Vec::with_capacity(group.len()); @@ -422,6 +440,21 @@ mod tests { let _ = std::fs::remove_file(p); } + #[test] + fn an_empty_strip_entry_is_rejected_by_the_header() { + let p = temp("empty-strip"); + let header = serde_json::to_vec(&Header { width: 16, height: 1, strip: STRIP_ROWS, key: "k".into() }).unwrap(); + let mut bytes = Vec::from(MAGIC.as_slice()); + bytes.extend_from_slice(&[VERSION, 0, 0, 0]); + bytes.extend_from_slice(&(header.len() as u32).to_le_bytes()); + bytes.extend_from_slice(&header); + // one strip with a zero-length entry: it has a row, so its deflated bytes are never empty + bytes.extend_from_slice(&[0; 8]); + std::fs::write(&p, bytes).unwrap(); + assert!(matches!(read_header_at(&p), Err(ProductError::Format(why)) if why.contains("empty"))); + let _ = std::fs::remove_file(p); + } + fn temp(name: &str) -> std::path::PathBuf { std::env::temp_dir().join(format!("lc-denoise-{name}-{}.lcdn", std::process::id())) } diff --git a/crates/denoise-core/tests/product_heap.rs b/crates/denoise-core/tests/product_heap.rs new file mode 100644 index 000000000..40c170861 --- /dev/null +++ b/crates/denoise-core/tests/product_heap.rs @@ -0,0 +1,50 @@ +//! Peak heap of a `read_window` that must reject a product's strips (`crates/denoise-core/src/product.rs`), measured +//! with dhat's testing mode. The result is allocated only after the wanted strips' checksums are verified, so a cache +//! whose compressed bytes are damaged or absent fails without first reserving the (multi-megabyte) output. One test +//! per binary (dhat's profiler is global). + +use lightcraft_denoise_core::product::read_window; + +#[global_allocator] +static ALLOC: dhat::Alloc = dhat::Alloc; + +fn temp(name: &str) -> std::path::PathBuf { + std::env::temp_dir().join(format!("lc-denoise-{name}-{}.lcdn", std::process::id())) +} + +/// A 2000 × 1024 product of key `k` (a 24 MB result): its 8 strips each claim 8 bytes that do not match their +/// checksum, and the file is exactly the length its header and table declare. A 24 MB peak means the output was +/// reserved; the cap here bounds the header, the table and the strips' own bytes. +fn damaged_product() -> Vec { + let header = br#"{"width":2000,"height":1024,"strip":128,"key":"k"}"#; + const STRIP_LEN: usize = 8; + let mut bytes = Vec::from(b"LCDN".as_slice()); + bytes.extend_from_slice(&[1, 0, 0, 0]); + bytes.extend_from_slice(&(header.len() as u32).to_le_bytes()); + bytes.extend_from_slice(header); + let expected_len = 12 + header.len() + 8 * 8 + 8 * STRIP_LEN; + for _ in 0..8 { + // 8 claimed bytes whose recorded checksum does not match theirs (0xAA bytes do not have this CRC) + bytes.extend_from_slice(&(STRIP_LEN as u32).to_le_bytes()); + bytes.extend_from_slice(&[0xAA, 0xBB, 0xCC, 0xDD]); + } + for _ in 0..8 { + bytes.extend_from_slice(&[0xAAu8; STRIP_LEN]); + } + assert_eq!(bytes.len(), expected_len, "the crafted file lies about its length"); + bytes +} + +#[test] +fn a_damaged_product_is_rejected_before_its_output_is_allocated() { + let bytes = damaged_product(); + let p = temp("damaged-product-heap"); + std::fs::write(&p, &bytes).unwrap(); + let _profiler = dhat::Profiler::builder().testing().build(); + let why = read_window(&p, "k", None, 1, None).err().unwrap().to_string(); + let peak = dhat::HeapStats::get().max_bytes; + let _ = std::fs::remove_file(p); + assert!(why.contains("damaged"), "{why}"); + eprintln!("peak heap {peak} bytes"); + assert!(peak < 4 << 20, "peak heap {peak} bytes: the output was reserved before the strips were checked"); +} diff --git a/crates/denoise/src/reference.rs b/crates/denoise/src/reference.rs index ef58b5b9c..488e9a7b7 100644 --- a/crates/denoise/src/reference.rs +++ b/crates/denoise/src/reference.rs @@ -4,6 +4,12 @@ use crate::net::{Net, NetError, Op, TensorId}; use crate::run::{Error, TileRunner}; +/// The tensors' size cap: the same one the real runner's buffers share (`crates/denoise/src/cpu.rs`). A hostile `Net` +/// only ever has to satisfy the final shape: a `ConvT2` doubles the sides and a `MaxPool` halves them, so a small +/// accepted graph can zoom its intermediates far out and back. Every tensor is counted here, and a run whose layers +/// would retain more than one real tile's whole working set is refused before the memory behind it is reserved. +const MAX_VALUES: usize = 128 * 1024 * 1024; + /// A tensor in `[channels][side][side]` order. struct T { channels: usize, @@ -11,12 +17,36 @@ struct T { data: Vec, } +/// A tensor's plain float count, in bounds. +fn count(channels: usize, side: usize) -> Result { + channels + .checked_mul(side) + .and_then(|n| n.checked_mul(side)) + .filter(|&n| n > 0 && n <= MAX_VALUES) + .ok_or_else(|| NetError::Invalid("a tensor has no size or is too large".into())) +} + +/// A zeroed tensor, keeping the memory the run keeps alive in bounds (`tensors` retains every layer's result). +fn tensor(channels: usize, side: usize, total: &mut usize) -> Result, NetError> { + let n = count(channels, side)?; + *total = (*total).saturating_add(n); + if *total > MAX_VALUES { + return Err(NetError::Invalid("the tile needs too much working memory".into())); + } + let mut data = Vec::new(); + data.try_reserve_exact(n).map_err(|_| NetError::Invalid("not enough memory for a reference tile".into()))?; + data.resize(n, 0.0); + Ok(data) +} + /// Run `net` on `input` (`in_channels` planes of `tile × tile`). pub fn run(net: &Net, tile: usize, input: &[f32]) -> Result, NetError> { if !net.fits_tile(tile) || input.len() != net.in_channels * tile * tile { return Err(NetError::Invalid("the tile does not fit the network".into())); } - let mut tensors: Vec = vec![T { channels: net.in_channels, side: tile, data: input.to_vec() }]; + let input = input.to_vec(); + let mut total = input.len(); + let mut tensors = vec![T { channels: net.in_channels, side: tile, data: input }]; let get = |tensors: &[T], id: TensorId| -> Result { if id < tensors.len() { Ok(id) } else { Err(NetError::Invalid("a layer reads a tensor that does not exist yet".into())) } }; @@ -27,7 +57,7 @@ pub fn run(net: &Net, tile: usize, input: &[f32]) -> Result, NetError> let b = c.src2.map(|s| get(&tensors, s)).transpose()?; let side = tensors[a].side; let pad = (c.k - 1) / 2; - let mut data = vec![0f32; c.cout * side * side]; + let mut data = tensor(c.cout, side, &mut total)?; // the input channels in order: the first source's, then the second's let planes: Vec<&[f32]> = std::iter::once(&tensors[a]).chain(b.map(|b| &tensors[b])).flat_map(|t| t.data.chunks(side * side)).collect(); @@ -56,7 +86,7 @@ pub fn run(net: &Net, tile: usize, input: &[f32]) -> Result, NetError> let a = get(&tensors, c.src)?; let side = tensors[a].side; let out_side = 2 * side; - let mut data = vec![0f32; c.cout * out_side * out_side]; + let mut data = tensor(c.cout, out_side, &mut total)?; for co in 0..c.cout { for y in 0..side { for x in 0..side { @@ -77,7 +107,7 @@ pub fn run(net: &Net, tile: usize, input: &[f32]) -> Result, NetError> Op::MaxPool2 { src } => { let a = get(&tensors, *src)?; let (channels, side) = (tensors[a].channels, tensors[a].side / 2); - let mut data = vec![0f32; channels * side * side]; + let mut data = tensor(channels, side, &mut total)?; for ch in 0..channels { for y in 0..side { for x in 0..side { @@ -92,7 +122,7 @@ pub fn run(net: &Net, tile: usize, input: &[f32]) -> Result, NetError> let a = get(&tensors, *src)?; let (channels, side) = (tensors[a].channels / 4, tensors[a].side); let out_side = 2 * side; - let mut data = vec![0f32; channels * out_side * out_side]; + let mut data = tensor(channels, out_side, &mut total)?; for ch in 0..channels { for y in 0..side { for x in 0..side { @@ -131,3 +161,39 @@ impl TileRunner for ReferenceRunner { run(&self.net, self.tile, input).map_err(|e| Error::Runtime(e.to_string())) } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::net::ConvT2; + + fn convt(src: TensorId, cin: usize, cout: usize) -> Op { + Op::ConvT2(ConvT2 { src, cin, cout, weight: vec![0.1; cin * cout * 4], bias: vec![0.0; cout], leaky: None }) + } + + /// A graph that fits the final-shape check while zooming its intermediates out and back: 256 `ConvT2`s double + /// the sides (256 tiers), 255 `MaxPool`s halve them back to the layer above the input. + fn hostile() -> Net { + let mut ops = Vec::new(); + let mut last = 0usize; + for _ in 0..256 { + ops.push(convt(last, 1, 1)); + last += 1; + } + for _ in 0..255 { + ops.push(Op::MaxPool2 { src: last }); + last += 1; + } + Net::new(1, ops).unwrap() + } + + /// The zoomed-out layers' memory was never reserved: either one tensor or the retained whole (the two bounds + /// share the cap) says why the run stops. + #[test] + fn a_graph_that_zooms_its_intermediates_out_and_back_is_refused_before_their_memory_is_reserved() { + assert!(matches!( + run(&hostile(), 1, &[1.0]), + Err(NetError::Invalid(why)) if why.contains("too large") || why.contains("too much working memory") + )); + } +} diff --git a/crates/merge/src/lib.rs b/crates/merge/src/lib.rs index b5ccd4bb8..ca7f91821 100644 --- a/crates/merge/src/lib.rs +++ b/crates/merge/src/lib.rs @@ -34,6 +34,8 @@ pub use pano::{PanoOptions, PanoResult, Projection, stitch}; pub enum MergeError { #[error("{0} photo(s) selected; at least {1} are needed")] TooFew(usize, usize), + #[error("{0} photo(s) selected; a stitch of more than {1} is refused: matching every pair would take far too long")] + TooMany(usize, usize), #[error("{0}")] Mismatch(String), #[error("could not read a photo: {0}")] diff --git a/crates/merge/src/pano/camera.rs b/crates/merge/src/pano/camera.rs index 7625170c3..4589f88db 100644 --- a/crates/merge/src/pano/camera.rs +++ b/crates/merge/src/pano/camera.rs @@ -61,10 +61,12 @@ pub struct PairMatch { /// Match every pair of images. `feats[k]` are in work coordinates scaled by `scales[k]`. pub fn match_pairs(feats: &[Features], scales: &[f64]) -> Vec { let n = feats.len(); - let pairs: Vec<(usize, usize)> = (0..n).flat_map(|i| (i + 1..n).map(move |j| (i, j))).collect(); - let mut out: Vec = pairs - .par_iter() - .filter_map(|&(i, j)| { + // the pairs are generated lazily: materialising all n(n−1)/2 up front is quadratic memory, including + // when every comparison comes up empty (no usable features) + let mut out: Vec = (0..n) + .into_par_iter() + .flat_map_iter(|i| ((i + 1)..n).map(move |j| (i, j))) + .filter_map(|(i, j)| { let m = features::match_features(&feats[i], &feats[j], 0.8); if m.len() < 12 { return None; diff --git a/crates/merge/src/pano/mod.rs b/crates/merge/src/pano/mod.rs index 5ce439681..ba7e1c63f 100644 --- a/crates/merge/src/pano/mod.rs +++ b/crates/merge/src/pano/mod.rs @@ -53,6 +53,11 @@ impl Default for PanoOptions { /// The largest panorama, in megapixels (Lightroom's limit too); a bigger one is scaled down. pub const MAX_MEGAPIXELS: f64 = 512.0; +/// Most frames one stitch accepts: pair matching compares every pair of frames (n(n−1)/2 runs), so an +/// unbounded frame count is quadratic CPU as well as quadratic memory while the frames load. A panorama +/// with more than [`MAX_FRAMES`] shots is refused with an error, not attempted. +pub const MAX_FRAMES: usize = 256; + /// The longest edge of a panorama, in pixels (Lightroom's limit too); a longer one is scaled down. pub const MAX_EDGE: f64 = 65_000.0; @@ -224,6 +229,9 @@ pub fn stitch(mut frames: Vec, opts: &PanoOptions, progress: &Progress) - if n < 2 { return Err(MergeError::TooFew(n, 2)); } + if n > MAX_FRAMES { + return Err(MergeError::TooMany(n, MAX_FRAMES)); + } check(progress, 0.01, "Finding features")?; let (cams, rms_before, rms) = register(&frames, progress)?; let used: Vec = (0..n).filter(|&i| cams[i].is_some()).collect(); diff --git a/crates/merge/tests/it/pano.rs b/crates/merge/tests/it/pano.rs index fdb736524..aeecde64c 100644 --- a/crates/merge/tests/it/pano.rs +++ b/crates/merge/tests/it/pano.rs @@ -246,6 +246,18 @@ fn unrelated_photos_do_not_stitch() { assert!(stitch(frames, &PanoOptions::default(), &no_progress).is_err()); } +/// Pair matching compares every pair of frames before anything is known about them: an unbounded +/// photo count is quadratic CPU and memory (issue #1519). More than `MAX_FRAMES` is refused at once, +/// before any frame is worked on. +#[test] +fn a_stitch_of_more_than_the_frame_cap_is_refused_up_front() { + let frames: Vec = (0..lightcraft_merge::pano::MAX_FRAMES + 1) + .map(|i| Frame::from_linear_rec2020(Rgb32f::from_fn(8, 8, |_, _| [(i % 255) as f32 / 255.0, 0.0, 0.0]), None)) + .collect(); + let err = stitch(frames, &PanoOptions::default(), &no_progress).unwrap_err(); + assert!(err.to_string().contains("refused"), "the frame cap errors, it does not begin the quadratic match: {err}"); +} + /// Issue #1455: a panorama came out at 40 MP whatever its photos' resolution. It now keeps their /// own resolution unless a size is asked for (up to Lightroom's 512 MP); a size scales it down, /// and a NaN one (from an agent) is no size at all, not a 0.1 MP thumbnail. diff --git a/crates/meta/src/exif.rs b/crates/meta/src/exif.rs index f24bb5f2f..adcba0f80 100644 --- a/crates/meta/src/exif.rs +++ b/crates/meta/src/exif.rs @@ -1,7 +1,7 @@ //! Exif / TIFF tags → [`Metadata`] (CIPA DC-008 Exif 2.32, TIFF 6.0, DNG 1.7 tag semantics). use crate::{DateTime, Flash, Gps, Metadata, Orientation}; -use lightcraft_tiff::{Ifd, Tiff, TiffError, tags as t}; +use lightcraft_tiff::{Ifd, ParseOptions, Tiff, TiffError, tags as t}; /// Strip an `Exif\0\0` (or `Exif\0\xff`) prefix as found in JPEG APP1 and some WebP `EXIF` chunks. pub fn strip_exif_header(b: &[u8]) -> &[u8] { @@ -16,8 +16,16 @@ pub fn read_exif(bytes: &[u8]) -> Metadata { /// Like [`read_exif`] but reports a structurally invalid TIFF header / IFD0. pub fn try_read_exif(bytes: &[u8]) -> Result { + try_read_exif_with(bytes, &ParseOptions::default()) +} + +/// [`try_read_exif`] with the caller's parser options (its value allocation budget in particular). +/// A decoder working from a sparse buffer — only selected ranges of the logical file are actually +/// loaded, the rest is zeros — must not size its allocations by the logical file length, which +/// unrelated bytes of the file can inflate at no cost to the attacker. +pub fn try_read_exif_with(bytes: &[u8], opts: &ParseOptions) -> Result { let data = strip_exif_header(bytes); - let tiff = Tiff::parse(data)?; + let tiff = Tiff::parse_with(data, opts)?; let mut m = from_tiff(&tiff); // Panasonic writes no LensModel, only the name in its maker note (and in a RW2 that note is not in the raw's own Exif) if m.lens_model.is_none() { diff --git a/crates/meta/src/focus_read.rs b/crates/meta/src/focus_read.rs index 5a5d349d1..65adbee75 100644 --- a/crates/meta/src/focus_read.rs +++ b/crates/meta/src/focus_read.rs @@ -171,7 +171,15 @@ fn tiff(r: &mut Reader, head: Vec) -> io::Result> { loaded.insert(0, head.len() as u64); for _ in 0..MAX_ROUNDS { match plan(&buf, &loaded) { - Step::Ready => return Ok(crate::try_read_exif(&buf).ok().and_then(|m| m.focus)), + Step::Ready => { + // The decoder's allocation budget follows the bytes actually fetched, not the logical + // file length: only the loaded ranges are data, and an unrelated entry that claims a + // huge value must be cut by what leaving it cost, not by what the buffer claims to hold. + // The same 8 × / + 1 MiB shape the general TIFF reader uses, over `reader.bytes` instead. + let opts = + lightcraft_tiff::ParseOptions { value_budget: Some(r.bytes.saturating_mul(8).saturating_add(1 << 20)), ..Default::default() }; + return Ok(crate::try_read_exif_with(&buf, &opts).ok().and_then(|m| m.focus)); + } Step::Nothing => return Ok(None), Step::Need(ranges) => { for (a, b) in ranges { diff --git a/crates/meta/src/lib.rs b/crates/meta/src/lib.rs index baebbfe0f..46c7a1043 100644 --- a/crates/meta/src/lib.rs +++ b/crates/meta/src/lib.rs @@ -27,7 +27,7 @@ mod xmp_merge; pub use container::{Embedded, embedded, jpeg_segments, png_chunks, webp_chunks}; pub use datetime::DateTime; -pub use exif::{from_tiff, read_exif, strip_exif_header, try_read_exif, write_exif}; +pub use exif::{from_tiff, read_exif, strip_exif_header, try_read_exif, try_read_exif_with, write_exif}; pub use focus::FocusPoint; pub use gpx::{GpxError, Match, TrackPoint, Tracklog, parse_gpx}; pub use iptc::parse_iptc; diff --git a/crates/meta/src/xmp.rs b/crates/meta/src/xmp.rs index 2dbf59482..d2f6f8ee2 100644 --- a/crates/meta/src/xmp.rs +++ b/crates/meta/src/xmp.rs @@ -154,6 +154,11 @@ const MAX_NODES: usize = 200_000; // Expanding a long namespace URI on many short names must not amplify a small packet without bound. pub(crate) const MAX_OPAQUE_NAME_BYTES: usize = 8 * 1024 * 1024; +/// Most text a converter's flattened property keys may add up to. Keys repeat their whole ancestor +/// path (`parent/field`), so a document with a long nested name and a wide child fan-out otherwise +/// retains its prefix once per property: node limits alone cannot bound that duplication. +const MAX_KEY_BYTES: usize = 1 << 20; + /// Names canonicalised by namespace URI (see [`NAMESPACES`]); conflicting foreign prefixes are opaque. pub(crate) fn canonical_name(qname: &str, scopes: &[Vec<(String, String)>], opaque_budget: &mut usize) -> Result { canonical(qname, scopes, opaque_budget) @@ -332,18 +337,29 @@ fn is_rdf_meta_attr(k: &str) -> bool { matches!(k, "rdf:about" | "rdf:ID" | "rdf:nodeID" | "xml:lang" | "rdf:parseType" | "rdf:resource" | "rdf:datatype") || k.starts_with("xmlns") } -fn collect_description(d: &Node, prefix: &str, props: &mut BTreeMap>) { +fn collect_description(d: &Node, prefix: &str, props: &mut BTreeMap>, budget: &mut usize) { for (k, v) in &d.attrs { - if !is_rdf_meta_attr(k) { + if !is_rdf_meta_attr(k) && take_key_bytes(&format!("{prefix}{k}"), budget) { props.entry(format!("{prefix}{k}")).or_default().push(v.clone()); } } for c in &d.children { - collect_property(c, prefix, props); + collect_property(c, prefix, props, budget); } } -fn collect_property(p: &Node, prefix: &str, props: &mut BTreeMap>) { +/// Charge one flattened key's length to the key budget; `false` when the budget is spent (the +/// property is then left out, like other elements a limit has cut). +fn take_key_bytes(key: &str, budget: &mut usize) -> bool { + let n = key.len(); + if *budget < n { + return false; + } + *budget -= n; + true +} + +fn collect_property(p: &Node, prefix: &str, props: &mut BTreeMap>, budget: &mut usize) { let key = format!("{prefix}{}", p.name); if let Some(arr) = p.children.iter().find(|c| matches!(c.name.as_str(), "rdf:Seq" | "rdf:Bag" | "rdf:Alt")) { let mut items: Vec<(bool, String)> = arr @@ -357,29 +373,36 @@ fn collect_property(p: &Node, prefix: &str, props: &mut BTreeMap = p.attrs.iter().filter(|(k, _)| !is_rdf_meta_attr(k)).collect(); if !field_attrs.is_empty() && p.children.is_empty() && p.text.trim().is_empty() { for (k, v) in field_attrs { - props.entry(format!("{struct_prefix}{k}")).or_default().push(v.clone()); + let full = format!("{struct_prefix}{k}"); + if take_key_bytes(&full, budget) { + props.entry(full).or_default().push(v.clone()); + } } return; } let v = p.attr("rdf:resource").map(str::to_string).unwrap_or_else(|| p.text.trim().to_string()); - props.entry(key).or_default().push(v); + if take_key_bytes(&key, budget) { + props.entry(key).or_default().push(v); + } } fn find_descriptions<'a>(n: &'a Node, out: &mut Vec<&'a Node>) { @@ -498,7 +521,8 @@ pub fn parse_xmp(s: &str) -> Result { let mut props = BTreeMap::new(); let mut values = BTreeMap::new(); for d in descs { - collect_description(d, "", &mut props); + let mut key_budget = MAX_KEY_BYTES; + collect_description(d, "", &mut props, &mut key_budget); if let XmpValue::Struct(m) = node_struct(d) { values.extend(m); } @@ -1171,4 +1195,29 @@ mod tests { assert_eq!(entity("#x41"), Some('A')); assert_eq!(entity("#9"), Some('\t')); } + + /// Flattened keys repeat their whole ancestor path, so a long nested name with a wide child + /// fan-out would otherwise retain its prefix once per leaf: the cumulative key budget stops it + /// (a 65 KB document with 3000 leaves must not retain ~2.4 MB of key text). + #[test] + fn flattened_keys_stay_within_the_cumulative_budget() { + let level = "n".repeat(200); + let mut doc = String::from( + "", + ); + for _ in 0..4 { + doc.push_str(&format!("")); + } + doc.push_str(&"".repeat(3000)); + doc.push_str(""); + for _ in 0..4 { + doc.push_str(&format!("")); + } + doc.push_str(""); + let d = parse_xmp(&doc).unwrap(); + let total: usize = d.properties.keys().map(String::len).sum(); + assert!(total <= MAX_KEY_BYTES, "flattened-key text {total} is over the {MAX_KEY_BYTES} budget"); + assert!(d.properties.len() < 3000, "keys stop being added once the budget is spent"); + assert!(!d.properties.is_empty(), "properties parsed before the budget are kept"); + } } diff --git a/crates/raw/src/tiffraw.rs b/crates/raw/src/tiffraw.rs index 8cd6a13e2..34bd7e53b 100644 --- a/crates/raw/src/tiffraw.rs +++ b/crates/raw/src/tiffraw.rs @@ -105,48 +105,72 @@ pub fn read_image(data: &[u8], info: &ImageInfo, order: ByteOrder, packing: Pack let chunks = info.chunks(data.len() as u64); let planar = info.planar == 2 && cpp > 1; let ccpp = if planar { 1 } else { cpp }; - let decoded: Vec> = - chunks.par_iter().map(|c| decode_chunk(data, info, c, order, packing, ccpp, bits, float).map(|p| (*c, p))).collect(); + // The chunks are decoded and copied into the output in groups whose combined nominal size stays within + // a few times the output buffer's: everything decoded at once would retain the whole image twice, and + // a hostile tile table's chosen padding could stretch that much further (issue #1517). One group + // still decodes in parallel; the grid order groups make, the copies stay disjoint. + let group_samples = (total / 4).max(1 << 20); let mut out_u16 = if float { Vec::new() } else { vec![0u16; total] }; let mut out_f32 = if float { vec![0f32; total] } else { Vec::new() }; - for r in decoded { - // a chunk that cannot be decoded is not a partial image: the file stays preview-only - let (c, px) = r?; - let (cw, ch) = (c.width as usize, c.height as usize); - let (x0, y0) = (c.x as usize, c.y as usize); - if x0 >= w || y0 >= h { - continue; + let mut decode_from = 0usize; + while decode_from < chunks.len() { + let mut to = decode_from; + let mut taken = 0usize; + for (k, c) in chunks[decode_from..].iter().enumerate() { + let n = chunk_samples(c, ccpp); + if taken > 0 && taken + n > group_samples { + break; + } + taken = taken.saturating_add(n); + to = decode_from + k + 1; } - let copy_w = cw.min(w - x0); - let copy_h = ch.min(h - y0); - for y in 0..copy_h { - for x in 0..copy_w { - for s in 0..ccpp { - let src = (y * cw + x) * ccpp + s; - let dst_s = if planar { c.plane as usize } else { s }; - if dst_s >= cpp { - continue; - } - let dst = ((y0 + y) * w + x0 + x) * cpp + dst_s; - match &px { - ChunkPx::U16(v) => { - if let (Some(o), Some(&s)) = (out_u16.get_mut(dst), v.get(src)) { - *o = s; - } + let decoded: Vec> = + chunks[decode_from..to].par_iter().map(|c| decode_chunk(data, info, c, order, packing, ccpp, bits, float).map(|p| (*c, p))).collect(); + for r in decoded { + // a chunk that cannot be decoded is not a partial image: the file stays preview-only + let (c, px) = r?; + let (cw, ch) = (c.width as usize, c.height as usize); + let (x0, y0) = (c.x as usize, c.y as usize); + if x0 >= w || y0 >= h { + continue; + } + let copy_w = cw.min(w - x0); + let copy_h = ch.min(h - y0); + for y in 0..copy_h { + for x in 0..copy_w { + for s in 0..ccpp { + let src = (y * cw + x) * ccpp + s; + let dst_s = if planar { c.plane as usize } else { s }; + if dst_s >= cpp { + continue; } - ChunkPx::F32(v) => { - if let (Some(o), Some(&s)) = (out_f32.get_mut(dst), v.get(src)) { - *o = s; + let dst = ((y0 + y) * w + x0 + x) * cpp + dst_s; + match &px { + ChunkPx::U16(v) => { + if let (Some(o), Some(&s)) = (out_u16.get_mut(dst), v.get(src)) { + *o = s; + } + } + ChunkPx::F32(v) => { + if let (Some(o), Some(&s)) = (out_f32.get_mut(dst), v.get(src)) { + *o = s; + } } } } } } } + decode_from = to; } Ok(if float { RawData::F32(out_f32) } else { RawData::U16(out_u16) }) } +/// A chunk's nominal decoded size in samples (per plane, as the decoder builds it). +fn chunk_samples(c: &Chunk, ccpp: usize) -> usize { + (c.width as usize).saturating_mul(c.height as usize).saturating_mul(ccpp) +} + #[allow(clippy::too_many_arguments)] fn decode_chunk(data: &[u8], info: &ImageInfo, c: &Chunk, order: ByteOrder, packing: Packing, cpp: usize, bits: u32, float: bool) -> Result { let src = chunk_bytes(data, c).ok_or_else(|| RawError::Corrupt("chunk offset past end of file".into()))?; @@ -180,8 +204,12 @@ fn decode_chunk(data: &[u8], info: &ImageInfo, c: &Chunk, order: ByteOrder, pack comp::LOSSY_JPEG => { // lossy DNG: each chunk is a baseline (DCT) JPEG of 8-bit samples let (px, jw, jh, jc) = lossy_jpeg(src, cpp, MAX_JPEG_SAMPLES)?; - let mut v = vec![0u16; n]; - for y in 0..ch.min(jh) { + // the buffer holds the rows the decoded JPEG covers (the caller zero-fills the tile's + // remainder and reads only rows it fills): a tile-many-larger decoded JPEG does not + // get the nominal tile's full buffer (issue #1517) + let rows = ch.min(jh); + let mut v = vec![0u16; rows.max(1).saturating_mul(cw).saturating_mul(cpp)]; + for y in 0..rows { for x in 0..cw.min(jw) { for k in 0..cpp { v[(y * cw + x) * cpp + k] = px[(y * jw + x) * jc + k.min(jc - 1)] as u16; @@ -518,6 +546,36 @@ mod lossy_tests { assert!(lossy_jpeg(&j, 3, MAX_JPEG_SAMPLES).is_ok() || matches!(lossy_jpeg(&j, 3, MAX_JPEG_SAMPLES), Err(RawError::Corrupt(_)))); } + /// A lossy tile whose JPEG decodes far less than the nominal tile is fine: the rows the JPEG + /// covers hold its pixels, the rest of the output stays zero (and the tile's intermediate buffer + /// is sized by the JPEG, not by the nominal tile). + #[test] + fn a_small_lossy_jpeg_in_a_large_nominal_tile_decodes_with_zeros_below() { + let px: Vec = (0..8 * 8).flat_map(|_| [30u8, 90, 150]).collect(); + let mut j = Vec::new(); + jpeg_encoder::Encoder::new(&mut j, 100).encode(&px, 8, 8, jpeg_encoder::ColorType::Rgb).unwrap(); + let info = ImageInfo { + width: 16, + height: 16, + bits_per_sample: vec![8, 8, 8], + samples_per_pixel: 3, + compression: comp::LOSSY_JPEG, + photometric: 34892, + planar: 1, + predictor: 1, + sample_format: 1, + new_subfile_type: 0, + layout: Layout::Tiles { tile_width: 16, tile_height: 16 }, + offsets: vec![0], + byte_counts: vec![j.len() as u64], + }; + let RawData::U16(v) = read_image(&j, &info, ByteOrder::Little, Packing::Msb).unwrap() else { panic!("integer samples") }; + assert_eq!(v.len(), 16 * 16 * 3, "the output is the full nominal image"); + let px = |x: usize, y: usize| &v[(y * 16 + x) * 3..(y * 16 + x) * 3 + 3]; + assert!((px(3, 3)[0] as i32 - 30).abs() <= 3 && (px(3, 3)[2] as i32 - 150).abs() <= 3, "covered rows: {:?}", px(3, 3)); + assert!(px(3, 12).iter().all(|&s| s == 0), "rows below the JPEG stay zero: {:?}", px(3, 12)); + } + /// A deflated tile is refused when its packed bytes are too few for deflate to have produced /// the nominal chunk, or when the stream stops short of the rows inside the image; a fully /// covered flat tile still decodes. diff --git a/crates/tiff/src/makernote.rs b/crates/tiff/src/makernote.rs index 7ed5e1997..288b366f6 100644 --- a/crates/tiff/src/makernote.rs +++ b/crates/tiff/src/makernote.rs @@ -55,7 +55,11 @@ pub fn parse_makernote(data: &[u8], offset: u64, len: u64, order: ByteOrder, mak Some(b"MM") => Some(ByteOrder::Big), _ => None, }; - let opts = ParseOptions { max_ifds: 64, max_depth: 3, ..Default::default() }; + // A fixed value budget: a maker note is at most tens of kilobytes of values in any honest file, and + // computing the budget from the buffer's length multiplies the file length (a hostile entry in a + // sparse header-read buffer, where the buffer holds the file's logical size but only a few + // kilobytes are loaded, would decode allocations no actual data backs). + let opts = ParseOptions { max_ifds: 64, max_depth: 3, value_budget: Some(1 << 20), ..Default::default() }; let try_parse = |kind: MakerNoteKind, ifd_at: u64, base: u64, ord: ByteOrder| -> Option { let (ifd, _) = parse_ifd_at(data, ifd_at, ord, base, false, &opts).ok()?; (!ifd.entries.is_empty()).then_some(MakerNote { kind, order: ord, base, ifd }) @@ -192,6 +196,35 @@ mod tests { assert_eq!(m.ifd.u64s(2).unwrap(), vec![9, 0]); } + /// A maker note's value budget is fixed, not computed from the length of the buffer it sits in: + /// an entry claiming more than the note budget (here 2 MB) is dropped, a fitting one is read. In a + /// sparse header read the buffer holds the file's logical size while only kilobytes were loaded, + /// so a length-scaled budget would let an unrelated entry decode megabytes of zeros (see the focus + /// reader, try_read_exif_with). + #[test] + fn a_note_entry_past_the_fixed_value_budget_is_dropped() { + let order = ByteOrder::Little; + let mut note = b"SONY DSC \0\0\0".to_vec(); + // the Sony IFD directly after the 12-byte header, one fitting BYTE entry and a decoy + // BYTE[2 MB] entry pointing into the file's own (zero-filled) body + order.put_u16(&mut note, 2); + order.put_u16(&mut note, 0x201b); + order.put_u16(&mut note, 1); + order.put_u32(&mut note, 1); + note.extend([3, 0, 0, 0]); + order.put_u16(&mut note, 0x9900); + order.put_u16(&mut note, 1); + order.put_u32(&mut note, 2 << 20); + order.put_u32(&mut note, 200); + order.put_u32(&mut note, 0); + let mut bytes = note.clone(); + bytes.resize(200 + (2 << 20), 0); // the decoy's value range is inside the real bytes + let m = parse_makernote(&bytes, 0, note.len() as u64, order, "SONY").unwrap(); + assert_eq!(m.kind, MakerNoteKind::Sony); + assert!(m.ifd.get(0x201b).is_some(), "the fitting entry is read"); + assert!(m.ifd.get(0x9900).is_none(), "the decoy is dropped by the value budget"); + } + #[test] fn garbage_note_is_none() { let (bytes, off, len) = with_note(ByteOrder::Little, vec![0xff; 40]); diff --git a/crates/tiff/src/reader.rs b/crates/tiff/src/reader.rs index bd728ca9c..70c66c5a7 100644 --- a/crates/tiff/src/reader.rs +++ b/crates/tiff/src/reader.rs @@ -338,6 +338,25 @@ pub fn decode(o: ByteOrder, typ: FieldType, b: &[u8]) -> Value { #[cfg(test)] mod tests { use super::*; + use crate::Value; + use crate::tags as t; + use crate::writer::{IfdBuilder, TiffWriter}; + + /// A value budget the caller sets bounds what is decoded: an entry claiming more value bytes than + /// the budget allows is dropped, entries that fit are read (the sparse header-read path, where the + /// buffer holds the file's logical size but only a few kilobytes were ever loaded, sizes its + /// budget from what was fetched, not from whatever length the file claims). + #[test] + fn the_value_budget_drops_over_budget_entries_and_keeps_the_rest() { + let mut ifd0 = IfdBuilder::new(); + ifd0.set(t::MAKE, Value::Ascii("Keep".into())); + ifd0.set(0x9000, Value::Byte(vec![0; 2 << 20])); + let file = TiffWriter { order: ByteOrder::Little, bigtiff: false }.write(&[ifd0]).unwrap(); + let opts = ParseOptions { value_budget: Some(1 << 20), ..Default::default() }; + let tiff = Tiff::parse_with(&file, &opts).unwrap(); + assert_eq!(tiff.ifds[0].string(t::MAKE), Some("Keep".to_string()), "a fitting entry is read"); + assert!(tiff.ifds[0].get(0x9000).is_none(), "the over-budget entry is dropped"); + } /// A BigTIFF whose first IFD, at 80, declares `declared` entries but holds one, followed by a next-IFD pointer /// `next`; a valid one-entry IFD sits at 16. From 3022baa8615a2956e239b7febf3fe4ff98289885 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Sun, 25 Oct 2026 16:12:03 -0400 Subject: [PATCH 3/3] security(raw,meta): follow-ups from review An oversized CR3 candidate is skipped without ending the search, repeated child IFD references count once toward the pending cap, deflated chunks lay their rows out as the unpacker reads them (16-bit words, 32-bit word alignment), the tile grid's nominal size is checked against the image before anything is decoded, chunk grouping saturates, and the XMP flattened-key budget is document-wide. The JPEG-header test now parses a real header. --- crates/meta/src/xmp.rs | 8 ++++-- crates/raw/src/preview.rs | 5 ++-- crates/raw/src/sparse.rs | 8 +++++- crates/raw/src/tiffraw.rs | 60 +++++++++++++++++++++++++++++---------- 4 files changed, 61 insertions(+), 20 deletions(-) diff --git a/crates/meta/src/xmp.rs b/crates/meta/src/xmp.rs index d2f6f8ee2..ac0a8808a 100644 --- a/crates/meta/src/xmp.rs +++ b/crates/meta/src/xmp.rs @@ -520,8 +520,9 @@ pub fn parse_xmp(s: &str) -> Result { find_descriptions(&tree, &mut descs); let mut props = BTreeMap::new(); let mut values = BTreeMap::new(); + // One budget for the whole document: several descriptions cannot each spend a fresh one. + let mut key_budget = MAX_KEY_BYTES; for d in descs { - let mut key_budget = MAX_KEY_BYTES; collect_description(d, "", &mut props, &mut key_budget); if let XmpValue::Struct(m) = node_struct(d) { values.extend(m); @@ -1208,7 +1209,10 @@ mod tests { for _ in 0..4 { doc.push_str(&format!("")); } - doc.push_str(&"".repeat(3000)); + // each leaf has a name of its own, so each one adds a key + for i in 0..3000 { + doc.push_str(&format!("")); + } doc.push_str(""); for _ in 0..4 { doc.push_str(&format!("")); diff --git a/crates/raw/src/preview.rs b/crates/raw/src/preview.rs index 8a7e180c7..4bcf6963e 100644 --- a/crates/raw/src/preview.rs +++ b/crates/raw/src/preview.rs @@ -400,7 +400,7 @@ const CR3_MAX_WALK: usize = 8 << 20; fn cr3_preview_boxes(bytes: &[u8]) -> Option<&[u8]> { let mut best: Option<&[u8]> = None; let mut walked = 0usize; - 'tags: for tag in [b"PRVW", b"THMB"] { + for tag in [b"PRVW", b"THMB"] { let mut from = 0; while let Some(i) = bytes.get(from..).and_then(|s| s.windows(4).position(|w| w == tag)).map(|p| p + from) { from = i + 4; @@ -412,8 +412,9 @@ fn cr3_preview_boxes(bytes: &[u8]) -> Option<&[u8]> { continue; } let jpeg = &bytes[j..j + len]; + // One that doesn't fit the budget left is skipped; a smaller one later may still fit. if walked + jpeg.len() > CR3_MAX_WALK { - break 'tags; + continue; } walked += jpeg.len(); if is_dct_jpeg(jpeg) { diff --git a/crates/raw/src/sparse.rs b/crates/raw/src/sparse.rs index a1a718f14..8a404a25e 100644 --- a/crates/raw/src/sparse.rs +++ b/crates/raw/src/sparse.rs @@ -314,6 +314,9 @@ fn walk_tiff(w: &mut Want, origin: u64, data: &[u8], extra: &[u16]) -> Result Result= len || !queued.insert(v) { + continue; + } if stack.len() >= MAX_PENDING { return Err("too many child IFDs queued to visit"); } @@ -376,7 +382,7 @@ fn walk_tiff(w: &mut Want, origin: u64, data: &[u8], extra: &[u16]) -> Result= MAX_PENDING { return Err("too many child IFDs queued to visit"); } diff --git a/crates/raw/src/tiffraw.rs b/crates/raw/src/tiffraw.rs index 34bd7e53b..57878b881 100644 --- a/crates/raw/src/tiffraw.rs +++ b/crates/raw/src/tiffraw.rs @@ -109,6 +109,12 @@ pub fn read_image(data: &[u8], info: &ImageInfo, order: ByteOrder, packing: Pack // a few times the output buffer's: everything decoded at once would retain the whole image twice, and // a hostile tile table's chosen padding could stretch that much further (issue #1517). One group // still decodes in parallel; the grid order groups make, the copies stay disjoint. + // The tile grid decides how much is decoded, whatever the file's bytes: the nominal sizes of all + // the chunks stay within a few times the image (edge tiles pad it) before any is decoded. + let nominal = chunks.iter().fold(0usize, |sum, c| sum.saturating_add(chunk_samples(c, ccpp))); + if nominal > total.saturating_mul(4).saturating_add(1 << 24) { + return Err(RawError::Corrupt("the tile grid is much larger than the image".into())); + } let group_samples = (total / 4).max(1 << 20); let mut out_u16 = if float { Vec::new() } else { vec![0u16; total] }; let mut out_f32 = if float { vec![0f32; total] } else { Vec::new() }; @@ -118,7 +124,7 @@ pub fn read_image(data: &[u8], info: &ImageInfo, order: ByteOrder, packing: Pack let mut taken = 0usize; for (k, c) in chunks[decode_from..].iter().enumerate() { let n = chunk_samples(c, ccpp); - if taken > 0 && taken + n > group_samples { + if taken > 0 && taken.saturating_add(n) > group_samples { break; } taken = taken.saturating_add(n); @@ -166,6 +172,21 @@ pub fn read_image(data: &[u8], info: &ImageInfo, order: ByteOrder, packing: Pack Ok(if float { RawData::F32(out_f32) } else { RawData::U16(out_u16) }) } +/// Bytes that `n` samples take in a row of an uncompressed or inflated chunk, laid out as +/// [`unpack_chunk`] reads them: whole bytes or 16-bit words, or 32-bit words for `Words32Msb`. +fn packed_row_bytes(n: u64, bits: u32, packing: Packing, float: bool) -> u64 { + if float { + return n.saturating_mul(u64::from(bits.div_ceil(8))); + } + let bit_len = n.saturating_mul(u64::from(bits)); + match (bits, packing) { + (8, _) => n, + (16, _) | (_, Packing::Word16) => n.saturating_mul(2), + (_, Packing::Msb | Packing::Lsb) => bit_len.div_ceil(8), + (_, Packing::Words32Msb) => bit_len.div_ceil(32).saturating_mul(4), + } +} + /// A chunk's nominal decoded size in samples (per plane, as the decoder builds it). fn chunk_samples(c: &Chunk, ccpp: usize) -> usize { (c.width as usize).saturating_mul(c.height as usize).saturating_mul(ccpp) @@ -219,9 +240,8 @@ fn decode_chunk(data: &[u8], info: &ImageInfo, c: &Chunk, order: ByteOrder, pack Ok(ChunkPx::U16(v)) } comp::ADOBE_DEFLATE | comp::DEFLATE => { - let bytes_per = if float { bits.div_ceil(8) as usize } else { 0 }; // row bytes in u64: a wrapping `cw · cpp · …` in usize could come out small on 32-bit targets and slip past the gate - let row_bytes = if float { cw as u64 * cpp as u64 * bytes_per as u64 } else { (cw as u64 * cpp as u64 * bits as u64).div_ceil(8) }; + let row_bytes = packed_row_bytes(cw as u64 * cpp as u64, bits, packing, float); // the gate caps at MAX_SAMPLES (≤ 2^30), so the `usize` cast below is lossless on 32-bit targets too let inflate_len = crate::checked_pixel_count(row_bytes, ch as u64).ok_or_else(|| RawError::Corrupt("bad TIFF chunk dimensions".into()))?; @@ -234,7 +254,7 @@ fn decode_chunk(data: &[u8], info: &ImageInfo, c: &Chunk, order: ByteOrder, pack let raw = inflate(src, inflate_len as usize)?; // The stream has to reach the last row inside the image (edge strips may stop there); // it is laid out in nominal rows, so shorter is a truncated chunk, not a picture. - let last = if float { vw as u64 * cpp as u64 * bytes_per as u64 } else { (vw as u64 * cpp as u64 * bits as u64).div_ceil(8) }; + let last = packed_row_bytes(vw as u64 * cpp as u64, bits, packing, float); let visible = row_bytes.saturating_mul(vh.saturating_sub(1) as u64).saturating_add(last); if (raw.len() as u64) < visible { return Err(RawError::Corrupt(format!( @@ -533,17 +553,27 @@ mod lossy_tests { /// from the JPEG's own dimensions. #[test] fn an_oversized_lossy_jpeg_header_is_refused_before_decoding() { - let mut j = vec![0xff, 0xd8]; // SOI - // SOF0: precision 8, 40000 (0x9c40) x 40000, three components - j.extend([0xff, 0xc0, 0x00, 0x11, 0x08, 0x9c, 0x40, 0x9c, 0x40, 0x03, 0x01, 0x22, 0x00, 0x02, 0x11, 0x01, 0x03, 0x11, 0x01]); - // SOS with three components so header parsing completes normally, then EOI - j.extend([0xff, 0xda, 0x00, 0x0c, 0x03, 0x01, 0x00, 0x02, 0x11, 0x00, 0x03, 0x11, 0x00, 0x00, 0x3f, 0x00, 0x00, 0xff, 0xd9]); - let e = lossy_jpeg(&j, 3, MAX_JPEG_SAMPLES).unwrap_err(); - assert!(matches!(e, RawError::Corrupt(_)), "{e:?}"); - // a header inside the budget passes the check (and then fails to decode only for lack of data) - j[12..14].copy_from_slice(&1024u16.to_be_bytes()); - j[14..16].copy_from_slice(&1024u16.to_be_bytes()); - assert!(lossy_jpeg(&j, 3, MAX_JPEG_SAMPLES).is_ok() || matches!(lossy_jpeg(&j, 3, MAX_JPEG_SAMPLES), Err(RawError::Corrupt(_)))); + // A real 8 × 8 baseline JPEG, whose frame header then claims other dimensions: the decoder + // parses the header, and the size check is what refuses it. + let px = vec![128u8; 8 * 8 * 3]; + let mut j = Vec::new(); + jpeg_encoder::Encoder::new(&mut j, 90).encode(&px, 8, 8, jpeg_encoder::ColorType::Rgb).unwrap(); + // SOF0: marker, length, precision, then the height and the width + let sof = j.windows(2).position(|w| w == [0xff, 0xc0]).expect("a baseline frame header"); + let claim = |j: &mut Vec, size: u16| { + j[sof + 5..sof + 7].copy_from_slice(&size.to_be_bytes()); + j[sof + 7..sof + 9].copy_from_slice(&size.to_be_bytes()); + }; + claim(&mut j, 40_000); + let RawError::Corrupt(refused) = lossy_jpeg(&j, 3, MAX_JPEG_SAMPLES).unwrap_err() else { panic!("a corrupt-data error") }; + assert!(refused.contains("would exceed"), "{refused}"); + // a header inside the budget passes the size check (and then fails to decode only for lack of data) + claim(&mut j, 1024); + match lossy_jpeg(&j, 3, MAX_JPEG_SAMPLES) { + Ok(_) => {} + Err(RawError::Corrupt(m)) => assert!(!m.contains("would exceed"), "the smaller header was refused for its size: {m}"), + Err(other) => panic!("unexpected error: {other:?}"), + } } /// A lossy tile whose JPEG decodes far less than the nominal tile is fine: the rows the JPEG