From 063f8629e4c1364a1e10733bcc1d49a2a9f2d074 Mon Sep 17 00:00:00 2001 From: Grzegorz Martens Date: Mon, 9 Nov 2026 13:36:29 +0100 Subject: [PATCH] M7: each filter stage of a font's stream is held to its limit, as without an allowance The Resources review of 8eead8ecd. The program read and the metrics reads passed their limit times the number of filter stages as one total, which any one stage could use: a FontFile2 of Flate making 128 MiB of whitespace, under seven ASCIIHex stages (a 130 KB file), made all of it before being refused, about 0.1 s a font where `main`'s bound stopped it at 16 MiB. `edit`'s `matching_thai_outline` reads such a program for each of a page's fonts with no total, and every `Metrics::from_dict` up to three times. Now each stage is held to the limit, as a decode without an allowance holds it, and the stages together to the limit for each of them, so every stream the stage scaling was for still reads: a full `/CIDToGIDMap` or a 12 MiB program behind ASCIIHex and Flate. The two bounds take an entry point of their own: `pdfcraft_filters::decode_tolerant_counted_each`, which `run` already supports, and `DecodeBudget::decode_stages_within`, which `Reads::decode` and the editor's program read use. A stage past the limit is charged what the stages before it decoded and the limit; past the stages' total, or past what is left, it is charged as `decode_within` charges. The program, the last stage, is so within the limit without a check of its own. Tests: `each_stage_is_held_to_its_own_bound_within_the_total` in `pdfcraft-filters` (a Flate stage stopped at its bound hands the next stage nothing), `each_stage_is_held_to_the_limit` in `pdfcraft-cos`, and in `pdfcraft-fonts` `an_editors_program_read_holds_each_stage_to_its_limit` and `each_stage_of_a_metrics_stream_is_held_to_its_limit`. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01AmsdeUCdTaD72jkDw2pJKY --- crates/cos/src/budget.rs | 78 +++++++++++++++++++++++++++++++++++ crates/cos/src/object.rs | 20 +++++++++ crates/filters/src/lib.rs | 36 ++++++++++++++++ crates/fonts/src/pdf.rs | 31 +++++--------- crates/fonts/src/pdf/tests.rs | 50 ++++++++++++++++++++++ 5 files changed, 195 insertions(+), 20 deletions(-) diff --git a/crates/cos/src/budget.rs b/crates/cos/src/budget.rs index 56286ce4a0..f456583732 100644 --- a/crates/cos/src/budget.rs +++ b/crates/cos/src/budget.rs @@ -283,6 +283,45 @@ impl DecodeBudget { self.charge(stream, max, Stream::decoded_within_counted).map(|(data, _)| data) } + /// Like [`DecodeBudget::decode_within`], with every filter stage held to `max`, as + /// [`Stream::decoded_within`] holds it, and the stages together to `max` for each of them. A + /// stream so reads here when it reads without an allowance and the allowance can pay for + /// what its stages decode, the output of one stage on top of another's. A stage past `max` + /// is an error that charges what the stages before it decoded and `max`, the work it took to + /// find out; past the stages' total it charges that total, and past what is left it spends + /// all of it, as [`DecodeBudget::decode_within`] does. + pub fn decode_stages_within(&mut self, stream: &Stream, max: usize) -> Result, CosError> { + if self.bytes == 0 && !stream.raw.is_empty() { + return Err(CosError::Filter(FilterError::LimitExceeded(self.limit).to_string())); + } + let total = max.saturating_mul(stream.filters().len().max(1)); + let room = total.min(self.bytes); + match stream.decoded_within_each_counted(max, room) { + Ok((data, _, spent)) => { + self.bytes = self.bytes.saturating_sub(spent); + Ok(data) + } + // A stage stopped at `max`, below what was left of the room. + Err((FilterError::LimitExceeded(_), spent)) if room.saturating_sub(spent) > max => { + self.bytes = self.bytes.saturating_sub(spent.saturating_add(max)); + Err(CosError::Filter(FilterError::LimitExceeded(max).to_string())) + } + // At the stages' total, with more left. + Err((FilterError::LimitExceeded(_), _)) if total < self.bytes => { + self.bytes = self.bytes.saturating_sub(total); + Err(CosError::Filter(FilterError::LimitExceeded(total).to_string())) + } + Err((FilterError::LimitExceeded(_), _)) => { + self.bytes = 0; + Err(CosError::Filter(FilterError::LimitExceeded(self.limit).to_string())) + } + Err((e, spent)) => { + self.bytes = self.bytes.saturating_sub(spent); + Err(CosError::Filter(e.to_string())) + } + } + } + /// [`DecodeBudget::decode_within`] (`usize::MAX` for no bound of its own), with the stream's /// `/DecodeParms` given by reference read in `doc`, also saying whether the stream is damaged: /// `true` when a filter stopped at corrupt data, so the bytes are only what decoded before it. @@ -1025,4 +1064,43 @@ mod tests { assert!(budget.decode_strict(&Document::new_empty(), &chained).is_ok()); assert_eq!(budget.remaining(), (10 << 20) - 3 * charged); } + + /// `decode_stages_within` holds every stage to the limit, as a decode without an allowance + /// does, and the stages together to the limit for each of them. A stream behind ASCIIHex and + /// Flate whose stages each stay within it reads, the hex stage's output charged on top; one + /// whose Flate stage would make more stops there, charged what the hex stage made and the + /// limit, the rest of the allowance left; a Flate stage making hex text past the limit leaves + /// the stream unread though the total has room, as `Stream::decoded_within` leaves it; and a + /// stream past what is left spends all of it. + #[test] + fn each_stage_is_held_to_the_limit() { + let chained = |data: &[u8], filters: [&str; 2]| { + let mut dict = Dict::new(); + dict.set(b"Filter".to_vec(), Object::Array(filters.into_iter().map(Object::name).collect())); + Stream::from_raw(dict, data.to_vec()) + }; + let hexed = |data: &[u8]| { + let flate = Stream::flate(Dict::new(), data); + let hex: Vec = flate.raw.iter().flat_map(|b| format!("{b:02X}").into_bytes()).chain(*b">").collect(); + (chained(&hex, ["ASCIIHexDecode", "FlateDecode"]), flate.raw.len()) + }; + let max = 1 << 16; + let (fits, inner) = hexed(&vec![0u8; max]); + let mut budget = DecodeBudget::new(1 << 20, 0); + assert_eq!(budget.decode_stages_within(&fits, max).map(|d| d.len()).ok(), Some(max)); + assert_eq!(budget.remaining(), (1 << 20) - inner - max); + let (bomb, inner) = hexed(&vec![0u8; 8 * max]); + let mut budget = DecodeBudget::new(1 << 20, 0); + assert!(budget.decode_stages_within(&bomb, max).is_err()); + assert_eq!(budget.remaining(), (1 << 20) - inner - max, "the Flate stage stopped at the limit"); + // Flate making 1.2 times the limit of hex text, which ASCIIHex halves: the stages together + // make 1.8 times the limit, within twice it, but the Flate stage alone is past it. + let text = "00".repeat(max * 6 / 10).into_bytes(); + let wide = chained(&Stream::flate(Dict::new(), &text).raw, ["FlateDecode", "ASCIIHexDecode"]); + assert!(wide.decoded_within(max).is_err()); + assert!(DecodeBudget::new(1 << 20, 0).decode_stages_within(&wide, max).is_err()); + let mut budget = DecodeBudget::new(max, 0); + assert!(budget.decode_stages_within(&fits, max).is_err()); + assert_eq!(budget.remaining(), 0); + } } diff --git a/crates/cos/src/object.rs b/crates/cos/src/object.rs index f40f175d69..f5c25091fe 100644 --- a/crates/cos/src/object.rs +++ b/crates/cos/src/object.rs @@ -382,6 +382,26 @@ impl Stream { self.decode_chain(max, decode).map(|(v, spent)| (v, damaged, spent)) } + /// [`Stream::decoded_within_counted`] with every filter stage also held to `each`, as + /// [`Stream::decoded_within`] holds it, while `max` bounds the stages together + /// ([`pdfcraft_filters::decode_tolerant_counted_each`]). Without filters the data is held to + /// both. + pub(crate) fn decoded_within_each_counted( + &self, + each: usize, + max: usize, + ) -> Result<(Vec, bool, usize), (pdfcraft_filters::FilterError, usize)> { + let mut damaged = false; + let decode = |chain: &[(Filter, Params)], data: &[u8], max| { + pdfcraft_filters::decode_tolerant_counted_each(chain, data, each.min(MAX_DECODED), max).map(|(v, partial, spent)| { + damaged = partial; + (v, spent) + }) + }; + let max = if self.readable_filters().is_ok_and(|chain| chain.is_empty()) { max.min(each) } else { max }; + self.decode_chain(max, decode).map(|(v, spent)| (v, damaged, spent)) + } + /// [`Stream::decoded_within`] spending from `work`, what an operation may still do across a /// document's images ([`pdfcraft_filters::decode_tolerant_within`]). pub(crate) fn decoded_spending(&self, max: usize, work: &mut u64) -> Result, CosError> { diff --git a/crates/filters/src/lib.rs b/crates/filters/src/lib.rs index 9342213577..4bee6b451c 100644 --- a/crates/filters/src/lib.rs +++ b/crates/filters/src/lib.rs @@ -212,6 +212,18 @@ pub fn decode_tolerant_counted(chain: &[(Filter, Params)], data: &[u8], max_outp run(chain, data, max_output, Some(max_output), false, false, None) } +/// [`decode_tolerant_counted`] with the two bounds apart: `max_stage` bounds every stage, as +/// [`decode_tolerant`]'s bound does, and `max_output` the bytes all stages decode together. A +/// stage stopped by either gives [`FilterError::LimitExceeded`] and the bytes decoded before it. +pub fn decode_tolerant_counted_each( + chain: &[(Filter, Params)], + data: &[u8], + max_stage: usize, + max_output: usize, +) -> Result<(Vec, bool, usize), (FilterError, usize)> { + run(chain, data, max_stage, Some(max_output), false, false, None) +} + /// [`decode_tolerant`], spending from `work`, what decoding may still cost across a run of images: /// a step for each byte of `data`, paid before anything is decoded, [`FLATE_READ_WORK`] for each /// byte a Flate stage reads, paid before it runs (again if it reads them twice), and one for each @@ -451,6 +463,30 @@ mod tests { assert_eq!(super::decode(&chain(super::MAX_CHAIN), &data, 1 << 20).ok(), Some(data)); } + /// [`decode_tolerant_counted_each`] holds every stage to its own bound as well as the stages + /// to their total: a Flate stage that would make more than that bound stops there, though + /// the total has room, and the stage after it gets nothing; within both, the chain reads + /// whole, one stage's output charged on top of another's. + #[test] + fn each_stage_is_held_to_its_own_bound_within_the_total() { + let hex = "00".repeat(1 << 16).into_bytes(); + let data = super::flate::encode(&hex); + let chain = [(Filter::Flate, Params::default()), (Filter::AsciiHex, Params::default())]; + // The total alone lets the Flate stage make all of the hex text, and the hex stage read it. + STAGE_BYTES.with(|c| c.set(0)); + let whole = decode_tolerant_counted(&chain, &data, 3 << 16).map(|(v, _, _)| v); + assert_eq!(whole.ok(), Some(vec![0u8; 1 << 16])); + assert_eq!(STAGE_BYTES.with(|c| c.get()), data.len() + hex.len()); + // Each stage held to 64 KiB, the Flate stage stops there and the hex stage gets nothing. + STAGE_BYTES.with(|c| c.set(0)); + let held = decode_tolerant_counted_each(&chain, &data, 1 << 16, 3 << 16); + assert!(matches!(held, Err((FilterError::LimitExceeded(m), 0)) if m == 1 << 16), "{held:?}"); + assert_eq!(STAGE_BYTES.with(|c| c.get()), data.len()); + // Within both, it reads whole. + let (out, _, spent) = decode_tolerant_counted_each(&chain, &data, 2 << 16, 3 << 16).unwrap(); + assert_eq!((out.len(), spent), (1 << 16, hex.len() + (1 << 16))); + } + use super::*; #[test] diff --git a/crates/fonts/src/pdf.rs b/crates/fonts/src/pdf.rs index 024d59f937..c8f34bd1c4 100644 --- a/crates/fonts/src/pdf.rs +++ b/crates/fonts/src/pdf.rs @@ -122,14 +122,8 @@ pub fn embedded_sfnt_within(doc: &Document, font: &Dict, budget: &mut DecodeBudg if budget.remaining() == 0 { continue; } - let stream = doc.with_direct_parms(stream); - // As in `Reads::decode`: the stages share the limit once for each of them, which any one - // may use, and the program itself must be within it. - let stages = stream.filters().len().max(1); - let Ok(mut bytes) = budget.decode_within(&stream, MAX_SFNT.saturating_mul(stages)) else { continue }; - if bytes.len() > MAX_SFNT { - continue; - } + // As in `Reads::decode`: each stage within the limit, the program, the last, too. + let Ok(mut bytes) = budget.decode_stages_within(&doc.with_direct_parms(stream), MAX_SFNT) else { continue }; let Ok(face) = skrifa::FontRef::from_index(&bytes, 0) else { continue }; if face.metrics(skrifa::instance::Size::unscaled(), skrifa::instance::LocationRef::default()).units_per_em == 0 { continue; @@ -402,13 +396,13 @@ impl<'a> Reads<'a> { /// by reference too ([`Document::with_direct_parms`]); `None` when it can't be, noting when the /// allowance is why. /// - /// `max` bounds what the stream decodes to, and its filter stages together may make up to `max` - /// for each of them, as much as a decode without an allowance lets them: a map of 65,536 CIDs - /// behind ASCIIHex and Flate, its Flate data smaller than the map, makes all of `MAX_CID_TO_GID` - /// in its last stage, with the hex stage's output on top. One stage may so make more than `max`, which a decode without an - /// allowance stops, so a stream that one leaves unread can read here, never the other way - /// round. A stream past `max` is unread, not refused, when what is left can pay for its stages; - /// when it can't, the stream spends it and counts as refused. + /// Each of its filter stages is held to `max`, as a decode without an allowance holds it, and + /// the stages together to `max` for each of them ([`DecodeBudget::decode_stages_within`]): a + /// map of 65,536 CIDs behind ASCIIHex and Flate, its Flate data smaller than the map, makes + /// all of `MAX_CID_TO_GID` in its last stage, with the hex stage's output on top. So a stream + /// reads here when it reads without an allowance and what is left pays for it. A stream past + /// `max` is unread, not refused, when what is left can pay for its stages; when it can't, the + /// stream spends it and counts as refused. fn decode(&mut self, stream: &Stream, max: usize) -> Option> { // Once the allowance is spent the decode refuses the stream; refused here, before the // parameters are resolved, which is work the allowance doesn't count. @@ -416,11 +410,8 @@ impl<'a> Reads<'a> { self.refused = true; return None; } - let stream = self.doc.with_direct_parms(stream); - let stages = stream.filters().len().max(1); - match self.budget.decode_within(&stream, max.saturating_mul(stages)) { - Ok(data) if data.len() <= max => Some(data), - Ok(_) => None, + match self.budget.decode_stages_within(&self.doc.with_direct_parms(stream), max) { + Ok(data) => Some(data), Err(_) => { self.refused |= self.budget.remaining() == 0; None diff --git a/crates/fonts/src/pdf/tests.rs b/crates/fonts/src/pdf/tests.rs index 6acc6dedd1..53d43f0c72 100644 --- a/crates/fonts/src/pdf/tests.rs +++ b/crates/fonts/src/pdf/tests.rs @@ -374,6 +374,56 @@ fn a_full_cid_map_reads_behind_one_or_two_filters() { } } +/// The editor's program read holds every filter stage to the program's limit: a program behind +/// ASCIIHex and Flate whose Flate stage would make 24 MiB stops at 16 MiB, charged that and what +/// the hex stage made, where the stages' total alone would let it make all 24 MiB. A program +/// within the limit behind the same two filters reads (see also +/// `an_editors_program_reads_behind_ascii_hex_and_flate_within_its_limit`). +#[test] +fn an_editors_program_read_holds_each_stage_to_its_limit() { + let mut doc = Document::new_empty(); + let flate = Stream::flate(Dict::new(), &vec![b' '; 24 << 20]); + let hex: Vec = flate.raw.iter().flat_map(|b| format!("{b:02X}").into_bytes()).chain(*b">").collect(); + let mut dict = Dict::new(); + dict.set(b"Filter".to_vec(), Object::Array(vec![Object::name("ASCIIHexDecode"), Object::name("FlateDecode")])); + let program = doc.add(Object::Stream(Stream::from_raw(dict, hex))); + let mut descriptor = Dict::new(); + descriptor.set(b"FontFile2".to_vec(), Object::Ref(program)); + let f = font(&mut doc, vec![("Subtype", Object::name("TrueType")), ("FontDescriptor", Object::Dict(descriptor))]); + let mut budget = DecodeBudget::new(usize::MAX / 2, 0); + assert!(embedded_sfnt_within(&doc, &f, &mut budget).is_none()); + assert_eq!(usize::MAX / 2 - budget.remaining(), flate.raw.len() + MAX_SFNT); +} + +/// Each stage of a stream a font's metrics are read from is held to the stream's limit, as a +/// decode without an allowance holds it: a `/CIDToGIDMap` behind Flate and ASCIIHex whose Flate +/// stage makes hex text past the map's limit is unread, as `Stream::decoded_within` leaves it, +/// though the two stages together make less than twice the limit; with its hex text within the +/// limit it reads. +#[test] +fn each_stage_of_a_metrics_stream_is_held_to_its_limit() { + let read = |cids: usize| { + let mut doc = Document::new_empty(); + let (mut f, _) = subset_type0(&mut doc, b""); + let map: Vec = (0..cids).flat_map(|cid| u16::try_from(cid % 2000).unwrap().to_be_bytes()).collect(); + let hex: Vec = map.iter().flat_map(|b| format!("{b:02X}").into_bytes()).collect(); + let mut dict = Dict::new(); + dict.set(b"Filter".to_vec(), Object::Array(vec![Object::name("FlateDecode"), Object::name("ASCIIHexDecode")])); + let stream = Stream::from_raw(dict, Stream::flate(Dict::new(), &hex).raw.to_vec()); + let unread = stream.decoded_within(MAX_CID_TO_GID).is_err(); + let map = Object::Ref(doc.add(Object::Stream(stream))); + let Some(Object::Array(mut descendants)) = f.get(b"DescendantFonts").cloned() else { panic!("descendant") }; + if let Some(Object::Dict(d)) = descendants.get_mut(0) { + d.set(b"CIDToGIDMap".to_vec(), map); + } + f.set(b"DescendantFonts".to_vec(), Object::Array(descendants)); + (Metrics::from_dict(&doc, &f).outlined_cids.is_empty(), unread) + }; + // 0.6 of the limit: its hex text, 1.2 of it, is past the limit; the stages make 1.8 of it. + assert_eq!(read(MAX_CID_TO_GID * 6 / 20), (true, true)); + assert_eq!(read(MAX_CID_TO_GID / 4), (false, false)); +} + /// The editor's program is held to its limit as the metrics reads' streams are: a 12 MiB program /// whose Flate data is about 8 MiB reads behind ASCIIHex and Flate as behind Flate alone, though /// the two stages make about 20 MiB together. A program of the limit reads behind either, and one a