security: bound codec allocation to what a hostile file declares (#2769)

* security: bound codec allocation to what a hostile file declares

A GIF's first image descriptor could name a frame up to four times the
canvas budget and be charged that allocation before any size check saw
it; PNG's compressed text chunks were expanded through an unbounded
inflate; JPEG handed hostile frame headers to the allocating decoders
(zune for the common cases, jpeg-decoder for CMYK/scaled/lossless) and
let an APP1 flood drive quadratic metadata reassembly. Each now refuses
on the same max_pixels budget the other decoders honour, with a
regression test per gate.

🤖 Generated with Codebuff
Co-Authored-By: Codebuff <noreply@codebuff.com>

* Fix clippy collapsible_if in DCT-scale size check

* Check jpeg-decoder header on scaled path; cap APP1 markers in decode_codes8

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Codebuff <noreply@codebuff.com>
Co-authored-by: Dylan Taylor <dylan@craftmaker.dev>
Co-authored-by: Claude Sonnet 5.5 <noreply@anthropic.com>

crates/codecs/src/jpeg.rs

@@ -17,6 +17,8 @@ pub(crate) struct Markers {
     pub sof: u8,
     /// Number of components in the first scan (< `components` for non-interleaved sequential files).
     pub first_scan_components: u8,
+    /// Number of APP1 segments: capped before the allocating decoders walk them again.
+    pub app1: u32,
     pub exif: Option<Vec<u8>>,
     /// Offset of the EXIF TIFF header within the file.
     pub exif_offset: usize,
@@ -83,6 +85,7 @@ pub(crate) fn parse_markers(b: &[u8]) -> Option<Markers> {
                 m.components = seg[5];
             }
             0xE1 => {
+                m.app1 = m.app1.saturating_add(1);
                 if seg.starts_with(b"Exif\0\0") && m.exif.is_none() {
                     m.exif = Some(seg[6..].to_vec());
                     m.exif_offset = start + 6;
@@ -121,6 +124,8 @@ pub(crate) fn decode(bytes: &[u8], opts: &DecodeOptions) -> Result<Decoded> {
     decode_with_fallback(bytes, opts, None)
 }
 
+const MAX_APP1: u32 = 128;
+
 /// Stored dimensions and EXIF orientation from the markers, without decoding the scan data. The
 /// file is refused, as a decode would refuse it, when it has no frame header, an unsupported frame
 /// type or component count, or when its markers don't run to an end-of-image marker after the
@@ -178,6 +183,11 @@ pub(crate) fn decode_with_fallback(bytes: &[u8], opts: &DecodeOptions, fallback:
         return Err(Error::Malformed(F, "no frame header".into()));
     }
     check_size(F, m.width as u64, m.height as u64, opts)?;
+    // The allocating decoders below parse the markers again, all of them: the count is capped so
+    // their quadratic/unbounded handling of APP1 metadata stays linear in file size.
+    if m.app1 > MAX_APP1 {
+        return Err(Error::Malformed(F, "too many APP1 markers".into()));
+    }
     let scale_to = opts.max_size.filter(|&(mw, mh)| mw > 0 && mh > 0 && (m.width >= mw.saturating_mul(2) || m.height >= mh.saturating_mul(2)));
     // zune-jpeg handles the common cases fastest; jpeg-decoder covers DCT scaling, CMYK/YCCK,
     // 12-bit, non-interleaved sequential scans (which zune-jpeg 0.5 mis-decodes with subsampling)
@@ -185,9 +195,9 @@ pub(crate) fn decode_with_fallback(bytes: &[u8], opts: &DecodeOptions, fallback:
     let lossless = m.sof == 0xC3;
     let non_interleaved = matches!(m.sof, 0xC0 | 0xC1) && m.first_scan_components < m.components;
     let raw = if m.components == 4 || scale_to.is_some() || m.precision > 8 || non_interleaved || lossless {
-        decode_jpeg_decoder(bytes, &m, scale_to)?
+        decode_jpeg_decoder(bytes, &m, scale_to, opts)?
     } else {
-        decode_zune(bytes, &m)?
+        decode_zune(bytes, &m, opts)?
     };
     // A raw container may carry the colour declaration of an otherwise untagged preview.
     // Apply it before linearization/resizing; existing JPEG metadata always wins.
@@ -196,7 +206,7 @@ pub(crate) fn decode_with_fallback(bytes: &[u8], opts: &DecodeOptions, fallback:
     finish(F, raw, meta, (m.width, m.height), opts)
 }
 
-fn decode_zune(bytes: &[u8], m: &Markers) -> Result<Raw> {
+fn decode_zune(bytes: &[u8], m: &Markers, budget: &DecodeOptions) -> Result<Raw> {
     use zune_core::bytestream::ZCursor;
     use zune_core::colorspace::ColorSpace;
     use zune_core::options::DecoderOptions;
@@ -207,6 +217,11 @@ fn decode_zune(bytes: &[u8], m: &Markers) -> Result<Raw> {
         ColorSpace::RGB
     });
     let mut d = zune_jpeg::JpegDecoder::new_with_options(ZCursor::new(bytes), opts);
+    // zune's `decode` allocates straight from the frame header it parses, with its own caps
+    // set in us: check that header against the same limits the other decoders honour first.
+    d.decode_headers().map_err(|e| Error::Malformed(F, e.to_string()))?;
+    let (zw, zh) = d.info().map(|i| (i.width as u64, i.height as u64)).ok_or_else(|| Error::Malformed(F, "no info".into()))?;
+    check_size(F, zw, zh, budget)?;
     let px = d.decode().map_err(|e| Error::Malformed(F, e.to_string()))?;
     let info = d.info().ok_or_else(|| Error::Malformed(F, "no info".into()))?;
     let (w, h) = (info.width as usize, info.height as usize);
@@ -217,10 +232,15 @@ fn decode_zune(bytes: &[u8], m: &Markers) -> Result<Raw> {
     Ok(Raw { width: w, height: h, model, alpha: false, premultiplied: false, buf: Buf::U8(px), bit_depth: 8 })
 }
 
-fn decode_jpeg_decoder(bytes: &[u8], m: &Markers, scale_to: Option<(u32, u32)>) -> Result<Raw> {
+fn decode_jpeg_decoder(bytes: &[u8], m: &Markers, scale_to: Option<(u32, u32)>, budget: &DecodeOptions) -> Result<Raw> {
     use jpeg_decoder::PixelFormat;
     let mut d = jpeg_decoder::Decoder::new(bytes);
     d.read_info().map_err(|e| Error::Malformed(F, e.to_string()))?;
+    // Its frame header is what a decode allocates from (progressive coefficient buffers follow the
+    // unscaled MCU grid): check it against the same limits the other decoders honour, before any
+    // DCT scale is requested.
+    let info = d.info().ok_or_else(|| Error::Malformed(F, "no info".into()))?;
+    check_size(F, info.width as u64, info.height as u64, budget)?;
     // `scale` is the DCT size, not a lossless resample: on a lossless frame it would cut the
     // output to the top-left of each plane. Those decode at full size; `finish` scales after.
     if let Some((mw, mh)) = scale_to.filter(|_| m.sof != 0xC3) {
@@ -369,11 +389,18 @@ pub(crate) fn decode_codes8(bytes: &[u8]) -> Result<(usize, usize, usize, Vec<u8
         return Err(Error::Malformed(F, "no frame header".into()));
     }
     check_size(F, m.width as u64, m.height as u64, &DecodeOptions::default())?;
+    if m.app1 > MAX_APP1 {
+        return Err(Error::Malformed(F, "too many APP1 markers".into()));
+    }
     if !matches!(m.components, 1 | 3) || m.precision != 8 {
         return Err(Error::Malformed(F, "expected an 8-bit greyscale or RGB image".into()));
     }
     let non_interleaved = matches!(m.sof, 0xC0 | 0xC1) && m.first_scan_components < m.components;
-    let raw = if non_interleaved { decode_jpeg_decoder(bytes, &m, None)? } else { decode_zune(bytes, &m)? };
+    let raw = if non_interleaved {
+        decode_jpeg_decoder(bytes, &m, None, &DecodeOptions::default())?
+    } else {
+        decode_zune(bytes, &m, &DecodeOptions::default())?
+    };
     let ch = raw.model.channels();
     match raw.buf {
         Buf::U8(v) if v.len() >= raw.width * raw.height * ch => Ok((raw.width, raw.height, ch, v)),
@@ -513,7 +540,7 @@ mod tests {
     fn lossless_grayscale_samples_are_native_endian_and_left_justified() {
         let bytes = lossless_jpeg(1, 8, 12);
         let m = parse_markers(&bytes).unwrap();
-        let raw = decode_jpeg_decoder(&bytes, &m, None).unwrap();
+        let raw = decode_jpeg_decoder(&bytes, &m, None, &DecodeOptions::default()).unwrap();
         let Buf::U16(v) = &raw.buf else { panic!("expected 16-bit samples") };
         assert_eq!(v, &vec![2048u16 << 4; 8], "0x0800 native-endian, left-justified");
         assert_eq!((raw.model, raw.bit_depth), (Model::Gray, 16));
@@ -525,7 +552,7 @@ mod tests {
     fn lossless_high_precision_rgb_is_16_bit() {
         let bytes = lossless_jpeg(3, 2, 12);
         let m = parse_markers(&bytes).unwrap();
-        let raw = decode_jpeg_decoder(&bytes, &m, None).unwrap();
+        let raw = decode_jpeg_decoder(&bytes, &m, None, &DecodeOptions::default()).unwrap();
         let Buf::U16(v) = &raw.buf else { panic!("expected 16-bit samples") };
         assert_eq!(v, &vec![2048u16 << 4; 6]);
         assert_eq!(raw.model, Model::Rgb);
@@ -536,7 +563,7 @@ mod tests {
     fn lossless_high_precision_cmyk_is_16_bit() {
         let bytes = lossless_jpeg(4, 2, 12);
         let m = parse_markers(&bytes).unwrap();
-        let raw = decode_jpeg_decoder(&bytes, &m, None).unwrap();
+        let raw = decode_jpeg_decoder(&bytes, &m, None, &DecodeOptions::default()).unwrap();
         let Buf::U16(v) = &raw.buf else { panic!("expected 16-bit samples") };
         assert_eq!(v, &vec![2048u16 << 4; 8]);
         assert_eq!((raw.model, raw.bit_depth), (Model::Cmyk, 16));
@@ -551,4 +578,77 @@ mod tests {
         assert_eq!((d.width, d.height), (8, 1));
         assert_eq!(d.bit_depth, 8);
     }
+
+    /// Rewrites an encoded file's frame-header dimensions: same tables and scan, a hostile
+    /// annotated size.
+    fn with_dims(bytes: &[u8], w: u16, h: u16) -> Vec<u8> {
+        let mut out = bytes.to_vec();
+        let mut p = 2usize;
+        while p + 1 < out.len() {
+            if out[p] != 0xFF {
+                p += 1;
+                continue;
+            }
+            match out[p + 1] {
+                0xC0..=0xCF if out[p + 1] != 0xC4 && out[p + 1] != 0xC8 && out[p + 1] != 0xCC => {
+                    out[p + 5..p + 7].copy_from_slice(&h.to_be_bytes());
+                    out[p + 7..p + 9].copy_from_slice(&w.to_be_bytes());
+                    return out;
+                }
+                0x01 | 0xD0..=0xD9 => p += 2,
+                0xFF => p += 1,
+                _ => {
+                    let Some(l) = out.get(p + 2..p + 4) else { return out };
+                    p += 2 + usize::from(u16::from_be_bytes([l[0], l[1]]));
+                }
+            }
+        }
+        out
+    }
+
+    /// A file claiming 65535×65535 must be refused for size before zune-jpeg allocates from
+    /// its frame header (4 GB+ of samples from roughly five hundred bytes of tables).
+    #[test]
+    fn zune_refuses_its_own_frame_header_over_the_budget() {
+        use crate::{ChromaSubsampling, EncodeImage, EncodeMeta, Samples, encode_jpeg};
+        let pixels = vec![128u8; 32 * 24];
+        let jpeg = encode_jpeg(&EncodeImage::new(32, 24, 1, Samples::U8(&pixels)), 90, ChromaSubsampling::S444, &EncodeMeta::default()).unwrap();
+        let hostile = with_dims(&jpeg, 65535, 65535);
+        assert_ne!(hostile, jpeg);
+        match crate::decode(&hostile, DecodeOptions::default()) {
+            Err(Error::TooLarge(w, h)) => assert_eq!((w, h), (65535, 65535)),
+            other => panic!("unexpected: {other:?}"),
+        }
+    }
+
+    /// The jpeg-decoder branch (a 4-component file) is checked the same way: its own
+    /// `read_info` result is what a full-size decode allocates from.
+    #[test]
+    fn jpeg_decoder_refuses_its_own_frame_header_over_the_budget() {
+        use crate::{ChromaSubsampling, EncodeImage, EncodeMeta, Samples, encode_jpeg};
+        let pixels = vec![128u8; 32 * 24 * 4];
+        let jpeg = encode_jpeg(&EncodeImage::new(32, 24, 4, Samples::U8(&pixels)), 90, ChromaSubsampling::S444, &EncodeMeta::default()).unwrap();
+        let hostile = with_dims(&jpeg, 65535, 65535);
+        match crate::decode(&hostile, DecodeOptions::default()) {
+            Err(Error::TooLarge(w, h)) => assert_eq!((w, h), (65535, 65535)),
+            other => panic!("unexpected: {other:?}"),
+        }
+    }
+
+    /// Extended XMP is reassembled from every APP1 fragment: an unbounded count is quadratic
+    /// memory in a marker walk, so the count is capped.
+    #[test]
+    fn an_app1_flood_is_refused_before_the_decoders_rewalk_it() {
+        let mut b = vec![0xFF, 0xD8];
+        // a 1-component SOF0: 8-bit, 2×2, component (1, 0x11, Q0)
+        b.extend_from_slice(&[0xFF, 0xC0, 0x00, 0x0B, 8, 0, 2, 0, 2, 1, 0x01, 0x11, 0x00]);
+        for _ in 0..200 {
+            b.extend_from_slice(&[0xFF, 0xE1, 0x00, 0x02]);
+        }
+        b.extend_from_slice(&[0xFF, 0xD9]);
+        match crate::decode(&b, DecodeOptions::default()) {
+            Err(Error::Malformed(F, m)) => assert!(m.contains("APP1"), "{m}"),
+            other => panic!("unexpected: {other:?}"),
+        }
+    }
 }

crates/codecs/src/other.rs

@@ -5,6 +5,9 @@ use crate::{DecodeOptions, Decoded, Error, Format, Result};
 use image::{DynamicImage, ImageDecoder};
 
 pub(crate) fn decode(bytes: &[u8], format: Format, opts: &DecodeOptions) -> Result<Decoded> {
+    if format == Format::Gif {
+        gated_first_frame(bytes, opts)?;
+    }
     let err = |e: image::ImageError| Error::Malformed(format, e.to_string());
     let cursor = std::io::Cursor::new(bytes);
     let (img, icc) = match format {
@@ -36,3 +39,83 @@ pub(crate) fn decode(bytes: &[u8], format: Format, opts: &DecodeOptions) -> Resu
     let raw = Raw { width: w as usize, height: h as usize, model, alpha, premultiplied: false, buf: Buf::U8(buf), bit_depth: 8 };
     finish(format, raw, Meta { icc, ..Default::default() }, (w, h), opts)
 }
+
+/// Bounds a GIF's first image descriptor: the decoder composites frames into the logical screen
+/// (bounded by [`check_size`] on the canvas) but allocates its own frame buffers at the frame's
+/// declared size, which a small file can inflate far past the canvas. The canvas check cannot
+/// see that size, so it is read from the block headers here.
+fn gated_first_frame(bytes: &[u8], opts: &DecodeOptions) -> Result<()> {
+    // The constants of the header grammar: signature, logical screen descriptor (with its global
+    // colour table), extension blocks (0x21, skipped by their sub-block chains) and image
+    // descriptors (0x2C).
+    const SCREEN: usize = 13;
+    if bytes.len() < SCREEN {
+        return Ok(()); // too broken for a frame; the decoder reports the damage itself
+    }
+    let gct_entries = 1usize << (u32::from(bytes[10] & 0x07) + 1); // at most 256
+    let gct_bytes = if (bytes[10] & 0x80) != 0 { gct_entries * 3 } else { 0 };
+    let mut p = SCREEN.saturating_add(gct_bytes);
+    while let Some(&kind) = bytes.get(p) {
+        match kind {
+            0x2C => {
+                // Image descriptor: two LE coordinates then two LE edges, after the separator.
+                let end = p.checked_add(10).filter(|&e| e <= bytes.len());
+                let Some(d) = end.and_then(|e| bytes.get(p + 1..e)) else {
+                    break; // cut short mid-descriptor: the decoder reports that
+                };
+                let (w, h) = (u16::from_le_bytes([d[4], d[5]]) as u64, u16::from_le_bytes([d[6], d[7]]) as u64);
+                return check_size(Format::Gif, w, h, opts);
+            }
+            0x21 => {
+                // Extension: a label byte, then <len,data> sub-blocks ended by a zero length.
+                p = p.saturating_add(2);
+                loop {
+                    let Some(&len) = bytes.get(p) else {
+                        return Ok(()); // ran past the data: decode reports the truncation
+                    };
+                    p = p.saturating_add(1);
+                    if len == 0 {
+                        break;
+                    }
+                    p = p.saturating_add(len as usize);
+                }
+            }
+            _ => p = p.saturating_add(1),
+        }
+    }
+    Ok(())
+}
+
+#[cfg(test)]
+mod tests {
+    use crate::{DecodeOptions, Error};
+
+    fn gif(blocks: &[u8]) -> Vec<u8> {
+        let mut b = b"GIF89a".to_vec();
+        // logical screen: 2×2, no global colour table, background 0, aspect 0
+        b.extend_from_slice(&[2, 0, 2, 0, 0x00, 0, 0]);
+        b.extend_from_slice(blocks);
+        b
+    }
+
+    /// `image`'s GIF decoder allocates its frame buffer at the image descriptor's declared
+    /// size, which the canvas bound ([`crate::decode`]'s `max_pixels` on the logical screen)
+    /// does not cover. A 20-byte file used to buy a 4 GB allocation before the gate.
+    #[test]
+    fn a_first_frame_over_the_bound_is_refused_without_allocating_it() {
+        let bytes = gif(&[0x2C, 0, 0, 0, 0, 0xFF, 0xFF, 0xFF, 0xFF, 0, 0x00]);
+        match crate::decode(&bytes, DecodeOptions::default()) {
+            Err(Error::TooLarge(w, h)) => assert_eq!((w, h), (65535, 65535)),
+            other => panic!("unexpected: {other:?}"),
+        }
+    }
+
+    /// A frame within the bound still reaches the decoder (which reports the truncated data);
+    /// the gate may only refuse on size.
+    #[test]
+    fn a_first_frame_within_the_bound_is_not_refused() {
+        let bytes = gif(&[0x2C, 0, 0, 0, 0, 1, 0, 1, 0, 0, 0x00]);
+        let r = crate::decode(&bytes, DecodeOptions::default());
+        assert!(matches!(r, Err(Error::Malformed(..)) | Err(Error::TooLarge(..))), "{r:?}");
+    }
+}

crates/codecs/src/png_codec.rs

@@ -7,6 +7,10 @@ use lightcraft_color::{Mat3, RgbSpace, Xy};
 
 const F: Format = Format::Png;
 
+/// Expansion cap for a compressed text chunk's payload: real XMP documents are a few hundred
+/// kilobytes, so anything past this is hostile input, not metadata.
+const MAX_TEXT_CHUNK: usize = 4 << 20;
+
 fn err(e: impl std::fmt::Display) -> Error {
     Error::Malformed(F, e.to_string())
 }
@@ -42,11 +46,18 @@ pub(crate) fn decode(bytes: &[u8], opts: &DecodeOptions) -> Result<Decoded> {
     };
     let raw = Raw { width: out.width as usize, height: out.height as usize, model, alpha, premultiplied: false, buf: samples, bit_depth: depth };
 
+    // png's `get_text()` inflates a compressed chunk with no bound of its own: a tiny
+    // iTXt can claim an XMP document of arbitrary size. Expand through the bounded path
+    // instead, and drop any XMP bigger than the cap.
     let xmp = info
         .utf8_text
         .iter()
         .find(|t| t.keyword == "XML:com.adobe.xmp")
-        .and_then(|t| t.get_text().ok())
+        .and_then(|t| {
+            let mut t = t.clone();
+            t.decompress_text_with_limit(MAX_TEXT_CHUNK).ok()?;
+            t.get_text().ok()
+        })
         .or_else(|| info.uncompressed_latin1_text.iter().find(|t| t.keyword == "XML:com.adobe.xmp").map(|t| t.text.clone()));
     let exif = info.exif_metadata.as_ref().map(|e| {
         let e = e.to_vec();
@@ -168,3 +179,55 @@ fn rgb_to_xyz_d50_checked(space: &RgbSpace) -> Option<Mat3> {
     }
     Some(crate::space::rgb_to_xyz_d50(space))
 }
+
+#[cfg(test)]
+mod tests {
+    use crate::DecodeOptions;
+
+    /// A compressed text chunk is expanded by `png` through our own cap: an XMP document
+    /// bigger than it is dropped, not inflated. Compressing an 8 MiB string is a few
+    /// kilobytes — the reverse of what the unbounded `get_text()` path allowed.
+    #[test]
+    fn compressed_xmp_over_the_cap_is_dropped() {
+        let out = png_with_compressed_xmp(&"x".repeat(8 << 20));
+        let d = crate::decode(&out, DecodeOptions::default()).unwrap();
+        assert!(d.xmp.is_none());
+        assert_eq!((d.width, d.height), (1, 1));
+    }
+
+    /// XMP within the cap is still read, compressed or not.
+    #[test]
+    fn compressed_xmp_under_the_cap_is_read() {
+        let out = png_with_compressed_xmp(&"<?xpacket?><xmp/>".repeat(100));
+        let d = crate::decode(&out, DecodeOptions::default()).unwrap();
+        assert!(d.xmp.as_deref().is_some_and(|x| x.contains("xmp/")));
+    }
+
+    /// A little-endian PNG with one compressed iTXt chunk carrying `payload` as XMP.
+    fn png_with_compressed_xmp(payload: &str) -> Vec<u8> {
+        use png::text_metadata::{EncodableTextChunk, ITXtChunk};
+        let mut base = Vec::new();
+        {
+            let mut enc = png::Encoder::new(&mut base, 1, 1);
+            enc.set_color(png::ColorType::Grayscale);
+            enc.set_depth(png::BitDepth::Eight);
+            let mut w = enc.write_header().unwrap();
+            w.write_image_data(&[0x7F]).unwrap();
+            w.finish().unwrap();
+        }
+        // `ITXtChunk::encode` writes the complete chunk (length, type, payload, CRC), with
+        // the text compressed.
+        let mut chunk = Vec::new();
+        let mut xmp = ITXtChunk::new("XML:com.adobe.xmp", payload);
+        xmp.compressed = true;
+        xmp.compress_text().unwrap();
+        xmp.encode(&mut chunk).unwrap();
+        // Text chunks after IDAT land in `info` once the stream is finished: insert ours
+        // before the trailing IEND (a zero-length chunk is exactly 12 bytes).
+        let split = base.len() - 12;
+        let mut out = base[..split].to_vec();
+        out.extend_from_slice(&chunk);
+        out.extend_from_slice(&base[split..]);
+        out
+    }
+}