diff --git a/crates/redact/README.md b/crates/redact/README.md index 0db0e18c49..ec73af0ae3 100644 --- a/crates/redact/README.md +++ b/crates/redact/README.md @@ -47,8 +47,9 @@ clear_marks(&mut doc, None)?; // remove marks without app is nested past the depth limit (`MAX_DEPTH`, 50) or its content can't be read; or more than 20,000 terms, or a run longer than 4,096 characters, to search for. Once the search's work limit (2^28 steps) runs out, every entry and packet not yet checked goes - too. An XMP packet that isn't valid UTF-8 or UTF-16, can't be decoded, or decodes to - more than 16 MiB goes without being read. + too. An XMP packet that isn't valid UTF-8 or UTF-16, can't be decoded strictly (damaged or + cut-short filter data, or more than one filter), or decodes to more than + 16 MiB goes without being read. - **Not scrubbed** (Sanitize's categories cover them): XMP streams on anything but the catalog and the pages (images, fonts, form XObjects, annotations and other objects), bookmark titles, and attachments (embedded files, their names and descriptions). diff --git a/crates/redact/src/metadata.rs b/crates/redact/src/metadata.rs index 6fced3ad91..8946cf46f6 100644 --- a/crates/redact/src/metadata.rs +++ b/crates/redact/src/metadata.rs @@ -12,8 +12,9 @@ //! removed run, is removed whole: editing the XML in place could leave it malformed or keep the //! term in another spelling. A PDF/A identification in that packet goes with it. //! -//! Fail-closed: XMP that isn't UTF-8 or UTF-16 goes, text too large to search against every term -//! counts as matching, and when the removed text itself was cut short (too many runs or terms, +//! Fail-closed: XMP that isn't UTF-8 or UTF-16, or whose filter data is damaged or behind more than +//! one filter (it is decoded strictly, so nothing after damage goes unsearched), goes; text too +//! large to search against every term counts as matching, and when the removed text itself was cut short (too many runs or terms, //! or a run too long to keep) every `/Info` string and every XMP stream goes, since what was //! never collected can't be searched for. Terms are matched without regard to case or spacing. @@ -273,8 +274,14 @@ pub(crate) fn decode_xmp(bytes: &[u8]) -> Option { /// The text values of an XMP stream, or `None` when it can't be read. fn xmp_values(doc: &Document, o: &Object) -> Option> { let Object::Stream(s) = &*doc.resolve(o) else { return None }; - // Decoding stops at the cap, so a small stream that inflates hugely costs no more memory. - let bytes = s.decoded_within(MAX_XMP_BYTES).ok()?; + // Decoded strictly: a tolerant decode returns whatever came before the damage, and the part + // after it would go unsearched. A chain of more than one filter has no strict decoder here, + // so it isn't read either (fail-closed; XMP is rarely filtered at all). Decoding stops at the + // cap, so a small stream that inflates hugely costs no more memory. + if s.filters().len() > 1 { + return None; + } + let bytes = s.decoded_first_filter_strict(MAX_XMP_BYTES).ok()?; decode_xmp(&bytes).map(|t| xml_values(&t)) } diff --git a/crates/redact/src/tests.rs b/crates/redact/src/tests.rs index e873a03b0e..b810043279 100644 --- a/crates/redact/src/tests.rs +++ b/crates/redact/src/tests.rs @@ -3040,6 +3040,7 @@ fn unused_descriptions_in_shared_or_inherited_resources_are_retired() { mod descriptions; mod inline_images; mod limits; +mod metadata_fail_closed; mod property_retirement; mod surviving_references; mod text_state; diff --git a/crates/redact/src/tests/metadata_fail_closed.rs b/crates/redact/src/tests/metadata_fail_closed.rs new file mode 100644 index 0000000000..0a958ccd88 --- /dev/null +++ b/crates/redact/src/tests/metadata_fail_closed.rs @@ -0,0 +1,157 @@ +//! #2539 review: each way the metadata check fails closed, end to end. When the removed text +//! can't be known in full, every `/Info` string and both the catalog's and the page's XMP go; +//! when one XMP packet can't be read, that packet goes and the rest of the metadata is checked +//! as usual. + +use super::*; + +const CLEAN_XMP: &str = r#"Annual figures"#; + +/// One page showing `content`, with `/Info` (two strings and a name), catalog XMP (object 7) +/// and page XMP (object 8). None of the metadata mentions anything the tests remove. +fn page_with_metadata(content: &[u8], catalog_xmp: Vec) -> Document { + let objs: Vec> = vec![ + b"<< /Type /Catalog /Pages 2 0 R /Metadata 7 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 << /Font << /F1 5 0 R >> >> /Metadata 8 0 R >>".to_vec(), + stream("", content), + FONT.replace("95 0 R", "6 0 R").into_bytes(), + widths(), + catalog_xmp, + stream("/Type /Metadata /Subtype /XML", CLEAN_XMP.as_bytes()), + b"<< /Title (Annual figures) /Producer (Test) /Trapped /False >>".to_vec(), + ]; + Document::open(Arc::new(Pdf::new().objs(objs).trailer_entry("/Info 9 0 R").build())).unwrap() +} + +fn clean_xmp() -> Vec { + stream("/Type /Metadata /Subtype /XML", CLEAN_XMP.as_bytes()) +} + +fn catalog_has_xmp(doc: &Document) -> bool { + doc.get(doc.root().unwrap()).as_dict().is_some_and(|c| c.contains(b"Metadata")) +} + +fn page_has_xmp(doc: &Document) -> bool { + pdfcraft_model::pages(doc)[0].dict.contains(b"Metadata") +} + +/// Mark the line at y 100 across the page and apply. +fn redact_line(doc: &mut Document) -> Report { + mark(doc, 0, &[[0.0, 95.0, 300.0, 110.0]], ""); + apply(doc, None).unwrap() +} + +/// Every `/Info` string and both XMP streams went, though none of them mentions what was removed. +fn assert_cleared(doc: &Document, r: &Report) { + assert_eq!(r.metadata, 4, "{r:?}"); + assert_eq!(info_keys(doc), ["Trapped"], "strings go, the name stays"); + assert!(!catalog_has_xmp(doc) && !page_has_xmp(doc), "both XMP streams go"); +} + +/// Control for the tests below: removing a short word that the metadata doesn't mention keeps +/// all of it. +#[test] +fn metadata_that_does_not_mention_the_removed_text_stays() { + let mut doc = page_with_metadata(b"BT /F1 10 Tf 10 100 Td (Bluefin) Tj ET", clean_xmp()); + let r = redact_line(&mut doc); + assert_eq!(r.metadata, 0, "{r:?}"); + assert_eq!(info_keys(&doc), ["Title", "Producer", "Trapped"]); + assert!(catalog_has_xmp(&doc) && page_has_xmp(&doc)); +} + +/// A single removed run longer than the collection keeps (64 KiB): the part past the limit is +/// never seen. The run is one letter, 70,000 spaces (one space once normalized) and another +/// letter, so what would be collected without the limit is short enough to search, and only +/// the limit itself clears the metadata. +#[test] +fn a_removed_run_over_64_kib_clears_the_metadata() { + // At 0.004 pt each glyph advances 0.002 pt, so all 70,002 fit in 141 pt of one line. + let text = format!("A{}B", " ".repeat(70_000)); + let content = format!("BT /F1 0.004 Tf 10 100 Td ({text}) Tj ET"); + let mut doc = page_with_metadata(content.as_bytes(), clean_xmp()); + let r = redact_line(&mut doc); + assert_eq!(r.glyphs, 70_002, "{r:?}"); + assert_cleared(&doc, &r); +} + +/// More than 20,000 distinct terms: each removed run is one word, far enough from the last to +/// start a run of its own, and each word is different. +#[test] +fn more_than_20000_terms_clear_the_metadata() { + let mut content = b"BT /F1 10 Tf 10 100 Td".to_vec(); + // Words alternate between x 10 and x 70; each is 30 pt wide, so the gap between two is + // 30 pt, more than a run spans. + for i in 0..20_001u32 { + let x = if i % 2 == 0 { 60 } else { -60 }; + content.extend_from_slice(format!(" (q{i:05}) Tj {x} 0 Td").as_bytes()); + } + content.extend_from_slice(b" ET"); + let mut doc = page_with_metadata(&content, clean_xmp()); + let r = redact_line(&mut doc); + assert_eq!(r.glyphs, 20_001 * 6, "{r:?}"); + assert_cleared(&doc, &r); +} + +/// A removed run longer than 4,096 characters can't be searched for whole. +#[test] +fn a_removed_run_over_4096_characters_clears_the_metadata() { + // 4,100 letters at 0.05 pt advance 0.025 pt each: 103 pt in all, one run, 4 KiB. + let content = format!("BT /F1 0.05 Tf 10 100 Td ({}) Tj ET", "Z".repeat(4_100)); + let mut doc = page_with_metadata(content.as_bytes(), clean_xmp()); + let r = redact_line(&mut doc); + assert_eq!(r.glyphs, 4_100, "{r:?}"); + assert_cleared(&doc, &r); +} + +/// The catalog's packet can't be read: that packet goes, unread, while the page's packet and +/// `/Info`, which don't mention the removed word, stay. +fn assert_only_the_catalog_packet_went(catalog_xmp: Vec) { + let mut doc = page_with_metadata(b"BT /F1 10 Tf 10 100 Td (Bluefin) Tj ET", catalog_xmp); + let r = redact_line(&mut doc); + assert_eq!(r.metadata, 1, "{r:?}"); + assert!(!catalog_has_xmp(&doc), "the packet that couldn't be read goes"); + assert!(page_has_xmp(&doc), "the readable packet is checked, and stays"); + assert_eq!(info_keys(&doc), ["Title", "Producer", "Trapped"]); +} + +/// XMP whose Flate data is corrupt can't be decoded, so it can't be searched. +#[test] +fn an_xmp_stream_whose_filter_data_is_corrupt_goes() { + assert_only_the_catalog_packet_went(stream("/Type /Metadata /Subtype /XML /Filter /FlateDecode", b"this is not deflate data at all")); +} + +/// A valid Flate stream cut off half way decodes, leniently, to the first part of the packet; +/// the rest was never seen, so the packet isn't searched but goes. +#[test] +fn an_xmp_stream_whose_filter_data_is_cut_short_goes() { + let full = Stream::flate(Dict::new(), CLEAN_XMP.as_bytes()); + let cut = full.raw.get(..full.raw.len() / 2).unwrap(); + assert_only_the_catalog_packet_went(stream("/Type /Metadata /Subtype /XML /Filter /FlateDecode", cut)); +} + +/// XMP that decodes to more than 16 MiB isn't read at all. +#[test] +fn an_xmp_stream_over_16_mib_goes() { + // A valid packet, padded with whitespace past the limit. + let padded = CLEAN_XMP.replace("", &format!("{}", " ".repeat(16 * 1024 * 1024))); + assert_only_the_catalog_packet_went(stream("/Type /Metadata /Subtype /XML", padded.as_bytes())); +} + +/// XMP that is neither UTF-8 nor UTF-16 (here Latin-1) isn't searched lossily. +#[test] +fn an_xmp_stream_in_another_encoding_goes() { + let latin1 = CLEAN_XMP.replace("Annual figures", "Ann\u{e9}e").replace('\u{e9}', "\u{0}"); + let bytes: Vec = latin1.bytes().map(|b| if b == 0 { 0xE9 } else { b }).collect(); + assert!(std::str::from_utf8(&bytes).is_err()); + assert_only_the_catalog_packet_went(stream("/Type /Metadata /Subtype /XML", &bytes)); +} + +/// XMP behind two filters has no strict decoder here, so it isn't read either. The first filter +/// alone decodes to valid ASCII (the packet in hex), which must not be searched as the packet. +#[test] +fn an_xmp_stream_behind_two_filters_goes() { + let hex: String = CLEAN_XMP.bytes().map(|b| format!("{b:02x}")).chain(std::iter::once(">".to_string())).collect(); + let Stream { raw, .. } = Stream::flate(Dict::new(), hex.as_bytes()); + assert_only_the_catalog_packet_went(stream("/Type /Metadata /Subtype /XML /Filter [/FlateDecode /ASCIIHexDecode]", &raw)); +}