Fix PNG text metadata decode budget bypass (#1460)

Bound decoded PNG text and XMP by one shared max_alloc budget before
growing output, retaining CRC validation and malformed ancillary handling.
Cover compressed, plain, Unicode and post-IDAT metadata with small
synthetic regression fixtures, and document the budget's scope.

Opt Streetblock into public GitHub profile-name credits at their request.

Co-authored-by: GPT-6 (Codex) <noreply@openai.com>
This commit is contained in:
Streetblock
2026-10-09 09:02:07 +07:00
committed by GitHub
co-authored by GPT-6
parent fd74419aeb
commit 7fd610741b
5 changed files with 379 additions and 23 deletions
+3
View File
@@ -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"
+2
View File
@@ -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
+145 -21
View File
@@ -18,8 +18,12 @@ fn err(e: impl std::fmt::Display) -> CodecError {
}
pub(crate) fn decode(bytes: &[u8], limits: &Limits) -> Result<Image, CodecError> {
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<Image, CodecError>
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<Image, CodecError>
{
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<usize> {
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<Option<String>, 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))
}
+4 -2
View File
@@ -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,
}
+225
View File
@@ -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<Image, CodecError> {
decode_with(bytes, &opts(Limits { max_alloc: max_alloc as u64, ..Limits::default() }))
}
fn zlib(text: &[u8]) -> Vec<u8> {
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<u8> {
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<u8> {
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<u8>], after_idat: &[Vec<u8>]) -> Vec<u8> {
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")));