diff --git a/crates/redact/src/interp.rs b/crates/redact/src/interp.rs index 6781a4060c..2dcafb339f 100644 --- a/crates/redact/src/interp.rs +++ b/crates/redact/src/interp.rs @@ -861,6 +861,8 @@ fn xobject( return None; } scope.report.forms_removed += 1; + // Its text was never read, so the metadata check can't know what went with it. + scope.removed_cut = true; out.replaced.push(name); out.doomed.push(r); return Some(None); @@ -887,6 +889,7 @@ fn xobject( } Err(_) if scope.mode == Mode::Apply => { scope.report.forms_removed += 1; + scope.removed_cut = true; out.replaced.push(name); out.doomed.push(r); return Some(None); diff --git a/crates/redact/src/metadata.rs b/crates/redact/src/metadata.rs index 84500ecc81..c50401ef34 100644 --- a/crates/redact/src/metadata.rs +++ b/crates/redact/src/metadata.rs @@ -26,8 +26,9 @@ const MAX_TERM_CHARS: usize = 4096; const MAX_TERMS: usize = 20_000; /// XMP packets larger than this, decoded, are removed without reading them. const MAX_XMP_BYTES: usize = 16 * 1024 * 1024; -/// Text whose length times the number of terms exceeds this is treated as matching rather than -/// searched (fail-closed), so a page of removed text against a large packet stays quick. +/// The searching (text length times terms, or value length times removed text) one scrub may +/// do in all; past it every entry and packet counts as matching (fail-closed), so a page of +/// removed text against a large packet, or against very many small values, stays quick. const MAX_WORK: usize = 1 << 28; /// Lower-case `s` with every run of whitespace made one space, trimmed. @@ -65,16 +66,41 @@ pub(crate) fn terms(removed: &[String]) -> Option> { Some(out) } -fn mentions(text: &str, terms: &[String]) -> bool { +/// The searching one scrub may do, across every `/Info` entry and XMP value: past it, each +/// further question is answered "yes" (fail-closed), so a packet of many small values can't add +/// up to more than one large one. +struct Work(std::cell::Cell); + +impl Work { + fn new(limit: usize) -> Self { + Self(std::cell::Cell::new(limit)) + } + + /// Take `cost` from what is left; `false` (and nothing left) when it doesn't fit. + fn spend(&self, cost: usize) -> bool { + match self.0.get().checked_sub(cost) { + Some(left) => { + self.0.set(left); + true + } + None => { + self.0.set(0); + false + } + } + } +} + +fn mentions(text: &str, terms: &[String], work: &Work) -> bool { let text = normalize(text); - text.len().saturating_mul(terms.len()) > MAX_WORK || terms.iter().any(|t| text.contains(t.as_str())) + !work.spend(text.len().saturating_mul(terms.len())) || terms.iter().any(|t| text.contains(t.as_str())) } /// Is `value` itself part of one of the removed `runs` (normalized), as whole words? It needs /// three letters or digits, so an entry such as "by" doesn't go with every sentence that /// contains it, and word boundaries, so an XMP value "True" doesn't go with "untrue". Too much /// text to search counts as part (fail-closed). -fn within(value: &str, runs: &[String]) -> bool { +fn within(value: &str, runs: &[String], work: &Work) -> bool { let value = normalize(value); if !significant(&value, 3) { return false; @@ -88,7 +114,7 @@ fn within(value: &str, runs: &[String]) -> bool { !(word(before) && value.starts_with(char::is_alphanumeric)) && !(word(after) && value.ends_with(char::is_alphanumeric)) }) }; - value.len().saturating_mul(total) > MAX_WORK || runs.iter().any(whole) + !work.spend(value.len().saturating_mul(total)) || runs.iter().any(whole) } /// Decode the five predefined XML entities and character references; anything else stays. @@ -98,7 +124,8 @@ fn unescape(s: &str) -> String { while let Some(at) = rest.find('&') { out.push_str(rest.get(..at).unwrap_or("")); let tail = rest.get(at..).unwrap_or(""); - let end = tail.find(';').filter(|&e| e <= 12); + // An entity ends within 12 bytes; looking further made a text of bare `&`s quadratic. + let end = tail.char_indices().take(13).find(|&(_, c)| c == ';').map(|(e, _)| e); let entity = end.and_then(|e| tail.get(1..e)); let decoded = entity.and_then(|e| match e { "amp" => Some('&'), @@ -243,13 +270,24 @@ fn xmp_values(doc: &Document, o: &Object) -> Option> { /// removed text was cut short; then, as when the terms themselves exceed their limits, every /// `/Info` string and every XMP stream goes. Returns how many entries and streams went. pub(crate) fn scrub(doc: &mut Document, removed: &[String], complete: bool) -> Result { + scrub_within(doc, removed, complete, MAX_WORK) +} + +/// [`scrub`] with the searching it may do given, so tests can run out of it. +pub(crate) fn scrub_within(doc: &mut Document, removed: &[String], complete: bool, limit: usize) -> Result { // `None` matches everything (fail-closed). let search = match terms(removed).filter(|_| complete) { Some(t) if t.is_empty() => return Ok(0), - Some(t) => Some((t, removed.iter().map(|r| normalize(r)).collect::>())), + Some(t) => { + // The same run removed many times is searched for once. + let mut seen = HashSet::new(); + let runs: Vec = removed.iter().map(|r| normalize(r)).filter(|r| seen.insert(r.clone())).collect(); + Some((t, runs)) + } None => None, }; - let hit = |text: &str| search.as_ref().is_none_or(|(terms, runs)| mentions(text, terms) || within(text, runs)); + let work = Work::new(limit); + let hit = |text: &str| search.as_ref().is_none_or(|(terms, runs)| mentions(text, terms, &work) || within(text, runs, &work)); let mut n = 0; // The information dictionary, direct in the trailer or an object of its own. @@ -278,8 +316,8 @@ pub(crate) fn scrub(doc: &mut Document, removed: &[String], complete: bool) -> R let doomed = match xmp_values(doc, &xmp) { None => true, Some(values) => { - let words = |(terms, _): &(Vec, Vec)| mentions(&values.join(" "), terms); - let part = |(_, runs): &(Vec, Vec)| values.iter().any(|v| within(v, runs)); + let words = |(terms, _): &(Vec, Vec)| mentions(&values.join(" "), terms, &work); + let part = |(_, runs): &(Vec, Vec)| values.iter().any(|v| within(v, runs, &work)); search.as_ref().is_none_or(|s| words(s) || part(s)) } }; diff --git a/crates/redact/src/tests.rs b/crates/redact/src/tests.rs index 19f553332f..270434aa95 100644 --- a/crates/redact/src/tests.rs +++ b/crates/redact/src/tests.rs @@ -257,6 +257,63 @@ fn metadata_is_cleared_when_the_removed_text_was_cut_short() { assert!(!has_xmp(&doc)); } +/// #2539 review: the searching a scrub may do is one budget for all of it, not a check per +/// value: very many small `/Info` values each fit, but together they don't, and past the budget +/// the rest counts as matching (fail-closed). With room to search, nothing that doesn't mention +/// the removed text goes. +#[test] +fn metadata_search_has_one_budget_for_every_value() { + let entries: String = (0..50).map(|i| format!("/K{i} (an unrelated value number {i:02} in the list) ")).collect(); + let info = format!("<< {entries}/Producer (Test) >>"); + let fresh = || with_metadata(b"BT /F1 10 Tf 10 100 Td (Bluefin) Tj ET", &info, ""); + let mut doc = fresh(); + assert_eq!(metadata::scrub_within(&mut doc, &["Bluefin".into()], true, 1 << 28).unwrap(), 0, "room to search: nothing goes"); + let mut doc = fresh(); + // Each value costs about 40; fifty of them don't fit in 1000. + let n = metadata::scrub_within(&mut doc, &["Bluefin".into()], true, 1000).unwrap(); + assert!(n > 20, "past the budget the rest goes ({n} went)"); +} + +/// #2539 review: an entity is looked for only in the 12 bytes after its `&`, so a packet of bare +/// `&` characters is read in linear time, and decoding is unchanged. +#[test] +fn xml_entities_are_found_near_their_ampersand_only() { + assert_eq!(metadata::xml_values("Résumé & AA �"), ["Résumé & AA �"]); + let bare = "&".repeat(200_000); + let started = std::time::Instant::now(); + let values = metadata::xml_values(&format!("{bare}")); + assert_eq!(values.concat().len(), bare.len()); + eprintln!("200000 bare ampersands: {:?}", started.elapsed()); +} + +/// #2539 review: Apply removes a form XObject whole when it is nested past the depth limit (or +/// its stream can't be decoded), without reading its text, so what it held is unknown: the +/// metadata check fails closed. +#[test] +fn a_form_removed_unread_clears_the_metadata() { + // Forms 5..=20 each draw the next; the last one shows text. The page draws form 5. + let mut objs: Vec> = vec![ + b"<< /Type /Catalog /Pages 2 0 R >>".to_vec(), + b"<< /Type /Pages /Kids [3 0 R] /Count 1 >>".to_vec(), + b"<< /Type /Page /Parent 2 0 R /MediaBox [0 0 300 300] /Contents 4 0 R /Resources << /XObject << /Fm 5 0 R >> >> >>".to_vec(), + stream("", b"q /Fm Do Q"), + ]; + for n in 5..=20 { + objs.push(stream( + &format!("/Type /XObject /Subtype /Form /BBox [0 0 300 300] /Resources << /XObject << /Fm {} 0 R >> >>", n + 1), + b"q /Fm Do Q", + )); + } + objs.push(stream("/Type /XObject /Subtype /Form /BBox [0 0 300 300]", b"0 g 10 10 50 50 re f")); + objs.push(b"<< /Title (Annual figures) /Producer (Test) >>".to_vec()); + let info = objs.len(); + let mut doc = Document::open(Arc::new(Pdf::new().objs(objs).trailer_entry(&format!("/Info {info} 0 R")).build())).unwrap(); + mark(&mut doc, 0, &[[0.0, 0.0, 300.0, 300.0]], ""); + let r = apply(&mut doc, None).unwrap(); + assert!(r.forms_removed >= 1, "{r:?}"); + assert!(!info_keys(&doc).contains(&"Title".to_string()), "the deep form's text was never read: {:?}", info_keys(&doc)); +} + /// A value shorter than the redacted sentence goes from XMP too, in a custom schema's element or /// attribute, as from `/Info` (the regression #2598 suggested keeping): the packets holding it are /// removed, one that doesn't stays.