diff --git a/contributors/people.toml b/contributors/people.toml index ecc935ebe0..33d2165da8 100644 --- a/contributors/people.toml +++ b/contributors/people.toml @@ -22,6 +22,9 @@ real_name = "Brandon Thomas" display_name = "Brandon Thomas" sync_github_name = false +[people.Streetblock] +sync_github_name = true + [people.switch87] real_name = "Gert Pellin" display_name = "Gert Pellin" diff --git a/crates/codecs/README.md b/crates/codecs/README.md index 9c467190f4..3e2d7d3e37 100644 --- a/crates/codecs/README.md +++ b/crates/codecs/README.md @@ -154,6 +154,8 @@ the same. decoding. Header dimensions are checked before the pixel buffer is allocated, and the budget is also passed to the underlying decoders. A violation returns `CodecError::LimitExceeded`. The defaults are 262144 px per side, 2^30 pixels and 8 GiB (2 GiB on 32-bit targets such as wasm). +PNG also bounds the aggregate decoded text and XMP (UTF-8 keywords and values) by `max_alloc`, +including chunks after IDAT. This budget is separate from pixels, not a total process-memory cap. ## Tests diff --git a/crates/codecs/src/codecs/png.rs b/crates/codecs/src/codecs/png.rs index 80d47cc998..97e63ac590 100644 --- a/crates/codecs/src/codecs/png.rs +++ b/crates/codecs/src/codecs/png.rs @@ -18,8 +18,12 @@ fn err(e: impl std::fmt::Display) -> CodecError { } pub(crate) fn decode(bytes: &[u8], limits: &Limits) -> Result { - let mut decoder = png::Decoder::new_with_limits(Cursor::new(bytes), png::Limits { bytes: limits.alloc_usize() }); + let mut input = Cursor::new(bytes); + let mut decoder = png::Decoder::new_with_limits(&mut input, png::Limits { bytes: limits.alloc_usize() }); decoder.set_transformations(png::Transformations::EXPAND); + // png's lazy text get_text() inflates without consulting its allocation limit. + // Read text separately below, without retaining compressed copies in the decoder. + decoder.set_ignore_text_chunk(true); { let info = decoder.read_header_info().map_err(map_png_err)?; let (w, h) = info.size(); @@ -46,9 +50,11 @@ pub(crate) fn decode(bytes: &[u8], limits: &Limits) -> Result let mut buf = vec![0u8; size]; let out = reader.next_frame(&mut buf).map_err(map_png_err)?; buf.truncate(out.buffer_size()); - // Collect chunks after IDAT (text may live there). Errors here are - // tolerated: pixel data is already complete. - let _ = reader.finish(); + // Collect ancillary chunks after IDAT. Tolerate malformed tails once pixels + // are complete, but never swallow an allocation-limit failure. + if let Err(e @ png::DecodingError::LimitsExceeded) = reader.finish() { + return Err(map_png_err(e)); + } if sample == SampleType::U16 { for c in buf.as_chunks_mut::<2>().0 { let v = u16::from_be_bytes([c[0], c[1]]); @@ -66,23 +72,10 @@ pub(crate) fn decode(bytes: &[u8], limits: &Limits) -> Result { meta.dpi = Some((d.xppu as f32 * METERS_PER_INCH, d.yppu as f32 * METERS_PER_INCH)); } - for t in &info.uncompressed_latin1_text { - meta.text.push((t.keyword.clone(), t.text.clone())); - } - for t in &info.compressed_latin1_text { - if let Ok(s) = t.get_text() { - meta.text.push((t.keyword.clone(), s)); - } - } - for t in &info.utf8_text { - if let Ok(s) = t.get_text() { - if t.keyword == XMP_KEYWORD { - meta.xmp = Some(s); - } else { - meta.text.push((t.keyword.clone(), s)); - } - } - } + drop(reader); + let end = usize::try_from(input.position()).map_err(|_| err("invalid PNG reader position"))?; + let consumed = bytes.get(..end).ok_or_else(|| err("invalid PNG reader position"))?; + read_text_metadata(consumed, limits.alloc_usize(), &mut meta)?; img.meta = meta; if images > 1 { img.warnings.push(DecodeWarning::MoreFrames { total: u32::try_from(images).ok() }); @@ -97,6 +90,137 @@ fn map_png_err(e: png::DecodingError) -> CodecError { } } +fn text_limit() -> CodecError { + CodecError::LimitExceeded("PNG decoded text metadata exceeds max_alloc".into()) +} + +fn split_nul(bytes: &[u8]) -> Option<(&[u8], &[u8])> { + let at = bytes.iter().position(|&b| b == 0)?; + Some((bytes.get(..at)?, bytes.get(at.checked_add(1)?..)?)) +} + +fn latin1_len(bytes: &[u8]) -> Option { + bytes.len().checked_add(bytes.iter().filter(|&&b| b >= 128).count()) +} + +/// Read only CRC-valid text chunks, including those after IDAT, with one shared +/// budget for decoded UTF-8 keywords and values (including overwritten XMP). +/// Invalid ancillary text stays optional, as it was with png's text decoder. +fn read_text_metadata(bytes: &[u8], mut remaining: usize, meta: &mut Metadata) -> Result<(), CodecError> { + let Some(mut rest) = bytes.get(8..) else { return Ok(()) }; + let (mut plain, mut compressed, mut international) = (Vec::new(), Vec::new(), Vec::new()); + while let Some((length, tail)) = rest.split_first_chunk::<4>() { + let Some((kind, tail)) = tail.split_first_chunk::<4>() else { break }; + let length = u32::from_be_bytes(*length); + // PNG chunk lengths are at most 2^31-1. Broken tails remain tolerated. + if length > i32::MAX as u32 { + break; + } + let Ok(length) = usize::try_from(length) else { break }; + let Some(data) = tail.get(..length) else { break }; + let Some((checksum, next)) = tail.get(length..).and_then(|t| t.split_first_chunk::<4>()) else { break }; + rest = next; + if kind == b"IEND" { + break; + } + if !matches!(kind, b"tEXt" | b"zTXt" | b"iTXt") { + continue; + } + let mut crc = flate2::Crc::new(); + crc.update(kind); + crc.update(data); + if crc.sum() != u32::from_be_bytes(*checksum) { + continue; + } + let Some((keyword, value)) = split_nul(data) else { continue }; + if keyword.is_empty() || keyword.len() > 79 { + continue; + } + let (value, deflated, latin1) = match kind { + b"tEXt" => (value, false, true), + b"zTXt" => { + let Some((&0, value)) = value.split_first() else { continue }; + (value, true, true) + } + _ => { + let Some(([flag, method], value)) = value.split_first_chunk::<2>() else { continue }; + if *flag > 1 || (*flag == 1 && *method != 0) { + continue; + } + let Some((language, value)) = split_nul(value) else { continue }; + let Some((translated, value)) = split_nul(value) else { continue }; + if !language.is_ascii() || std::str::from_utf8(translated).is_err() { + continue; + } + (value, *flag == 1, false) + } + }; + let keyword_len = latin1_len(keyword).ok_or_else(text_limit)?; + let limit = remaining.saturating_sub(keyword_len); + let Some(text) = bounded_text(value, deflated, latin1, limit)? else { continue }; + remaining = remaining.checked_sub(keyword_len).and_then(|left| left.checked_sub(text.len())).ok_or_else(text_limit)?; + let keyword: String = keyword.iter().map(|&b| char::from(b)).collect(); + match kind { + b"tEXt" => plain.push((keyword, text)), + b"zTXt" => compressed.push((keyword, text)), + _ if keyword == XMP_KEYWORD => meta.xmp = Some(text), + _ => international.push((keyword, text)), + } + } + // Preserve the previous decoder's grouping of text entries by chunk type. + meta.text.extend(plain); + meta.text.extend(compressed); + meta.text.extend(international); + Ok(()) +} + +/// No input-sized allocation until its decoded size fits. Inflation uses a +/// fixed scratch buffer and checks each batch before growing the output. +fn bounded_text(input: &[u8], compressed: bool, latin1: bool, limit: usize) -> Result, CodecError> { + let mut output = Vec::new(); + let mut append = |bytes: &[u8]| -> Result<(), CodecError> { + let added = if latin1 { latin1_len(bytes).ok_or_else(text_limit)? } else { bytes.len() }; + if output.len().checked_add(added).is_none_or(|len| len > limit) { + return Err(text_limit()); + } + output.try_reserve(added).map_err(|_| text_limit())?; + if latin1 { + // Every Latin-1 code point fits in at most two UTF-8 bytes. + let mut utf8 = [0; 2]; + for &b in bytes { + output.extend_from_slice(char::from(b).encode_utf8(&mut utf8).as_bytes()); + } + } else { + output.extend_from_slice(bytes); + } + Ok(()) + }; + if compressed { + let mut decoder = flate2::Decompress::new(true); + let mut scratch = [0; 8192]; + loop { + let start_in = decoder.total_in(); + let start_out = decoder.total_out(); + let Some(tail) = usize::try_from(start_in).ok().and_then(|at| input.get(at..)) else { return Ok(None) }; + let Ok(status) = decoder.decompress(tail, &mut scratch, flate2::FlushDecompress::None) else { return Ok(None) }; + let Some(batch) = usize::try_from(decoder.total_out() - start_out).ok().and_then(|len| scratch.get(..len)) else { return Ok(None) }; + append(batch)?; + if status == flate2::Status::StreamEnd { + break; + } + if start_in == decoder.total_in() && start_out == decoder.total_out() { + return Ok(None); // Truncated stream: never keep partial text. + } + } + } else { + if !latin1 && std::str::from_utf8(input).is_err() { + return Ok(None); + } + append(input)?; + } + Ok(String::from_utf8(output).ok()) +} + fn is_latin1_keyword(k: &str) -> bool { !k.is_empty() && k.len() <= 79 && k.chars().all(|c| (' '..='~').contains(&c) || ('\u{a1}'..='\u{ff}').contains(&c)) } diff --git a/crates/codecs/src/options.rs b/crates/codecs/src/options.rs index a97eb15d9e..79b288f34c 100644 --- a/crates/codecs/src/options.rs +++ b/crates/codecs/src/options.rs @@ -3,8 +3,8 @@ use crate::error::CodecError; use crate::image::{ChannelLayout, SampleType}; -/// Decompression-bomb guards. All checks happen on header values *before* -/// the pixel buffer is allocated. +/// Decompression-bomb guards. Dimension checks precede pixel allocation; +/// metadata decoding checks its budget before growing decoded output. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub struct Limits { pub max_width: u32, @@ -13,6 +13,8 @@ pub struct Limits { pub max_pixels: u64, /// 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. pub max_alloc: u64, } diff --git a/crates/codecs/tests/limits.rs b/crates/codecs/tests/limits.rs index e808a9cae7..b3d89d2e76 100644 --- a/crates/codecs/tests/limits.rs +++ b/crates/codecs/tests/limits.rs @@ -93,6 +93,231 @@ fn png_bomb_header_rejected_before_alloc() { assert!(is_limit(decode(&b))); } +const PNG_TEXT_BUDGET: usize = 64 << 10; + +fn png_text_decode(bytes: &[u8], max_alloc: usize) -> Result { + decode_with(bytes, &opts(Limits { max_alloc: max_alloc as u64, ..Limits::default() })) +} + +fn zlib(text: &[u8]) -> Vec { + use std::io::Write; + let mut encoder = flate2::write::ZlibEncoder::new(Vec::new(), flate2::Compression::fast()); + encoder.write_all(text).unwrap(); + encoder.finish().unwrap() +} + +// zTXt and iTXt payloads supplied here are already compressed. +fn png_text_stream(ty: &[u8; 4], keyword: &[u8], payload: &[u8]) -> Vec { + let mut data = keyword.to_vec(); + data.push(0); + match ty { + b"tEXt" => {} + b"zTXt" => data.push(0), + b"iTXt" => data.extend_from_slice(&[1, 0, 0, 0]), + _ => panic!("unexpected text chunk"), + } + data.extend_from_slice(payload); + png_chunk(ty, &data) +} + +fn png_text(ty: &[u8; 4], keyword: &[u8], text: &[u8]) -> Vec { + if ty == b"tEXt" { png_text_stream(ty, keyword, text) } else { png_text_stream(ty, keyword, &zlib(text)) } +} + +fn png_with_text(before_idat: &[Vec], after_idat: &[Vec]) -> Vec { + let mut image = Vec::new(); + { + let mut encoder = png::Encoder::new(&mut image, 1, 1); + encoder.set_color(png::ColorType::Grayscale); + encoder.set_depth(png::BitDepth::Eight); + let mut writer = encoder.write_header().unwrap(); + writer.write_image_data(&[127]).unwrap(); + writer.finish().unwrap(); + } + let mut at = 8; + let mut idat = None; + let iend = loop { + let len = u32::from_be_bytes(image[at..at + 4].try_into().unwrap()) as usize; + let ty = &image[at + 4..at + 8]; + if ty == b"IDAT" && idat.is_none() { + idat = Some(at); + } + if ty == b"IEND" { + break at; + } + at += len + 12; + }; + let idat = idat.unwrap(); + let mut output = image[..idat].to_vec(); + for chunk in before_idat { + output.extend_from_slice(chunk); + } + output.extend_from_slice(&image[idat..iend]); + for chunk in after_idat { + output.extend_from_slice(chunk); + } + output.extend_from_slice(&image[iend..]); + output +} + +#[test] +fn png_compressed_text_bombs_obey_limits_before_and_after_idat() { + let bomb = vec![b'A'; PNG_TEXT_BUDGET * 8]; + for ty in [b"zTXt", b"iTXt"] { + for after_idat in [false, true] { + let chunks = [png_text(ty, b"Comment", &bomb)]; + let bytes = if after_idat { png_with_text(&[], &chunks) } else { png_with_text(&chunks, &[]) }; + assert!(bytes.len() < PNG_TEXT_BUDGET / 2, "fixture must fit comfortably within the compressed-input budget"); + assert!(is_limit(png_text_decode(&bytes, PNG_TEXT_BUDGET)), "{ty:?}, after IDAT={after_idat}"); + } + } +} + +#[test] +fn png_text_chunks_share_an_aggregate_budget() { + for text_len in [(PNG_TEXT_BUDGET - 3) / 3, PNG_TEXT_BUDGET / 3] { + let text = vec![b'A'; text_len]; + let before = [png_text(b"tEXt", b"A", &text), png_text(b"zTXt", b"B", &text)]; + let after = [png_text(b"iTXt", b"C", &text)]; + let result = png_text_decode(&png_with_text(&before, &after), PNG_TEXT_BUDGET); + if 3 * (text_len + 1) <= PNG_TEXT_BUDGET { + let image = result.unwrap(); + assert_eq!(image.meta.text.len(), 3); + assert!(image.meta.text.iter().all(|(_, value)| value.len() == text_len)); + } else { + assert!(is_limit(result), "chunks fitting individually must still share a budget"); + } + } +} + +#[test] +fn png_text_accepts_the_exact_utf8_budget() { + for ty in [b"tEXt", b"zTXt", b"iTXt"] { + let text = vec![b'A'; PNG_TEXT_BUDGET - 1]; + let bytes = png_with_text(&[png_text(ty, b"K", &text)], &[]); + let image = png_text_decode(&bytes, PNG_TEXT_BUDGET).unwrap(); + assert_eq!(image.meta.text, vec![("K".into(), String::from_utf8(text).unwrap())]); + assert!(is_limit(png_text_decode(&bytes, PNG_TEXT_BUDGET - 1)), "{ty:?}: keyword and text both count"); + } +} + +#[test] +fn png_plain_itxt_obeys_the_exact_budget_and_validates_fields() { + let mut payload = b"K\0\0\0en\0Keyword\0".to_vec(); + payload.extend_from_slice(&vec![b'A'; PNG_TEXT_BUDGET - 1]); + let bytes = png_with_text(&[], &[png_chunk(b"iTXt", &payload)]); + let image = png_text_decode(&bytes, PNG_TEXT_BUDGET).unwrap(); + assert_eq!(image.meta.text, vec![("K".into(), "A".repeat(PNG_TEXT_BUDGET - 1))]); + assert!(is_limit(png_text_decode(&bytes, PNG_TEXT_BUDGET - 1))); + + let invalid_fields: &[&[u8]] = &[ + b"K\0\0\0\0\0\xFF", // Invalid text UTF-8. + b"K\0\0\0\xFF\0\0text", // Non-ASCII language tag. + b"K\0\0\0\0\xFF\0text", // Invalid translated-keyword UTF-8. + b"K\0\x02\0\0\0text", // Unknown compression flag. + b"K\0\x01\x01\0\0text", // Unknown compression method for compressed text. + b"K\0\0", // Missing compression method. + b"K\0\0\0\0text", // Missing translated-keyword separator. + ]; + for fields in invalid_fields { + let bytes = png_with_text(&[png_chunk(b"iTXt", fields)], &[png_text(b"tEXt", b"Good", b"retained")]); + let image = png_text_decode(&bytes, PNG_TEXT_BUDGET).unwrap(); + assert_eq!(image.meta.text, vec![("Good".into(), "retained".into())]); + } +} + +#[test] +fn png_latin1_expansion_counts_keyword_and_text_utf8_bytes() { + let text_len = (PNG_TEXT_BUDGET - 2) / 2; + for ty in [b"tEXt", b"zTXt", b"iTXt"] { + let text = if ty == b"iTXt" { "é".repeat(text_len).into_bytes() } else { vec![0xE9; text_len] }; + let bytes = png_with_text(&[], &[png_text(ty, &[0xE9], &text)]); + let image = png_text_decode(&bytes, PNG_TEXT_BUDGET).unwrap(); + assert_eq!(image.meta.text, vec![("é".into(), "é".repeat(text_len))]); + assert!(is_limit(png_text_decode(&bytes, PNG_TEXT_BUDGET - 1)), "{ty:?}: Latin-1 expands when stored as UTF-8"); + } +} + +#[test] +fn png_duplicate_xmp_chunks_each_consume_budget() { + const KEYWORD: &[u8] = b"XML:com.adobe.xmp"; + let text_len = (PNG_TEXT_BUDGET - 2 * KEYWORD.len()) / 2; + for extra in [0, 1] { + let first = png_text(b"iTXt", KEYWORD, &vec![b'A'; text_len + extra]); + let last_text = vec![b'B'; text_len]; + let last = png_text(b"iTXt", KEYWORD, &last_text); + let result = png_text_decode(&png_with_text(&[first], &[last]), PNG_TEXT_BUDGET); + if extra == 0 { + let image = result.unwrap(); + assert_eq!(image.meta.xmp, Some(String::from_utf8(last_text).unwrap())); + assert!(image.meta.text.is_empty()); + } else { + assert!(is_limit(result), "replacing an XMP packet must not reset its budget charge"); + } + } +} + +#[test] +fn png_corrupt_text_is_still_skipped() { + let valid = zlib(b"discard"); + let mut bad_header = valid.clone(); + bad_header[0] = 0; + let truncated = valid[..valid.len() - 4].to_vec(); + for ty in [b"zTXt", b"iTXt"] { + for stream in [&bad_header, &truncated] { + let before = [png_text_stream(ty, b"Bad", stream)]; + let after = [png_text(b"tEXt", b"Good", b"retained")]; + let image = png_text_decode(&png_with_text(&before, &after), PNG_TEXT_BUDGET).unwrap(); + assert_eq!(image.meta.text, vec![("Good".into(), "retained".into())]); + } + } + let invalid_utf8 = png_text(b"iTXt", b"Bad", &[0xFF]); + let image = png_text_decode(&png_with_text(&[], &[invalid_utf8]), PNG_TEXT_BUDGET).unwrap(); + assert!(image.meta.text.is_empty()); +} + +#[test] +fn png_text_with_bad_crc_and_text_after_iend_are_ignored() { + let bomb = vec![b'A'; PNG_TEXT_BUDGET * 8]; + for ty in [b"zTXt", b"iTXt"] { + let valid = png_text(ty, b"Comment", &bomb); + let mut bad_crc = valid.clone(); + *bad_crc.last_mut().unwrap() ^= 1; + for after_idat in [false, true] { + let bytes = if after_idat { png_with_text(&[], &[bad_crc.clone()]) } else { png_with_text(&[bad_crc.clone()], &[]) }; + let image = png_text_decode(&bytes, PNG_TEXT_BUDGET).unwrap(); + assert!(image.meta.text.is_empty()); + } + let mut bytes = png_with_text(&[], &[]); + bytes.extend_from_slice(&valid); + let image = png_text_decode(&bytes, PNG_TEXT_BUDGET).unwrap(); + assert!(image.meta.text.is_empty()); + } +} + +#[test] +fn png_malformed_chunk_tails_remain_tolerated_after_pixels() { + let base = png_with_text(&[png_text(b"tEXt", b"Good", b"retained")], &[png_chunk(b"vpAg", &[])]); + let iend = base.len() - 12; + let tails: &[&[u8]] = &[&[], b"\x00\x00\x00", b"\x00\x00\x00\x10iTXtK\0\0", b"\xFF\xFF\xFF\xFFiTXt"]; + for tail in tails { + let mut bytes = base[..iend].to_vec(); + bytes.extend_from_slice(tail); + let image = png_text_decode(&bytes, PNG_TEXT_BUDGET).unwrap(); + assert_eq!(image.dimensions(), (1, 1)); + assert_eq!(image.meta.text, vec![("Good".into(), "retained".into())]); + } +} + +#[test] +fn png_invalid_optional_text_does_not_spend_an_exhausted_budget() { + let full = png_text(b"tEXt", b"K", &vec![b'A'; PNG_TEXT_BUDGET - 1]); + let invalid = [png_chunk(b"iTXt", b"Bad\0\0\0\0\0\xFF"), png_text_stream(b"zTXt", b"Bad", b"invalid")]; + let image = png_text_decode(&png_with_text(&[full], &invalid), PNG_TEXT_BUDGET).unwrap(); + assert_eq!(image.meta.text.len(), 1); + assert_eq!(image.meta.text[0].1.len(), PNG_TEXT_BUDGET - 1); +} + #[test] fn pnm_bomb_header_rejected() { assert!(is_limit(decode(b"P6\n200000 200000\n255\n\0\0\0")));