diff --git a/crates/redact/src/tags.rs b/crates/redact/src/tags.rs index 648c0c29e5..cece365293 100644 --- a/crates/redact/src/tags.rs +++ b/crates/redact/src/tags.rs @@ -193,7 +193,11 @@ pub(crate) fn clean_within( let touches = |doc: &Document, x: &Object, pg: Option| { mcid_of(doc, x, pg, page, stream).is_some_and(|m| stream.is_some() || changed.contains(&m) || empty.contains(&m)) }; + let mut steps = 0usize; while let Some((k, pg, above)) = todo.pop() { + if over_budget(&mut steps, todo.len(), limit) { + return Err(RedactError::Tags); + } match k { Object::Array(a) => todo.extend(a.into_iter().map(|x| (x, pg, above))), // A number or name tree node written in place. @@ -304,7 +308,7 @@ pub(crate) fn clean_within( /// Where the walk reached an object from: the nearest element above it that is an object of its /// own, and whether an element written in place between them carries a description (which only /// rewriting the object holding it could remove). -#[derive(Clone, Copy, Default)] +#[derive(Clone, Copy, Default, PartialEq, Eq, Hash)] struct Above { element: Option, described_in_place: bool, @@ -326,8 +330,22 @@ impl Above { /// The title (`/T`) is included, as producers often fill it with the element's text. const DESCRIPTIONS: [&[u8]; 4] = [b"Alt", b"ActualText", b"T", b"E"]; +/// Charges one step of a structure-tree walk visiting at most `limit` references. Kids are queued +/// for every element that lists them, and an indirect `/K` array shared by many elements is +/// listed again for each, so the steps and the queue are bounded too, not only the distinct +/// references: past either the cleanup can't be shown complete, and is refused. +fn over_budget(steps: &mut usize, pending: usize, limit: usize) -> bool { + *steps += 1; + *steps > limit.saturating_mul(STEPS_PER_REFERENCE) || pending > limit.saturating_mul(STEPS_PER_REFERENCE) +} + +/// Work items a walk may spend per reference it may visit: elements, their marked-content kids and +/// the tree nodes between them, with room to spare for real trees. +const STEPS_PER_REFERENCE: usize = 8; + /// Each element's parents as the walk found them: through `/K`, which `/P` may not agree with. -struct Parents(HashMap>); +/// A parent listing the element many times (a shared `/K` array of repeats) is held once. +struct Parents(HashMap>); impl Parents { fn new() -> Self { @@ -336,7 +354,7 @@ impl Parents { fn record(&mut self, r: ObjRef, above: Above) { if above.element.is_some() || above.described_in_place { - self.0.entry(r).or_default().push(above); + self.0.entry(r).or_default().insert(above); } } } @@ -474,7 +492,11 @@ pub(crate) fn drop_object_refs_within(doc: &mut Document, gone: &HashSet let mut seen = HashSet::new(); let mut parents = Parents::new(); let mut hits: Vec = Vec::new(); + let mut steps = 0usize; while let Some((k, above)) = todo.pop() { + if over_budget(&mut steps, todo.len(), limit) { + return Err(RedactError::Tags); + } match k { Object::Array(a) => todo.extend(a.into_iter().map(|x| (x, above))), // A structure element written in place: its kids are followed; one that tags a diff --git a/crates/redact/src/tests.rs b/crates/redact/src/tests.rs index 3c170aba20..58fefa7e05 100644 --- a/crates/redact/src/tests.rs +++ b/crates/redact/src/tests.rs @@ -3897,3 +3897,27 @@ fn a_redacted_form_xobject_drops_the_source_text_of_added_content() { let bytes = write_full(&doc, &SaveOptions { object_streams: false, ..SaveOptions::default() }).unwrap(); assert!(!bytes.windows(4).any(|w| w == b"1234"), "redacted text left in the saved file"); } + +/// 2 000 elements share one indirect `/K` array that lists one leaf element 2 000 times: the walk +/// queued and recorded every listing, four million, for a few thousand objects. Its work is now +/// bounded by the references it may visit, and past that the cleanup is refused. +#[test] +fn a_shared_kids_array_of_repeats_is_refused_not_expanded() { + const N: usize = 2_000; + let mut doc = one_page(b"BT /F1 10 Tf 10 100 Td (SECRET) Tj ET", "", vec![]); + let leaf = doc.add(Object::Dict(Dict::from_iter([(b"S".to_vec(), Object::name("Span"))]))); + let shared = doc.add(Object::Array(vec![Object::Ref(leaf); N])); + let elements: Vec = (0..N) + .map(|_| Object::Ref(doc.add(Object::Dict(Dict::from_iter([(b"S".to_vec(), Object::name("P")), (b"K".to_vec(), Object::Ref(shared))]))))) + .collect(); + let root = doc.add(Object::Dict(Dict::from_iter([(b"Type".to_vec(), Object::name("StructTreeRoot")), (b"K".to_vec(), Object::Array(elements))]))); + let catalog = doc.root().unwrap(); + doc.update_dict(catalog, |d| d.set(b"StructTreeRoot".to_vec(), Object::Ref(root))).unwrap(); + let gone: HashSet = [ObjRef::new(999_999, 0)].into_iter().collect(); + let started = std::time::Instant::now(); + assert!(matches!(crate::tags::drop_object_refs_within(&mut doc, &gone, 10_000), Err(RedactError::Tags))); + let page = pdfcraft_model::pages(&doc)[0].obj; + let changed: HashSet = [0].into_iter().collect(); + assert!(matches!(crate::tags::clean_within(&mut doc, page, None, &changed, &Default::default(), 10_000), Err(RedactError::Tags))); + assert!(started.elapsed() < std::time::Duration::from_secs(10), "{:?}", started.elapsed()); +}