From b88f72b9df68f07a080772d7fb16a92d6f7db837 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Fri, 23 Oct 2026 22:56:20 -0400 Subject: [PATCH] sec: bound heic-rs 0.1.4 work around hostile HEIF containers heic-rs has no aggregate work budget in container parsing: ipma association entries rescan earlier item ids after each other and an iloc construction-method-2 extent rebuilds its source item afresh. Cap the ipma/iloc boxes before heic-rs reads them, and stop probe paying for a reconstruction heft beyond the file's own size. decode: refuse files carrying more than 12 container transforms (each would re-copy the full picture; CodeRabbit row #44), check a grid's payload canvas against the pixel limit before allocating for it (#48), and to_rgb16 refuses an oversized w*h*ch rather than asking for a multi-gigabyte buffer. --- crates/heif/src/census.rs | 122 +++++++++++++++++++++++++++++ crates/heif/src/lib.rs | 42 +++++++++- crates/heif/src/tests_synthetic.rs | 13 +++ crates/heif/src/ycc.rs | 26 ++++++ 4 files changed, 200 insertions(+), 3 deletions(-) create mode 100644 crates/heif/src/census.rs diff --git a/crates/heif/src/census.rs b/crates/heif/src/census.rs new file mode 100644 index 000000000..6dbe035fc --- /dev/null +++ b/crates/heif/src/census.rs @@ -0,0 +1,122 @@ +//! Caps on the container's metadata boxes, counted before heic-rs touches the file. +//! +//! heic-rs 0.1.4 (pinned `=0.1.4`) has no aggregate work budget inside its container parsing: +//! every `ipma` association entry rescans the vector of already-seen item ids (quadratic in +//! the number of entries), and an `iloc` extent using construction method 2 reconstructs its +//! source item afresh for every extent that references it, with no shared cache. Legitimate +//! phone-photo metadata never comes near these caps — real corpora keep both well under a few +//! kilobytes — so anything above them is hostile or corrupt and is refused before the +//! countable CPU work happens. +//! +//! [`depth_boxes`] stops silently at structural blips: heic-rs's own parser reports those +//! better than this walk could, and an `ipma` box past the cap is caught wherever it parses. + +use heic_rs::boxes::BoxIter; + +use crate::Error; + +/// The largest `ipma` box (item ↔ property associations) payload accepted, 64 KiB. An entry +/// costs at least 3 bytes, so this bounds heic-rs's per-entry rescan of earlier item ids to +/// ~21 000 entries ≈ 220 million comparisons: long enough to be annoying, never minutes. +const MAX_IPMA_BYTES: usize = 64 << 10; + +/// The largest `iloc` box (item locations) payload accepted, 64 KiB. An extent costs at least +/// 12 bytes, so this bounds both the parse itself and how many extents a +/// construction-method-2 reconstruction chain can visit. +const MAX_ILOC_BYTES: usize = 64 << 10; + +/// Falls through unless the container declares more `ipma` or `iloc` payload than a +/// legitimate phone photo ever does. +pub fn cap(bytes: &[u8]) -> Result<(), Error> { + let mut c = Census::default(); + depth_boxes(&mut c, BoxIter::new(bytes), 0); + if c.ipma > MAX_IPMA_BYTES { + return Err(Error::Limit(format!("the image carries {} bytes of property associations, over the {MAX_IPMA_BYTES}-byte cap", c.ipma))); + } + if c.iloc > MAX_ILOC_BYTES { + return Err(Error::Limit(format!("the image carries {} bytes of item locations, over the {MAX_ILOC_BYTES}-byte cap", c.iloc))); + } + Ok(()) +} + +/// The metadata-box payload sizes seen so far. +#[derive(Default)] +struct Census { + ipma: usize, + iloc: usize, +} + +/// Walks boxes, adding the payload sizes of those named in [`Census`]. +/// +/// Only `meta` and `iprp` (the two nesting boxes the metadata is built from) are descended +/// into, no deeper than that depth, so a nested-box trick can't hide a box from the count. +fn depth_boxes(c: &mut Census, boxes: BoxIter<'_>, depth: u32) { + if depth > 3 { + return; + } + for b in boxes { + // A structural error stops the walk; `heic_rs::probe` reports it properly afterwards. + let Ok(b) = b else { return }; + if b.is(b"ipma") { + c.ipma = c.ipma.saturating_add(b.payload.len()); + } else if b.is(b"iloc") { + c.iloc = c.iloc.saturating_add(b.payload.len()); + } else if b.is(b"meta") { + // `meta` is a FullBox: version and flags sit between its header and its children. + if b.payload.len() >= 4 { + depth_boxes(c, BoxIter::new(&b.payload[4..]), depth + 1); + } + } else if b.is(b"iprp") { + depth_boxes(c, b.children(), depth + 1); + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + /// Assembles ftyp + meta(version/flags + iprp(ipco, ipma)) by hand; only the structure + /// the census walks matters, no item is ever defined. + fn file_with(ipma_payload: &[u8]) -> Vec { + let mut out = b"\0\0\0\x18ftypheic\0\0\0\0mif1heic".to_vec(); + let mut iprp = Vec::new(); + iprp.extend(box_of(b"ipco", b"")); + iprp.extend(box_of(b"ipma", ipma_payload)); + let mut meta = vec![0u8, 0, 0, 0]; // version 0, flags 0 + meta.extend(box_of(b"iprp", &iprp)); + out.extend(box_of(b"meta", &meta)); + out + } + + fn box_of(kind: &[u8; 4], body: &[u8]) -> Vec { + let mut out = Vec::with_capacity(8 + body.len()); + let size = u32::try_from(8 + body.len()).expect("test box fits in u32"); + out.extend_from_slice(&size.to_be_bytes()); + out.extend_from_slice(kind); + out.extend_from_slice(body); + out + } + + #[test] + fn an_oversized_ipma_is_refused() { + // 40 000 entries of 6 bytes: 240 KiB of ipma, far over the cap, and quadratic if + // parsed (each entry rescans the earlier item ids in heic-rs 0.1.4). + let mut ipma = Vec::new(); + for _ in 0..40_000u32 { + ipma.extend_from_slice(&[0u8, 0, 0, 0, 0, 0]); + } + let bytes = file_with(&ipma); + let r = crate::probe(&bytes).map(|_| ()); + assert!(matches!(r, Err(Error::Limit(ref m)) if m.contains("associations")), "{r:?}"); + } + + #[test] + fn a_undersized_ipma_is_untouched_by_the_caps() { + // The same shape with a normal ipma (two entries): the caps pass, and whatever error + // follows must not come from them. + let bytes = file_with(&[0, 0, 4, 1, 1, 2, 0, 0]); + let r = crate::probe(&bytes); + assert!(!matches!(r, Err(Error::Limit(ref m)) if m.contains("cap")), "{r:?}"); + } +} diff --git a/crates/heif/src/lib.rs b/crates/heif/src/lib.rs index 21b9cf647..58b2be254 100644 --- a/crates/heif/src/lib.rs +++ b/crates/heif/src/lib.rs @@ -19,6 +19,7 @@ #![forbid(unsafe_code)] #![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)] +mod census; mod orient; /// Lossless synthetic HEIF files for tests, here and in the crates above (feature `testdata`). #[cfg(any(test, feature = "testdata"))] @@ -149,6 +150,7 @@ pub fn probe(bytes: &[u8]) -> Result { } fn probe_unguarded(bytes: &[u8]) -> Result { + census::cap(bytes)?; let info = heic_rs::probe(bytes).map_err(|e| match e { // No `meta` box: no still image, only a `moov` image sequence (a video track). heic_rs::Error::MissingBox("meta") => Error::Unsupported(SEQUENCE), @@ -156,13 +158,28 @@ fn probe_unguarded(bytes: &[u8]) -> Result { })?; let ctx = Context::open(bytes).map_err(err)?; let id = ctx.meta.primary; - let coded = match ctx.grid(id).map_err(err)? { + let mut coded = match ctx.grid(id).map_err(err)? { Some((_, tiles)) => tiles, None => vec![id], }; + // A `dimg` listing may name the same tile more than once; reconstrutable (`iloc` + // construction method 2) items are moreover rebuilt for every extent that references + // them in heic-rs 0.1.4, so our probe would pay the reconstruction again per tile. Each + // unique item only once, then the total reconstruction (all of it assembled from `bytes` + // by heic-rs) is held below one copy of the file — above that a repeat or chain wants + // more bytes than exist (CodeRabbit #21/#50 on this sweep). Order doesn't matter here: + // this only checks that every tile's bytes lie in the file. + coded.sort_unstable(); + coded.dedup(); + let mut reconstructed = 0_usize; for item in coded { - // Borrowed from `bytes` unless split across extents: cheap. - ctx.item_data(item).map_err(err)?; + reconstructed = reconstructed.saturating_add(ctx.item_data(item).map_err(err)?.len()); + if reconstructed > bytes.len() { + return Err(Error::Limit(format!( + "the container holds {reconstructed} bytes of item data, over the {} bytes of the file itself", + bytes.len() + ))); + } } Ok(Info { width: info.width, @@ -232,6 +249,13 @@ fn decode_unguarded(bytes: &[u8], options: &Options) -> Result { /// `with_alpha` and the item has an alpha auxiliary image. fn decode_item(ctx: &Context<'_>, id: u32, options: &Options, with_alpha: bool) -> Result { let p = ctx.props(id).map_err(err)?; + // heic-rs 0.1.4 keeps every associated transform and its [`orient::apply`] allocates the + // full picture again for each: a file could chain the repetition and multiply decode CPU + // without changing any checked size. Real files carry at most one `clap`, `irot` and + // `imir`, so refuse anything approaching [`MAX_TRANSFORMS`] (CodeRabbit #44). + if p.transforms.len() > MAX_TRANSFORMS { + return Err(Error::Limit(format!("the image carries {} transform properties, over the {MAX_TRANSFORMS}-property cap", p.transforms.len()))); + } let limit = options.max_pixels; // The declared sizes are refused before anything is decoded or allocated. let (cw, ch) = ctx.coded_size(id, &p).map_err(err)?; @@ -245,6 +269,14 @@ fn decode_item(ctx: &Context<'_>, id: u32, options: &Options, with_alpha: bool) let signal = ycc::Signal { matrix: colour.matrix, full_range: colour.full_range }; let alpha_id = if with_alpha { ctx.alpha_item(id).map_err(err)? } else { None }; let (w, h) = planes.size(); + // A grid's canvas dimensions come from its payload, not the checked `ispe` (CodeRabbit + // #48): check the real one, before [`ycc::to_rgb16`] allocates for it. + match (u32::try_from(w), u32::try_from(h)) { + (Ok(wu), Ok(hu)) => { + heic_rs::image::check_pixels(wu, hu, limit).map_err(err)?; + } + _ => return Err(Error::Limit(format!("the image canvas is {w}x{h}, beyond the {limit}-pixel limit"))), + } let mode = if options.apply_transforms && orient::odd_crop(w, h, &p.transforms)? { ycc::Upsampling::Bilinear } else { ycc::Upsampling::Nearest }; let mut data = ycc::to_rgb16(&planes, signal, alpha_id.is_some(), mode) .ok_or_else(|| Error::Malformed("the decoded pictures don't cover the image".into()))?; @@ -281,6 +313,10 @@ fn decode_item(ctx: &Context<'_>, id: u32, options: &Options, with_alpha: bool) /// cover its canvas rounded up to whole tiles, so legitimate grids sit near 1. const GRID_BUDGET: u64 = 2; +/// The most transform properties (`clap`/`irot`/`imir`) one item may carry. Real files have at +/// most one of each, so anything approaching this is a reordered or hostile association list. +const MAX_TRANSFORMS: usize = 12; + /// The decoded pictures of an item: one, or a grid's tiles in `dimg` order. struct Coded<'a> { frames: Vec, diff --git a/crates/heif/src/tests_synthetic.rs b/crates/heif/src/tests_synthetic.rs index 3259dc96d..1286ff82a 100644 --- a/crates/heif/src/tests_synthetic.rs +++ b/crates/heif/src/tests_synthetic.rs @@ -185,3 +185,16 @@ fn truncated_and_corrupted_files_are_errors_not_panics() { let _ = decode_thumbnail(&b, 1, &Options::default()); } } + +/// CodeRabbit #44: heic-rs keeps every associated transform and each one re-allocates the +/// whole picture, so 13 of them are over the [`crate::MAX_TRANSFORMS`] cap and refused. +#[test] +fn too_many_transform_properties_are_refused() { + let bytes = build(&Spec { props: std::iter::repeat_n(Prop::Irot(0), 13).collect(), ..Spec::new(32, 32, &grey) }); + let r = decode(&bytes, &Options::default()); + assert!(matches!(r, Err(Error::Limit(ref m)) if m.contains("transform")), "{r:?}"); + // At the cap they're not: 12 identity rotations decode normally. + let bytes = build(&Spec { props: std::iter::repeat_n(Prop::Irot(0), 12).collect(), ..Spec::new(32, 32, &grey) }); + let d = decode(&bytes, &Options::default()).unwrap(); + assert_eq!((d.width, d.height), (32, 32)); +} diff --git a/crates/heif/src/ycc.rs b/crates/heif/src/ycc.rs index a0429b453..384d25f2a 100644 --- a/crates/heif/src/ycc.rs +++ b/crates/heif/src/ycc.rs @@ -161,6 +161,13 @@ pub fn to_rgb16(planes: &Planes<'_>, signal: Signal, alpha: bool, mode: Upsampli if !(8..=16).contains(&bd) || w == 0 || h == 0 { return None; } + // The default caller limit is 2²8 pixels, at most four channels per pixel, so the largest + // legitimate `w·h·ch` is 2²30 samples. Anything more is a malformed or hostile canvas the + // decode path would have refused before calling here (CodeRabbit #48): refuse rather than + // ask for a multi-gigabuffer (or worse, over usize). + if w.checked_mul(h)?.checked_mul(ch)? > 1 << 30 { + return None; + } let max = ((1u32 << bd) - 1) as f32; let half = (1u32 << (bd - 1)) as f32; let scale = (1u32 << (bd - 8)) as f32; @@ -323,4 +330,23 @@ mod tests { .is_none() ); } + + /// CodeRabbit #48: a plane claiming 500 000 × 500 000 pixels must be refused without ever + /// allocating for it (before the cap this vec was never built; upper-bounding allocation + /// would have been a 7.5-gibabuffer or a panic instead of an error). + #[test] + fn an_oversized_canvas_is_refused_without_allocation() { + let f = Frame { + width: 500_000, + height: 500_000, + bit_depth: 8, + chroma: ChromaFormat::Yuv420, + y: vec![128], + cb: vec![128; 4], + cr: vec![128; 4], + y_stride: 0, + c_stride: 0, + }; + assert!(to_rgb16(&Planes::Picture(&f), Signal { matrix: 6, full_range: true }, false, Upsampling::Nearest).is_none()); + } }