From f2d64ca1367412dbf998de27fa9777d419089f9b Mon Sep 17 00:00:00 2001 From: Grzegorz Martens Date: Tue, 10 Nov 2026 15:53:40 +0100 Subject: [PATCH] M8: redaction refuses a page whose content it can't fully parse The redaction walk reads past bytes the content parser can't read: a stray `)` or `}` in a `TJ` array, say. Text they hide is neither found nor removed, while other readers draw it: Poppler's `pdftotext` extracts `[(TAIL) ) (SECRET)] TJ` as TAILSECRET. The only refusal was a side effect of the reads after the walk, which run only when the walk rewrote something, read what it rewrote as rewritten, and in the font cleanup's case only when the resources list a font. So Apply Redactions reported success and left the text where the walk rewrites nothing (the only text under the mark in such a `TJ`, or one in a form with resources of its own), in a stream it rewrites (it wrote the array out as `(TAIL) (SECRET) TJ`, without the stray byte or the brackets), and on a page whose font is set only through an ExtGState. Remove Hidden Information never counted hidden text in such a `TJ`: it found nothing to remove, or left it beside what it removed. The walk now records that it met such bytes (`Output::unparsed`, from `Parsed::skipped`, nested forms included), and Apply Redactions refuses the page with `RedactError::Unreadable`, as it does an inline image whose end is a guess. Remove Hidden Information counts such a page in its scan, so the removal runs and refuses it, as it does content that doesn't decode; it refuses an inline image whose end is a guess the same way, which it took on trust before. Apply walks only the forms that reach a mark, so bytes in a form away from the marks don't make the walk refuse the page; in a form without resources of its own, the check after the walk still refuses them when it runs, as before. Tests (`crates/redact/src/tests/unparsed.rs`): text under a mark in four broken `TJ` arrays, alone and beside other text; the same in a stream the walk rewrites; in a form under the mark with resources of its own; on a page without font resources; hidden text in such a `TJ`, alone and beside other hidden text, through the scan, Remove Hidden Information and Sanitize Document; hidden text beside an inline image the renderer can't end; and a form away from the marks, which is still redacted around. Two tests in `tests.rs` now expect the walk's refusal; the check after the walk keeps a case of its own, a form without resources away from the mark. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01AmsdeUCdTaD72jkDw2pJKY --- crates/redact/README.md | 5 +- crates/redact/src/interp.rs | 7 ++ crates/redact/src/lib.rs | 6 +- crates/redact/src/sanitize.rs | 11 +++ crates/redact/src/tests.rs | 48 +++++++----- crates/redact/src/tests/unparsed.rs | 114 ++++++++++++++++++++++++++++ 6 files changed, 170 insertions(+), 21 deletions(-) create mode 100644 crates/redact/src/tests/unparsed.rs diff --git a/crates/redact/README.md b/crates/redact/README.md index e47df98f17..d8f4b46e94 100644 --- a/crates/redact/README.md +++ b/crates/redact/README.md @@ -130,7 +130,10 @@ Text is placed with the font and size set by `Tf` or by an ExtGState's `/Font` t form XObject starts from the text state of the content that draws it. Content that can't be decoded makes the operation fail rather than leave it unredacted, and so does an inline image the renderer's tokenizer reads no image for (`RedactError::Unreadable`): where the page's operators -resume after it would be a guess. A Redact annotation with neither a usable `/QuadPoints` array nor a usable +resume after it would be a guess. So do bytes the content parser can't read, in the page's content +or in a form under an area (a stray `)` or `}` in a `TJ` array, say): the interpreter reads past +them, so text they hide, which other readers may draw, would be neither found nor removed. Remove +hidden information refuses a page for both too, in whatever content it walks. A Redact annotation with neither a usable `/QuadPoints` array nor a usable `/Rect` makes it fail with `RedactError::UnreadableMark` before anything changes, instead of skipping that mark. The count covers the marks on the pages being applied. diff --git a/crates/redact/src/interp.rs b/crates/redact/src/interp.rs index 1e69cb32e2..ae525989a1 100644 --- a/crates/redact/src/interp.rs +++ b/crates/redact/src/interp.rs @@ -143,6 +143,11 @@ pub(crate) struct Output { /// ([`pdfcraft_content::Parsed::ambiguous_images`]): what this scope placed after it may not /// be what the page shows, so a redaction must not trust it. pub ambiguous: bool, + /// The content holds bytes the parser can't read ([`pdfcraft_content::Parsed::skipped`]), in + /// this scope or a nested form: the walk reads past them, so text they hide (a stray `)` in a + /// `TJ` array, say), which other readers may still draw, is neither found nor removed, and a + /// rewrite would drop the bytes but keep the strings. A redaction must not trust it. + pub unparsed: bool, pub tag_changes: Vec<(ObjRef, ObjRef, HashSet, HashSet)>, pub error: Option, } @@ -407,6 +412,7 @@ fn process_from(doc: &mut Document, scope: &mut Scope<'_>, streams: &[Vec], let joined = Pieces::join(streams); let mut parsed = joined.parse(); walk.out.ambiguous = parsed.ambiguous_images > 0; + walk.out.unparsed = parsed.skipped > 0; // A rewritten piece loses its comments, so keep where they began and ended: a comment that // ran on into the next piece still hides that piece's first line. let comment_edges = joined.comment_edges(&parsed); @@ -1150,6 +1156,7 @@ fn xobject( out.residue += inner.residue; out.vertical |= inner.vertical; out.ambiguous |= inner.ambiguous; + out.unparsed |= inner.unparsed; out.doomed.extend_from_slice(&inner.doomed); out.retired_properties.extend(inner.retired_properties); // A Type 3 glyph without resources shown inside the form may paint with the page's diff --git a/crates/redact/src/lib.rs b/crates/redact/src/lib.rs index 7c1a65f55a..5938f20a68 100644 --- a/crates/redact/src/lib.rs +++ b/crates/redact/src/lib.rs @@ -791,8 +791,10 @@ fn redact_page_content( } // An inline image the renderer reads no image for (no `EI` it accepts, or parameters it // can't read): where the page's operators resume after it is a guess, so neither removal - // nor the verify pass can be trusted on this page. - if out.ambiguous { + // nor the verify pass can be trusted on this page. Likewise bytes the parser can't read in + // what the walk read, the page's content or a form under a mark: text they hide is neither + // found nor removed, and other readers may draw it. + if out.ambiguous || out.unparsed { return Err(RedactError::Unreadable(pi + 1)); } doomed_xobjects.extend_from_slice(&out.doomed); diff --git a/crates/redact/src/sanitize.rs b/crates/redact/src/sanitize.rs index 0d4ee2e18a..3da224e8ac 100644 --- a/crates/redact/src/sanitize.rs +++ b/crates/redact/src/sanitize.rs @@ -659,6 +659,17 @@ fn content_pass(doc: &mut Document, text: bool, layers: bool, write: bool) -> Re if let Some(error) = out.error { return Err(error.into()); } + // Bytes the parser can't read, or an inline image whose end is a guess: the walk reads past + // them, so hidden text or a hidden layer's content they hold, which other readers may draw, + // is neither found nor removed. As for content that doesn't decode, count the page so the + // removal runs and refuses it. + if out.unparsed || out.ambiguous { + if write { + return Err(RedactError::Unreadable(pi + 1)); + } + total += 1; + continue; + } let blocks = scope.layer_blocks; let glyphs = scope.report.glyphs; let retained_for_page = std::mem::take(&mut scope.retained_off_layers); diff --git a/crates/redact/src/tests.rs b/crates/redact/src/tests.rs index 46ac131450..4e132f20c6 100644 --- a/crates/redact/src/tests.rs +++ b/crates/redact/src/tests.rs @@ -3297,31 +3297,42 @@ fn image_and_a_tail(catalog: &str, image: &str, image_dict: &str, tail: &[u8]) - ]) } -/// The refusal of the check of what content still draws through its resources (`resource_ops`). -fn unread_content_refusal(result: &Result) -> bool { - matches!(result, Err(RedactError::Cos(pdfcraft_cos::CosError::Syntax { detail, .. })) - if detail == "redaction could not fully read the content that uses these resources") -} - -/// What the page draws can't be told when a content stream the redaction leaves as it was holds -/// bytes the parser can't read: the redaction of an image is refused, as it was already on a page -/// with fonts or properties to clean up. +/// What the page draws can't be told when content the redaction leaves as it was holds bytes the +/// parser can't read: the redaction of an image is refused. In a content stream of the page, the +/// walk reads past them and refuses the page itself (`Unreadable`). In a form without resources of +/// its own that lies away from the mark, which the walk doesn't enter, the check of what the page +/// still draws through its resources refuses it. #[test] fn unreadable_bytes_in_a_stream_left_as_it_was_refuse_the_redaction_of_an_image() { for (tail, refused) in [(&b"q Q )"[..], true), (b"q Q", false)] { let mut doc = image_and_a_tail("", "Im1", "", tail); mark(&mut doc, 0, &[[0.0, 200.0, 300.0, 300.0]], ""); - let result = apply(&mut doc, None); - if refused { - assert!(unread_content_refusal(&result), "{result:?}"); - } else { - assert_eq!(result.map(|r| r.images_removed).ok(), Some(1)); - } + let result = apply(&mut doc, None).map(|r| r.images_removed); + assert_eq!(result, if refused { Err(RedactError::Unreadable(1)) } else { Ok(1) }); } + let mut doc = image_and_a_tail("", "Im1", "", b"/Fm Do"); + let bbox = Object::Array(vec![0.into(), 0.into(), 300.into(), 100.into()]); + let form = Dict::from_iter([(b"Type".to_vec(), Object::name("XObject")), (b"Subtype".to_vec(), Object::name("Form")), (b"BBox".to_vec(), bbox)]); + let form = doc.add(Object::Stream(Stream::from_raw(form, b"q Q )".to_vec()))); + doc.update_dict(ObjRef::new(3, 0), |d| { + if let Some(Object::Dict(res)) = d.get_mut(b"Resources") + && let Some(Object::Dict(xo)) = res.get_mut(b"XObject") + { + xo.set(b"Fm".to_vec(), Object::Ref(form)); + } + }) + .unwrap(); + mark(&mut doc, 0, &[[0.0, 200.0, 300.0, 300.0]], ""); + let result = apply(&mut doc, None); + assert!( + matches!(&result, Err(RedactError::Cos(pdfcraft_cos::CosError::Syntax { detail, .. })) if detail == "redaction could not fully inspect inherited form resources"), + "{result:?}" + ); } -/// Remove Hidden Information makes the same check: removing an off layer's image is refused on a -/// page whose drawing can't be told. +/// Remove Hidden Information refuses the same page: removing an off layer's image is refused on a +/// page whose drawing can't be told. Its walk reads every content stream, and refuses the page +/// itself. #[test] fn unreadable_bytes_in_a_stream_left_as_it_was_refuse_removing_a_hidden_image() { use crate::sanitize::{Hidden, remove_hidden}; @@ -3329,7 +3340,7 @@ fn unreadable_bytes_in_a_stream_left_as_it_was_refuse_removing_a_hidden_image() let mut doc = image_and_a_tail("/OCProperties << /OCGs [7 0 R] /D << /OFF [7 0 R] >> >>", "ImOC", "/OC 7 0 R", tail); let result = remove_hidden(&mut doc, &[Hidden::HiddenLayers]); if refused { - assert!(unread_content_refusal(&result), "{result:?}"); + assert_eq!(result, Err(RedactError::Unreadable(1))); } else { assert!(result.unwrap().iter().any(|(h, n)| *h == Hidden::HiddenLayers && *n > 0)); } @@ -6152,6 +6163,7 @@ mod metadata_matching; mod property_retirement; mod surviving_references; mod text_state; +mod unparsed; /// `[(FIRSTARR)] [(LASTARR)] TJ` is malformed (a surplus array operand), but the operator still /// draws only the one immediately before it (LASTARR, 7 glyphs, 35 pt wide from x = 10) -- not diff --git a/crates/redact/src/tests/unparsed.rs b/crates/redact/src/tests/unparsed.rs new file mode 100644 index 0000000000..4fcc9fcb75 --- /dev/null +++ b/crates/redact/src/tests/unparsed.rs @@ -0,0 +1,114 @@ +//! Bytes the content parser can't read are read past by the redaction walk, so text they hide (a +//! stray `)` or `}` in a `TJ` array) is neither found nor removed, while other readers still draw +//! it. A page whose walk meets such bytes is refused, by Apply Redactions and by Remove Hidden +//! Information. + +use super::*; +use crate::sanitize::{Hidden, remove_hidden, sanitize, scan}; + +/// `TJ` arrays each holding a byte the parser can't read beside the text. +const BROKEN: [&[u8]; 4] = [b"[(TAIL) ) (SECRET)] TJ", b"[(TAIL) } (SECRET)] TJ", b"[(TAILSECRET) )] TJ", b"[) (TAILSECRET)] TJ"]; + +/// Over the top third of the 300 × 300 page. +const MARK: [f64; 4] = [0.0, 200.0, 300.0, 300.0]; + +/// `tj` shown in /F1 at (20, 260), under [`MARK`]. +fn shown(tj: &[u8]) -> Vec { + [b"BT /F1 10 Tf 20 260 Td ".as_slice(), tj, b" ET"].concat() +} + +/// The only text under the mark sits in such a `TJ`: the walk finds nothing there and rewrites +/// nothing, so it went on, and the text stayed in the file. +#[test] +fn text_under_a_mark_beside_bytes_the_parser_cannot_read_is_refused() { + for tj in BROKEN { + let mut doc = one_page(&shown(tj), "", vec![]); + mark(&mut doc, 0, &[MARK], ""); + assert_eq!(apply(&mut doc, None), Err(RedactError::Unreadable(1)), "{}", String::from_utf8_lossy(tj)); + } + // With other text on the page, outside the mark. + let mut doc = one_page(&[b"BT /F1 10 Tf 20 20 Td (PUBLIC) Tj ET ".as_slice(), &shown(BROKEN[0])].concat(), "", vec![]); + mark(&mut doc, 0, &[MARK], ""); + assert_eq!(apply(&mut doc, None), Err(RedactError::Unreadable(1)), "beside other text"); +} + +/// In a stream the walk rewrites, it removed the text it could read and wrote the broken `TJ` out +/// as `(TAIL) (SECRET) TJ`: the stray byte and the brackets went, the strings stayed. +#[test] +fn bytes_the_parser_cannot_read_in_a_stream_the_walk_rewrites_are_refused() { + for tj in BROKEN { + let mut doc = one_page(&[b"BT /F1 10 Tf 20 230 Td (SECRET) Tj ET ".as_slice(), &shown(tj)].concat(), "", vec![]); + mark(&mut doc, 0, &[MARK], ""); + assert_eq!(apply(&mut doc, None), Err(RedactError::Unreadable(1)), "{}", String::from_utf8_lossy(tj)); + } +} + +/// A form under the mark with resources of its own, which the walk enters but doesn't rewrite: +/// no later read of the page follows it. +#[test] +fn bytes_the_parser_cannot_read_in_a_form_under_a_mark_are_refused() { + let form = stream("/Type /XObject /Subtype /Form /BBox [0 0 300 300] /Resources << /Font << /F1 5 0 R >> >>", shown(BROKEN[0])); + let mut doc = one_page(b"/Fm Do", "/XObject << /Fm 7 0 R >>", vec![form]); + mark(&mut doc, 0, &[MARK], ""); + assert_eq!(apply(&mut doc, None), Err(RedactError::Unreadable(1))); +} + +/// A form the mark doesn't reach isn't walked: bytes in it can't hide anything under the mark, so +/// the page is redacted. +#[test] +fn bytes_the_parser_cannot_read_in_a_form_away_from_the_marks_are_left() { + let form = stream( + "/Type /XObject /Subtype /Form /BBox [0 0 300 100] /Resources << /Font << /F1 5 0 R >> >>", + [b"BT /F1 10 Tf 20 20 Td ".as_slice(), BROKEN[0], b" ET"].concat(), + ); + let mut doc = one_page(b"BT /F1 10 Tf 20 230 Td (SECRET) Tj ET /Fm Do", "/XObject << /Fm 7 0 R >>", vec![form]); + mark(&mut doc, 0, &[MARK], ""); + assert_eq!(apply(&mut doc, None).unwrap().glyphs, 6); + assert!(!content(&doc, 0).contains("(SECRET)")); +} + +/// The page sets its font only through an ExtGState `/Font` and lists no `/Font` resources, so the +/// cleanups after the walk, which look at the listed fonts, read nothing. +#[test] +fn bytes_the_parser_cannot_read_on_a_page_without_font_resources_are_refused() { + let mut doc = pdf(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 5 0 R] /Resources << /ExtGState << /GS1 << /Font [6 0 R 10] >> >> >> >>" + .to_vec(), + stream("", b"BT /GS1 gs 20 230 Td (SECRET) Tj ET"), + stream("", b"BT /GS1 gs 20 260 Td [(TAIL) ) (SECRET)] TJ ET"), + FONT.replace("95 0 R", "7 0 R").into_bytes(), + widths(), + ]); + mark(&mut doc, 0, &[MARK], ""); + assert_eq!(apply(&mut doc, None), Err(RedactError::Unreadable(1))); +} + +/// Hidden text inside a broken `TJ` is never counted: alone, Remove Hidden Information found +/// nothing to remove and Sanitize Document went on; beside other hidden text, the removal went on +/// and left it. The scan now counts the page, and removing hidden text refuses it. +#[test] +fn hidden_text_beside_bytes_the_parser_cannot_read_is_refused() { + let alone = b"BT /F1 10 Tf 3 Tr 20 260 Td [(TAIL) ) (HIDDEN)] TJ ET".as_slice(); + let beside = [b"BT /F1 10 Tf 3 Tr 20 230 Td (HIDDEN) Tj ET ".as_slice(), alone].concat(); + for content in [alone, &beside] { + let label = String::from_utf8_lossy(content); + let doc = one_page(content, "", vec![]); + let counts: std::collections::HashMap = scan(&doc).into_iter().collect(); + assert!(counts[&Hidden::HiddenText] > 0, "{label}"); + let mut doc = one_page(content, "", vec![]); + assert_eq!(remove_hidden(&mut doc, &[Hidden::HiddenText]), Err(RedactError::Unreadable(1)), "{label}"); + let mut doc = one_page(content, "", vec![]); + assert_eq!(sanitize(&mut doc), Err(RedactError::Unreadable(1)), "{label}"); + } +} + +/// An inline image whose end the parser guesses, as Apply Redactions already refuses it: what +/// follows it may not be what the page draws, so Remove Hidden Information refuses it too. +#[test] +fn hidden_text_beside_an_inline_image_the_renderer_cannot_end_is_refused() { + let page = b"BT /F1 10 Tf 3 Tr 10 100 Td (HIDDEN) Tj ET BI /W 1 /H 1 /CS /G /BPC 8 /F /AHx ID 7F".as_slice(); + let mut doc = one_page(page, "", vec![]); + assert_eq!(remove_hidden(&mut doc, &[Hidden::HiddenText]), Err(RedactError::Unreadable(1))); +}