diff --git a/crates/codecs/src/codecs/jpeg.rs b/crates/codecs/src/codecs/jpeg.rs index 26a47c13d6..393f8e9512 100644 --- a/crates/codecs/src/codecs/jpeg.rs +++ b/crates/codecs/src/codecs/jpeg.rs @@ -137,32 +137,39 @@ enum DataEnd { /// Walks the marker segments (skipped by their length) and the entropy-coded scan data /// (skipped up to the next marker that isn't a stuffed `FF 00`, a restart or a fill byte), /// so an embedded thumbnail's end marker or data after the end of the image don't count. -fn data_end(b: &[u8]) -> DataEnd { +/// Also returns how many scan-data bytes the walk crossed: the truncated-input budget is +/// measured on these, not on the file length, which APP padding can inflate without +/// adding image data. +fn data_end(b: &[u8]) -> (DataEnd, u64) { // Where the bytes run out: inside the image data once a scan has held some. - let cut = |scan_data: bool| if scan_data { DataEnd::Truncated } else { DataEnd::Empty }; + let cut = |scan_data: bool, scanned: u64| (if scan_data { DataEnd::Truncated } else { DataEnd::Empty }, scanned); let mut i = 2; // past SOI let mut scan_data = false; + let mut scanned = 0u64; loop { - let Some(&byte) = b.get(i) else { return cut(scan_data) }; + let Some(&byte) = b.get(i) else { return cut(scan_data, scanned) }; if byte != 0xFF { // Not where a marker should be: too odd to judge. - return DataEnd::Complete; + return (DataEnd::Complete, scanned); } - let Some(&code) = b.get(i + 1) else { return cut(scan_data) }; + let Some(&code) = b.get(i + 1) else { return cut(scan_data, scanned) }; match code { - 0xD9 => return DataEnd::Complete, + 0xD9 => return (DataEnd::Complete, scanned), 0xFF => i += 1, 0x01 | 0xD0..=0xD8 => i += 2, _ => { let Some(len) = b.get(i + 2..i + 4).and_then(|s| <[u8; 2]>::try_from(s).ok()).map(u16::from_be_bytes) else { - return cut(scan_data); + return cut(scan_data, scanned); }; i = i.saturating_add(2 + usize::from(len)); if code == 0xDA { let start = i; loop { let Some(p) = b.get(i..).and_then(|r| r.iter().position(|&v| v == 0xFF)) else { - return cut(scan_data || b.len() > start); + // EOF inside this scan: everything from the scan start to the end + // of the file is scan data (inside a declared segment it may not + // be, so only count what remains). + return cut(scan_data || b.len() > start, scanned + u64::try_from(b.len().saturating_sub(start)).unwrap_or(0)); }; i += p; match b.get(i + 1) { @@ -172,6 +179,7 @@ fn data_end(b: &[u8]) -> DataEnd { } } scan_data |= i > start; + scanned += u64::try_from(i - start).unwrap_or(0); } } } @@ -182,12 +190,12 @@ fn err(e: impl std::fmt::Display) -> CodecError { CodecError::malformed(F, e) } -/// Most pixels one byte of a truncated file may account for. Even a flat image costs a few bits -/// per 8×8 block, and truncated photographs are far above this. +/// Most pixels one byte of truncated scan data may account for. Even a flat image costs a few +/// bits per 8×8 block, and truncated photographs are far above this. const MAX_TRUNCATED_PIXELS_PER_BYTE: u64 = 2048; pub(crate) fn decode(bytes: &[u8], limits: &Limits) -> Result { - let end = data_end(bytes); + let (end, scan_bytes) = data_end(bytes); if end == DataEnd::Empty { return Err(err("the file ends before any image data")); } @@ -209,8 +217,10 @@ pub(crate) fn decode(bytes: &[u8], limits: &Limits) -> Result }; limits.check(w, h, layout, SampleType::U8)?; // The backend fills whatever the scan no longer describes, so a short scan under a large - // frame is a gigabyte of synthesized output from a few kilobytes of file. - if end == DataEnd::Truncated && u64::from(w) * u64::from(h) / MAX_TRUNCATED_PIXELS_PER_BYTE > bytes.len() as u64 { + // frame is a gigabyte of synthesized output from a few kilobytes of file. The budget is + // measured on the scan data itself: APP padding can inflate `bytes.len()` arbitrarily + // without describing one more pixel. + if end == DataEnd::Truncated && u64::from(w) * u64::from(h) / MAX_TRUNCATED_PIXELS_PER_BYTE > scan_bytes { return Err(err("the truncated scan is too short for the frame it declares")); } dec.set_options(dec.options().jpeg_set_out_colorspace(out_cs)); @@ -451,7 +461,7 @@ mod tests { let img = Image::from_raw(16, 16, ChannelLayout::Rgb, SampleType::U8, vec![128; 16 * 16 * 3]).unwrap(); let full = crate::encode(&img, Format::Jpeg, &EncodeOptions::default()).unwrap(); let cut = &full[..full.len() - 4]; - assert_eq!(data_end(cut), DataEnd::Truncated); + assert_eq!(data_end(cut).0, DataEnd::Truncated); assert!(crate::decode_as(Format::Jpeg, cut).is_ok()); let sof = cut.windows(2).position(|w| w == [0xFF, 0xC0] || w == [0xFF, 0xC2]).unwrap(); let mut huge = cut.to_vec(); @@ -460,6 +470,36 @@ mod tests { assert!(e.to_string().contains("too short"), "{e}"); } + /// The truncated-scan budget counts scan data, not the file length: APP padding before the + /// window cannot admit an oversized output. The same padded file with its real size still + /// decodes (with the truncation warning). + #[test] + fn app_padding_cannot_admit_an_oversized_truncated_frame() { + let img = Image::from_raw(16, 16, ChannelLayout::Rgb, SampleType::U8, vec![128; 16 * 16 * 3]).unwrap(); + let full = crate::encode(&img, Format::Jpeg, &EncodeOptions::default()).unwrap(); + let cut = &full[..full.len() - 4]; + // Four full-length COMMENT segments (~256 KiB) stand where APP padding would; they are + // complete, so the file otherwise parses normally. + let fill = [b'p'; 65533]; + let mut padded = cut[..2].to_vec(); + for _ in 0..4 { + padded.extend_from_slice(&[0xFF, 0xFE]); + padded.extend_from_slice(&0xFFFFu16.to_be_bytes()); + padded.extend_from_slice(&fill); + } + padded.extend_from_slice(&cut[2..]); + assert_eq!(data_end(&padded).0, DataEnd::Truncated); + // Well above the frame's pixels / budget, yet far above the scan data: the old + // file-length bound admitted this frame. + assert!(padded.len() > (u64::from(20_000) * u64::from(20_000) / MAX_TRUNCATED_PIXELS_PER_BYTE) as usize, "padding too small"); + assert!(crate::decode_as(Format::Jpeg, &padded).is_ok()); + let sof = padded.windows(2).position(|w| w == [0xFF, 0xC0] || w == [0xFF, 0xC2]).unwrap(); + let mut huge = padded.clone(); + huge[sof + 5..sof + 9].copy_from_slice(&[0x4E, 0x20, 0x4E, 0x20]); + let e = crate::decode_as(Format::Jpeg, &huge).unwrap_err(); + assert!(e.to_string().contains("too short"), "{e}"); + } + /// CMYK JPEG output must omit JFIF while retaining Adobe's CMYK marker. #[test] fn cmyk_jpeg_omits_jfif_but_keeps_adobe_cmyk_marker() { diff --git a/crates/codecs/src/options.rs b/crates/codecs/src/options.rs index 1f7b249722..2279504413 100644 --- a/crates/codecs/src/options.rs +++ b/crates/codecs/src/options.rs @@ -14,7 +14,9 @@ pub struct Limits { /// Maximum bytes for the decoded pixel buffer (and passed on as the /// allocation budget to underlying decoders). /// PNG also applies this to the aggregate decoded text/XMP UTF-8 bytes, - /// including keywords, separately from pixels. This is not a total RSS cap. + /// including keywords, separately from pixels. Animated WebP charges + /// every retained frame's full pixel copy plus its own copy of the + /// file's ICC profile against this budget. This is not a total RSS cap. pub max_alloc: u64, }