From a525cf1d424de0cdf1dc50e34f1b6cfedcdda24e Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 14:12:51 +0000 Subject: [PATCH 01/22] fix(js): refuse code compiled from strings that skipped the script checks eval and the Function constructors compiled code built at run time, which never passed the size and nesting checks a script passes before it runs, so a short form script could hand boa's recursive parser a deeply nested string and overflow the stack. Form scripts now refuse to compile code from strings. XFA scripts, which the runner itself starts through eval, hold such code to the same checks as the script. --- crates/js/src/lib.rs | 42 +++++++++++++++++++++++++++++++++++++++++- crates/js/src/tests.rs | 33 +++++++++++++++++++++++++++++++++ crates/js/src/xfa.rs | 5 ++++- 3 files changed, 78 insertions(+), 2 deletions(-) diff --git a/crates/js/src/lib.rs b/crates/js/src/lib.rs index 785345c672..ae94feb0aa 100644 --- a/crates/js/src/lib.rs +++ b/crates/js/src/lib.rs @@ -27,9 +27,11 @@ use std::cell::RefCell; use std::collections::BTreeSet; use std::sync::Arc; +use boa_engine::context::HostHooks; use boa_engine::object::builtins::{JsArray, JsFunction}; use boa_engine::object::{FunctionObjectBuilder, ObjectInitializer}; use boa_engine::property::Attribute; +use boa_engine::realm::Realm; use boa_engine::{Context, JsError, JsNativeError, JsObject, JsResult, JsString, JsValue, NativeFunction, Source, js_string}; /// The kinds of field, as `field.type` names them. @@ -1128,8 +1130,46 @@ pub fn run_shared(script: &str, event: &Event, doc: &DocInfo, fields: Arc JsResult<()> { + Err(JsNativeError::typ().with_message("scripts can't compile code from strings (eval, Function)").into()) + } +} + +/// Host hooks that hold code compiled from strings to the checks a script passes ([`refuse`]), +/// for runners that start the script itself through `eval`. +pub(crate) struct CheckedCodeFromStrings; + +impl HostHooks for CheckedCodeFromStrings { + fn ensure_can_compile_strings(&self, _: Realm, parameters: &[JsString], body: &JsString, _: bool, _: &mut Context) -> JsResult<()> { + let units = parameters.iter().chain(std::iter::once(body)).fold(0usize, |n, s| n.saturating_add(s.len())); + if units > MAX_SCRIPT_BYTES { + return Err(JsNativeError::range().with_message("the code is too long to compile").into()); + } + // As one text: brackets left open in one parameter carry into the next. + let mut source = parameters.iter().map(JsString::to_std_string_lossy).collect::>().join(","); + source.push('\n'); + source.push_str(&body.to_std_string_lossy()); + match refuse(&source) { + Some(why) => Err(JsNativeError::range().with_message(why).into()), + None => Ok(()), + } + } +} + fn run_here(script: &str, event: &Event, doc: &DocInfo, fields: Arc>, doc_scripts: &[String], limits: Limits) -> Outcome { - let mut ctx = Context::default(); + let mut ctx = match Context::builder().host_hooks(std::rc::Rc::new(NoCodeFromStrings)).build() { + Ok(ctx) => ctx, + Err(e) => { + let error = Some(format!("the script engine could not start: {e}")); + return Outcome { rc: true, value: event.value.clone(), change: event.change.clone(), error, ..Default::default() }; + } + }; ctx.runtime_limits_mut().set_loop_iteration_limit(limits.loop_iterations); ctx.runtime_limits_mut().set_recursion_limit(limits.recursion); ctx.insert_data(RefCell::new(Host { diff --git a/crates/js/src/tests.rs b/crates/js/src/tests.rs index 7a9efb5dfd..5b6adef53c 100644 --- a/crates/js/src/tests.rs +++ b/crates/js/src/tests.rs @@ -313,6 +313,23 @@ fn ordinary_nesting_still_runs() { assert_eq!(go(&s, &Event::field("Calculate", "a", "")).value, "1000"); } +/// Code built from strings at run time would reach the parser without the size and nesting +/// checks the script itself passed, so `eval` and the `Function` constructors refuse it. +#[test] +fn scripts_cannot_compile_code_from_strings() { + for s in [ + "event.value = eval('1 + 1');", + "event.value = (0, eval)('1 + 1');", + "event.value = new Function('return 2')();", + "event.value = Function('a', 'return a')(2);", + "event.value = (function* () {}).constructor('yield 2')().next().value;", + "event.value = new Array(1001).join('(') + '1' + new Array(1001).join(')'); event.value = eval(event.value);", + ] { + let o = go(s, &Event::field("Calculate", "a", "")); + assert!(o.error.as_deref().is_some_and(|e| e.contains("compile code from strings")), "{s}: {o:?}"); + } +} + mod xfa_model { use crate::Limits; use crate::xfa::*; @@ -398,6 +415,22 @@ mod xfa_model { assert_eq!(o.effects, vec![XfaEffect::SetValue { som: "form1[0].page1[0].total[0]".into(), value: "5".into() }]); } + /// The script runs through `eval`, so code built from strings at run time is held to the + /// script's own size and nesting checks instead of being refused outright. + #[test] + fn code_built_from_strings_is_checked_like_the_script() { + let total = "form1[0].page1[0].total[0]"; + assert_eq!(go("eval('qty.rawValue * 2')", total, "calculate").result.as_deref(), Some("8")); + assert_eq!(go("new Function('return 3')()", total, "calculate").result.as_deref(), Some("3")); + for s in [ + "eval(new Array(1001).join('(') + '1' + new Array(1001).join(')'))", + "new Function(new Array(1001).join('[') + new Array(1001).join(']'))", + ] { + let o = go(s, total, "calculate"); + assert!(o.error.as_deref().is_some_and(|e| e.contains("nested too deeply")), "{s}: {o:?}"); + } + } + /// A form whose one field `d` holds the ISO date 2026-10-20, shown as 20261020. pub(super) fn dated_form() -> XfaNode { let d = XfaNode { formatted: Some("20261020".into()), ..field("d", "form1[0].page1[0].d[0]", "2026-10-20", false) }; diff --git a/crates/js/src/xfa.rs b/crates/js/src/xfa.rs index 5a693db9f0..490408a106 100644 --- a/crates/js/src/xfa.rs +++ b/crates/js/src/xfa.rs @@ -1473,7 +1473,10 @@ pub(crate) fn warm_up() { /// `started` is called once the engine is ready: a time limit on the script runs from then. pub(crate) fn run_here(script: &str, event: &XfaEvent, doc: &XfaDoc, root: XfaNode, limits: Limits, started: &dyn Fn()) -> XfaOutcome { warm_up(); - let mut ctx = Context::default(); + let mut ctx = match Context::builder().host_hooks(std::rc::Rc::new(crate::CheckedCodeFromStrings)).build() { + Ok(ctx) => ctx, + Err(e) => return XfaOutcome { error: Some(format!("the script engine could not start: {e}")), ..Default::default() }, + }; ctx.runtime_limits_mut().set_loop_iteration_limit(limits.loop_iterations); ctx.runtime_limits_mut().set_recursion_limit(limits.recursion); let host_state = XHost::new(root, &event.target, doc); From 5dc64face0131a62712d8f414352787e4ef92617 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 14:23:04 +0000 Subject: [PATCH 02/22] fix(automation): keep page text from tools when copying is not allowed text_lines returned a copy-restricted document's lines, ocr_recognize returned the words it recognised, and text_edit on a paragraph echoed the paragraph's own text, all without checking the copying permission that the other text tools check. text_lines is refused like them; recognising and restyling still work with permission to change the document, and leave the text out. --- crates/automation/src/a11y.rs | 8 +++-- crates/automation/src/lib.rs | 13 ++++++-- crates/automation/src/tools.rs | 4 +-- crates/automation/tests/it/automation.rs | 38 ++++++++++++++++++++++++ 4 files changed, 56 insertions(+), 7 deletions(-) diff --git a/crates/automation/src/a11y.rs b/crates/automation/src/a11y.rs index 2124549731..21bde353e9 100644 --- a/crates/automation/src/a11y.rs +++ b/crates/automation/src/a11y.rs @@ -626,13 +626,17 @@ impl Automation { pub(crate) fn ocr_recognize(&mut self, a: &Args) -> Result { let settings = self.ocr_settings(a)?; let pages = if a.opt_ints("pages")?.is_some() { self.pages(a, "pages")? } else { Vec::new() }; - let id = self.doc(a)?.id; + let doc = self.doc(a)?; + // Recognizing needs only permission to change the document; the words it found are + // returned only when copying is allowed too. + let (id, copy) = (doc.id, doc.allows_copying()); let found = self.session.recognize_text(id, &pages, settings).map_err(failed)?; let list: Vec = found .iter() .map(|p| match &p.skipped { Some(why) => json!({ "page": p.page + 1, "skipped": why }), - None => json!({ "page": p.page + 1, "words": p.words.len(), "already_on_page": p.already_on_page, "text": p.text() }), + None if copy => json!({ "page": p.page + 1, "words": p.words.len(), "already_on_page": p.already_on_page, "text": p.text() }), + None => json!({ "page": p.page + 1, "words": p.words.len(), "already_on_page": p.already_on_page }), }) .collect(); Ok(json!({ "words": found.iter().map(|p| p.words.len()).sum::(), "pages": list })) diff --git a/crates/automation/src/lib.rs b/crates/automation/src/lib.rs index c90240875b..522705bb15 100644 --- a/crates/automation/src/lib.rs +++ b/crates/automation/src/lib.rs @@ -435,7 +435,7 @@ impl Automation { "doc_revisions" => self.doc_revisions(&a)?, "text_lines" => { let page = self.page(&a)?; - let doc = self.doc(&a)?; + let doc = self.doc_to_copy(&a)?; let info = &doc.info.pages[page]; let r = |x: f32| (x as f64 * 100.0).round() / 100.0; let lines: Vec = doc @@ -709,7 +709,11 @@ impl Automation { let moved = [block.rect[0] + dx, block.rect[1] + dy, block.rect[2] + dx, block.rect[3] + dy]; let wanted: String = text.split_whitespace().collect(); let mut out = self.apply(&a, Edit::EditTextBlock { page, block: index, text, style })?; - let after = self.doc(&a)?.text_blocks(page); + let doc = self.doc(&a)?; + let after = doc.text_blocks(page); + // Without `text` the paragraph keeps the document's own words: not echoed when + // the document doesn't allow copying them. + let copy = doc.allows_copying(); // Only a paragraph that now shows exactly the requested text is the edited one. A // neighbour that overlaps the old box (overlaid text, a text layer) must not take // its number, and neither must a paragraph the edit merged it into: rewriting that @@ -717,7 +721,10 @@ impl Automation { let edited = block_overlapping(&after, moved) .filter(|&i| !wanted.is_empty() && after.get(i).is_some_and(|b| b.text.split_whitespace().collect::() == wanted)); if let Some(b) = edited.and_then(|i| after.get(i)) { - out["paragraph"] = json!({ "text": b.text, "lines": b.lines.len(), "font": b.base_font }); + out["paragraph"] = json!({ "lines": b.lines.len(), "font": b.base_font }); + if copy { + out["paragraph"]["text"] = json!(b.text); + } } let stale = self.follow_edit(id, page, k, edited.and_then(|i| after.get(i)), &after); if !stale.is_empty() { diff --git a/crates/automation/src/tools.rs b/crates/automation/src/tools.rs index 9ce6b12d4f..4fff59c07d 100644 --- a/crates/automation/src/tools.rs +++ b/crates/automation/src/tools.rs @@ -1172,7 +1172,7 @@ pub fn tools() -> Vec { )), t("js_enabled", "JavaScript on or off", "Preferences ▸ JavaScript ▸ Enable Acrobat JavaScript: set it with `enabled`, or read it. With JavaScript off, field scripts other than Acrobat's AF calls don't run.") .with(schema(json!({ "enabled": { "type": "boolean" } }), &[])), - t("ocr_recognize", "Recognize text (OCR)", "Scan & OCR ▸ Recognize text: render pages, read the words in them and add them as invisible text over the page image, so scanned pages become searchable and selectable (a searchable image; the image is not changed). Pages that already have text are skipped unless skip_text_pages is false; then pictures on those pages are read too, and recognised words that the page's own text already has (an earlier OCR layer included) are left out and counted as already_on_page. Returns each page's recognised text, word count or why it was skipped. Needs the OCR models (ocr_status). Undoable as one step.") + t("ocr_recognize", "Recognize text (OCR)", "Scan & OCR ▸ Recognize text: render pages, read the words in them and add them as invisible text over the page image, so scanned pages become searchable and selectable (a searchable image; the image is not changed). Pages that already have text are skipped unless skip_text_pages is false; then pictures on those pages are read too, and recognised words that the page's own text already has (an earlier OCR layer included) are left out and counted as already_on_page. Returns each page's recognised text (left out when the document's security doesn't allow copying), word count or why it was skipped. Needs the OCR models (ocr_status). Undoable as one step.") .cmd("ocr.recognize") .with(schema( json!({ @@ -1332,7 +1332,7 @@ pub fn tools() -> Vec { .ro() .cmd("edit.edit_text") .with(schema(json!({ "doc": doc(), "page": { "type": "integer", "minimum": 1 }, "paragraph": { "type": "integer", "minimum": 1 }, "text": { "type": "string" }, "size": { "type": "number" }, "width": { "type": "number" }, "font_resource":{"type":"string"}, "ranges":{"type":"array","maxItems":1024,"items":{"type":"object"}} }), &["doc", "page", "paragraph"])), - t("text_edit", "Edit text", "Replace the text of one paragraph (paragraph, from text_paragraphs: rewrapped to the paragraph's width with its line spacing) or one line (line, from text_lines) in place, keeping position, size and colour. For a mixed-style region, editor_text supplies visual row breaks; surviving text keeps its source-run styles and inserted text inherits the surrounding run. For a paragraph, also change its formatting: font (helvetica, times, courier) with bold/italic, size (points), color (#rrggbb), align (left, center, right, justify), underline, line_spacing (× size), char_spacing (points) and scale (horizontal, percent), or move it (dx, dy in points; up is +dy) and rewrap it to a new width (points); text may then be omitted. Its own font is reused when it can show every character and no font/weight change is requested; otherwise it uses a standard font matching the source style or, for text no standard font can show, real fallback outlines from PdfCraft's own faces, from the one that has every character: the craft-fonts Japanese faces (Mincho/Gothic matching the requested/source family and available weight) or, behind them, the bundled Inter face, which carries the Latin, Greek and Cyrillic letters they lack (Uzbek қ, Kazakh ә). The result shows the font used. Missing Japanese weights use Regular, not synthetic bold; the small web build has only Gothic Regular. Paragraph bold may be set without font to keep the source family. Text that no available font can show is refused. A paragraph number is refused when an earlier edit merged that paragraph into a neighbour (list the page again); the result's stale_paragraphs names the numbers gone. Undoable.") + t("text_edit", "Edit text", "Replace the text of one paragraph (paragraph, from text_paragraphs: rewrapped to the paragraph's width with its line spacing) or one line (line, from text_lines) in place, keeping position, size and colour. For a mixed-style region, editor_text supplies visual row breaks; surviving text keeps its source-run styles and inserted text inherits the surrounding run. For a paragraph, also change its formatting: font (helvetica, times, courier) with bold/italic, size (points), color (#rrggbb), align (left, center, right, justify), underline, line_spacing (× size), char_spacing (points) and scale (horizontal, percent), or move it (dx, dy in points; up is +dy) and rewrap it to a new width (points); text may then be omitted. Its own font is reused when it can show every character and no font/weight change is requested; otherwise it uses a standard font matching the source style or, for text no standard font can show, real fallback outlines from PdfCraft's own faces, from the one that has every character: the craft-fonts Japanese faces (Mincho/Gothic matching the requested/source family and available weight) or, behind them, the bundled Inter face, which carries the Latin, Greek and Cyrillic letters they lack (Uzbek қ, Kazakh ә). The result shows the font used, and for a paragraph its text unless the document's security doesn't allow copying. Missing Japanese weights use Regular, not synthetic bold; the small web build has only Gothic Regular. Paragraph bold may be set without font to keep the source family. Text that no available font can show is refused. A paragraph number is refused when an earlier edit merged that paragraph into a neighbour (list the page again); the result's stale_paragraphs names the numbers gone. Undoable.") .cmd("edit.edit_text") .with(schema( json!({ "doc": doc(), "page": { "type": "integer", "minimum": 1 }, "line": { "type": "integer", "minimum": 1 }, diff --git a/crates/automation/tests/it/automation.rs b/crates/automation/tests/it/automation.rs index 0ebf3a9bc0..778deaaec4 100644 --- a/crates/automation/tests/it/automation.rs +++ b/crates/automation/tests/it/automation.rs @@ -2130,6 +2130,44 @@ fn protecting_through_tools() { ok(&mut c, "doc_open", json!({ "path": "open.pdf" })); } +/// A document that allows changes but not copying: the tools that return a page's text refuse, +/// or leave the text out where the tool also changes the page (a restyled paragraph, recognized +/// words). +#[test] +fn copy_restricted_text_stays_in_the_document() { + let dir = workdir("copy-restricted-text"); + let mut a = auto(&dir); + let doc = ok(&mut a, "doc_open", json!({ "path": "a.pdf" }))["doc"].as_u64().unwrap(); + let locked = json!({ "open_password": "open", "permissions_password": "boss", "changes": "any-except-extract" }); + let mut protect = locked.clone(); + protect["doc"] = json!(doc); + ok(&mut a, "doc_protect", protect); + ok(&mut a, "doc_save", json!({ "doc": doc, "path": "locked.pdf" })); + let re = ok(&mut a, "doc_open", json!({ "path": "locked.pdf", "password": "open" }))["doc"].as_u64().unwrap(); + match a.call("text_lines", &json!({ "doc": re, "page": 1 })) { + Err(ToolError::Failed(m)) => assert!(m.contains("don't allow copying"), "{m}"), + other => panic!("text_lines is refused: {other:?}"), + } + let restyled = ok(&mut a, "text_edit", json!({ "doc": re, "page": 1, "paragraph": 1, "size": 30 })); + assert!(!restyled.to_string().contains("Page"), "{restyled}"); + if ok(&mut a, "ocr_status", json!({}))["available"] != true { + eprintln!("skipped the OCR part: OCR models not installed"); + return; + } + let text = ok(&mut a, "doc_create", json!({ "from": "text", "text": "Recognised words stay private" }))["doc"].as_u64().unwrap(); + ok(&mut a, "doc_export_images", json!({ "doc": text, "folder": "scan", "dpi": 150, "pages": [1] })); + let png = std::fs::read_dir(dir.join("scan")).unwrap().next().unwrap().unwrap().path(); + let scan = ok(&mut a, "doc_create", json!({ "from": "images", "paths": [png.to_string_lossy()] }))["doc"].as_u64().unwrap(); + let mut protect = locked; + protect["doc"] = json!(scan); + ok(&mut a, "doc_protect", protect); + ok(&mut a, "doc_save", json!({ "doc": scan, "path": "scan.pdf" })); + let scan = ok(&mut a, "doc_open", json!({ "path": "scan.pdf", "password": "open" }))["doc"].as_u64().unwrap(); + let r = ok(&mut a, "ocr_recognize", json!({ "doc": scan })); + assert!(r["words"].as_u64().unwrap() >= 3, "{r}"); + assert!(r["pages"][0].get("text").is_none() && !r.to_string().to_lowercase().contains("private"), "{r}"); +} + /// A form secured with "fill-sign" changes and no open password (how secured forms are usually /// distributed): its existing signature field can be signed without the permissions password, /// the update keeps the encryption, and the signature validates after reopening. A new field From 30300ae71be58cc5e1800cdbd19c9ec432f7a0ee Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 14:37:46 +0000 Subject: [PATCH 03/22] fix(engine): compare only documents that allow copying their text Compare read both documents' words out with no check of the copying permission, and its result, report and difference comments carry that text, so a copy-restricted document's text could be taken out through a comparison. Both documents must now allow copying. --- crates/engine/src/compare.rs | 5 ++++- crates/engine/src/tests.rs | 6 ++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/crates/engine/src/compare.rs b/crates/engine/src/compare.rs index 50508e20ea..c23db91f1f 100644 --- a/crates/engine/src/compare.rs +++ b/crates/engine/src/compare.rs @@ -46,10 +46,13 @@ impl crate::Document { } impl Session { - /// Compare files: the text differences from document `old` to document `new`. + /// Compare files: the text differences from document `old` to document `new`. The result + /// carries both documents' text, so both must allow copying it. pub fn compare(&self, old: DocId, new: DocId) -> Result { let a = self.get(old).ok_or(EditError::NoDocument)?; let b = self.get(new).ok_or(EditError::NoDocument)?; + a.check_copying()?; + b.check_copying()?; Ok(pdfcraft_compare::compare(&a.words(), &b.words())) } diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 0df176226a..f93e765260 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -1311,6 +1311,12 @@ fn copy_restricted_documents_refuse_extraction_unless_opened_by_the_owner() { let oid = s.open("r.pdf", None, bytes, Some("owner")).unwrap(); assert!(s.get(oid).unwrap().allows_copying()); assert!(extracting.iter().all(|c| enabled(&s, oid, c))); + // Comparing reads both documents' text out, into the result, the report and the comments. + let refused = || Err(EditError::NotPermitted("copying text and images")); + assert_eq!((s.compare(id, oid).map(|_| ()), s.compare(oid, id).map(|_| ())), (refused(), refused())); + assert_eq!(s.compare_report(oid, id).map(|_| ()), refused()); + assert!(s.compare(oid, oid).is_ok()); + assert_eq!(s.mark_differences(id, oid).map(|_| ()), refused()); // Unencrypted documents allow everything. let (s, plain) = session_with(1); assert!(s.get(plain).unwrap().check_copying().is_ok() && extracting.iter().all(|c| enabled(&s, plain, c))); From 28ab616eb62a7ddc13b32895cd2215146f7ff812 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 14:37:55 +0000 Subject: [PATCH 04/22] fix(engine): print as low-resolution pictures when only that is allowed Printing checked the print permission but not the separate high-quality one (Table 22, bit 12), so a document limited to low-resolution printing came out as its original vector pages, text included. Such a document now always prints as pictures of at most 150 dpi, whatever the settings ask for. --- crates/engine/src/lib.rs | 12 +++++++++++- crates/engine/src/tests.rs | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index e20cbc596e..fc1a8d2074 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -232,6 +232,10 @@ pub struct DocId(pub u64); /// Undo/redo depth. Snapshots share unchanged data, so this is memory-cheap. const MAX_UNDO: usize = 100; +/// The highest resolution a document that allows only low-resolution printing (Table 22, bit 12 +/// clear) is printed at. +const LOW_RESOLUTION_PRINT_DPI: f64 = 150.0; + /// The passwords that go with one state of a document (protection can change them). #[derive(Clone, Debug, Default, PartialEq)] struct Keys { @@ -4522,9 +4526,15 @@ impl Session { if !doc.allows_printing() { return Err(EditError::NotPermitted("printing")); } + // Table 22, bit 12 clear: only a low-level representation may be printed, so the pages + // always go as pictures, at no more than the low resolution. + let low_res = doc.permissions().is_some_and(|p| !p.print_high_quality()).then(|| print::Raster { + dpi: settings.raster.map_or(LOW_RESOLUTION_PRINT_DPI, |r| r.dpi.min(LOW_RESOLUTION_PRINT_DPI)), + grayscale: settings.raster.is_some_and(|r| r.grayscale), + }); // Print as image: the pages used become pictures first, then the picture document is // imposed in their place (its page k is the k-th distinct source page used). - if let Some(raster) = &settings.raster { + if let Some(raster) = low_res.as_ref().or(settings.raster.as_ref()) { let mut distinct: Vec = Vec::new(); for &p in &settings.pages { if !distinct.contains(&p) { diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index f93e765260..2d67305445 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -1340,6 +1340,38 @@ fn print_restricted_documents_disable_print_unless_opened_by_the_owner() { assert!(enabled(&s, low, "print.dialog"), "low-resolution printing is printing"); } +/// Table 22, bit 12 clear: printing is limited to a low-level representation. Whatever the +/// settings ask for, the print-ready PDF carries each page as a picture of at most 150 dpi, with +/// no text or vector artwork left; the owner password lifts the limit. +#[test] +fn low_resolution_printing_prints_pictures_of_at_most_150_dpi() { + let bytes = protected("", "owner", -1 ^ (1 << 11)); + let mut s = Session::new(); + let low = s.open("low.pdf", None, bytes.clone(), None).unwrap(); + let width = f64::from(s.get(low).unwrap().info.pages[0].width); + let actual = print::Settings { pages: vec![0], layout: print::Layout::Size(print::SizeMode::Actual), ..Default::default() }; + for raster in [None, Some(print::Raster { dpi: 600.0, grayscale: false })] { + let printed = s.print_pdf(low, &print::Settings { raster, ..actual.clone() }).unwrap(); + let mut s2 = Session::new(); + let id2 = s2.open("print.pdf", None, Arc::new(printed.clone()), None).unwrap(); + assert!(page_texts(&s2, id2).iter().all(|t| t.trim().is_empty()), "{raster:?}: no text is left"); + let doc = pdfcraft_cos::Document::open(Arc::new(printed)).unwrap(); + let widths: Vec = doc + .scan_objects() + .filter_map(|(_, o)| match &*o { + pdfcraft_cos::Object::Stream(st) if st.dict.name(b"Subtype") == Some(b"Image") => st.dict.int(b"Width"), + _ => None, + }) + .collect(); + assert!(!widths.is_empty() && widths.iter().all(|&w| w as f64 <= width * 150.0 / 72.0 + 1.0), "{raster:?}: {widths:?} px for {width} pt"); + } + let owner = s.open("low.pdf", None, bytes, Some("owner")).unwrap(); + let printed = s.print_pdf(owner, &actual).unwrap(); + let mut s2 = Session::new(); + let id2 = s2.open("print.pdf", None, Arc::new(printed), None).unwrap(); + assert!(page_texts(&s2, id2).iter().any(|t| !t.trim().is_empty()), "the owner prints the pages themselves"); +} + #[test] fn the_permissions_password_unlocks_an_open_document() { // Only a permissions password (#995, #1077): opens without asking. Changes are allowed, From 73f11b2a29075094381d0a9d4c1bbcdb90627c8e Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 14:37:55 +0000 Subject: [PATCH 05/22] fix(engine): fail an action whose information-removal step fails A step that applies an edit logged any error and carried on, which is right for a flatten with nothing to flatten but not for removing hidden information or sanitizing: on a signed file the removal was refused, and the unchanged file was written out as the action's successful result. Those steps now fail the action unless there was nothing to remove. --- crates/engine/src/actions.rs | 6 +++++- crates/engine/src/tests.rs | 18 ++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/crates/engine/src/actions.rs b/crates/engine/src/actions.rs index f679cf87cd..f7d11d47ed 100644 --- a/crates/engine/src/actions.rs +++ b/crates/engine/src/actions.rs @@ -191,9 +191,13 @@ fn apply_step(s: &mut Session, id: DocId, step: &Step, log: &mut Vec) -> return Ok(None); } }; - // Steps that find nothing to do (no comments to flatten, …) aren't failures. + // Steps that find nothing to do (no comments to flatten, …) aren't failures. A removal step + // that didn't remove is: the file would be passed on as cleaned with the data still in it. + let removes = matches!(step, Step::RemoveHiddenInformation | Step::Sanitize); match s.apply(id, edit) { Ok(()) => log.push(step.label().to_string()), + Err(EditError::Redact(pdfcraft_redact::RedactError::NothingToApply)) if removes => log.push(format!("{}: nothing to remove", step.label())), + Err(e) if removes => return Err(e), Err(e) => log.push(format!("{}: {e}", step.label())), } Ok(None) diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 2d67305445..217d8d166b 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -4274,6 +4274,24 @@ fn actions_run_their_steps_on_files() { assert!(actions::run_on(&custom, "bad.pdf", Arc::new(b"nope".to_vec()), |_, _| {}).is_err()); } +/// A removal step that fails (here: a signed file can't be rewritten without its hidden data) +/// fails the action, so the file is not passed on as cleaned. +#[test] +fn actions_fail_when_hidden_information_could_not_be_removed() { + let p12 = std::fs::read(concat!(env!("CARGO_MANIFEST_DIR"), "/../sign/tests/data/ec-p256.p12")).unwrap(); + let digital_id = sign::pkcs12::open(&p12, "test").unwrap(); + let mut s = Session::new().with_clock(|| 1_800_000_000); + let id = s.open("fixture.pdf", None, Arc::new(fixture(2)), None).unwrap(); + s.apply(id, Edit::SetInfo { key: "Author".into(), value: "Private Name".into() }).unwrap(); + let saved = s.save_bytes(id).unwrap(); + s.mark_saved(id, saved, None).unwrap(); + let signed = s.sign(id, &digital_id, SignOptions { page: 1, ..SignOptions::default() }).unwrap(); + let built = actions::builtin(); + let sanitize = built.iter().find(|a| a.name == "Sanitize Documents").unwrap(); + let e = actions::run_on(sanitize, "signed.pdf", signed, |_, _| {}).map(|r| r.log).unwrap_err(); + assert!(e.starts_with("Sanitize document:"), "{e}"); +} + #[test] fn exporting_office_files_keeps_images() { let mut s = Session::new(); From c3bc881607b71be14cdccbba50fc5e81eb54247d Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 14:39:40 +0000 Subject: [PATCH 06/22] fix(platform): a saved file keeps its group along with its mode The atomic writer gave the new file the replaced file's mode but left it in the saver's own group, so a file readable by one group became readable by another. The staging file joins the replaced file's group before it gets the mode and before anything is written; where it can't, group and others get no access. --- crates/platform/src/staging.rs | 69 +++++++++++++++++++++++++++++----- 1 file changed, 60 insertions(+), 9 deletions(-) diff --git a/crates/platform/src/staging.rs b/crates/platform/src/staging.rs index 0b727449cb..f6a98c4fe4 100644 --- a/crates/platform/src/staging.rs +++ b/crates/platform/src/staging.rs @@ -31,16 +31,16 @@ pub fn create_staging( create_staging_as(dir, name, form, suffixes, None) } -/// [`create_staging`], giving the new file the permissions `keep` on Unix. It is created readable -/// and writable by its owner only and then given `keep`, so its mode is never more open than that -/// of the file it will replace, not even while it is still empty (a handle opened then would keep -/// reading what is written later). Only the mode is copied: the owner and group are the saver's. +/// [`create_staging`], giving the new file the mode and group of the file `keep` describes on +/// Unix. It is created readable and writable by its owner only and then given them, so its mode +/// is never more open than that of the file it will replace, not even while it is still empty (a +/// handle opened then would keep reading what is written later). The owner is the saver's. fn create_staging_as( dir: &Path, name: &str, form: StagingName, suffixes: impl IntoIterator, - keep: Option<&std::fs::Permissions>, + keep: Option<&std::fs::Metadata>, ) -> std::io::Result<(PathBuf, std::fs::File)> { // At most 128 bytes of the target's name, cut between characters, so the staging name fits // the 255-byte (Linux, macOS) and 255-unit (Windows) limits however long that name is. @@ -68,7 +68,7 @@ fn create_staging_as( if let Some(keep) = keep { // A volume that can't change modes (FAT, some network shares) leaves the file // owner-only: stricter than the file it replaces, never looser. - let _ = file.set_permissions(keep.clone()); + let _ = file.set_permissions(kept_permissions(&file, keep)); } return Ok((tmp, file)); } @@ -81,6 +81,26 @@ fn create_staging_as( Err(std::io::Error::new(std::io::ErrorKind::AlreadyExists, "every temporary file name tried is taken")) } +/// The permissions a staging file gets to replace the file `keep` describes. The mode's group bits +/// are meant for that file's group, so the staging file joins it first. Where it can't (the saver +/// isn't a member), group and others get nothing: someone the group bits shut out would +/// otherwise read the file as one of the others. +#[cfg(unix)] +fn kept_permissions(file: &std::fs::File, keep: &std::fs::Metadata) -> std::fs::Permissions { + use std::os::unix::fs::{MetadataExt as _, PermissionsExt as _}; + let mut mode = keep.permissions(); + let joined = file.metadata().is_ok_and(|m| m.gid() == keep.gid()) || std::os::unix::fs::fchown(file, None, Some(keep.gid())).is_ok(); + if !joined { + mode.set_mode(mode.mode() & !0o077); + } + mode +} + +#[cfg(not(unix))] +fn kept_permissions(_: &std::fs::File, keep: &std::fs::Metadata) -> std::fs::Permissions { + keep.permissions() +} + /// Unpredictable staging-name suffixes, so a name can't be planted in advance. Each is the /// standard library's keyed hash of a counter (`RandomState::hash_one`, currently SipHash-1-3), /// keyed from the operating system's random source. That is all a staging name needs, but it is @@ -98,7 +118,7 @@ pub fn staging_suffixes() -> impl Iterator { /// and the target is untouched until the new file is complete. The target's folder must exist; /// a folder at the target is refused by the rename. Flushing is the staging file's own /// (`sync_all`); parent-directory durability after a power loss is not promised. On Unix a file -/// that is replaced keeps its permissions, so a private file stays private; a new file gets the +/// that is replaced keeps its permissions and group, so a private file stays private; a new file gets the /// usual mode for a new file. On Windows and macOS a replaced file keeps its creation date, where /// the volume stores one. /// On macOS, accessible extended attributes of an existing regular destination are copied @@ -126,9 +146,9 @@ pub fn write_atomically_with( } let dir = path.parent().filter(|d| !d.as_os_str().is_empty()).unwrap_or(Path::new(".")); let name = path.file_name().and_then(|n| n.to_str()).unwrap_or("save"); - // A file that is replaced keeps its permissions (#1790). Unix modes only: Windows is as before. + // A file that is replaced keeps its permissions (#1790) and group. Unix only: Windows is as before. #[cfg(unix)] - let keep = std::fs::metadata(path).ok().map(|m| m.permissions()); + let keep = std::fs::metadata(path).ok(); #[cfg(not(unix))] let keep = None; // A saved file is the same document: it keeps its creation date (#2658). @@ -583,6 +603,37 @@ mod tests { let _ = std::fs::remove_dir_all(&dir); } + /// A replaced file keeps its group as well as its mode, so group permissions never reach a + /// group that couldn't read the file before (the saver's own group, by default). + #[cfg(target_os = "linux")] + #[test] + fn a_replaced_file_keeps_its_group() { + use std::os::unix::fs::{MetadataExt as _, PermissionsExt as _}; + let dir = test_dir("group"); + let target = dir.join("out.pdf"); + std::fs::write(&target, b"OLD").unwrap(); + let created = std::fs::metadata(&target).unwrap().gid(); + let status = std::fs::read_to_string("/proc/self/status").unwrap(); + let groups = status.lines().find_map(|l| l.strip_prefix("Groups:")).unwrap_or_default(); + let Some(other) = groups.split_whitespace().filter_map(|g| g.parse::().ok()).find(|&g| g != created) else { + eprintln!("skipped: the test needs a second group"); + return; + }; + std::os::unix::fs::chown(&target, None, Some(other)).unwrap(); + std::fs::set_permissions(&target, std::fs::Permissions::from_mode(0o640)).unwrap(); + let mut staged = (0, 0); + write_atomically_with(&target, b"NEW", StagingName::SuffixThenTag, [7], |f| { + let m = f.metadata()?; + staged = (m.gid(), m.permissions().mode() & 0o777); + Ok(()) + }) + .unwrap(); + assert_eq!(staged, (other, 0o640), "the staging file has the target's group before it is flushed"); + let m = std::fs::metadata(&target).unwrap(); + assert_eq!((m.gid(), m.permissions().mode() & 0o777), (other, 0o640)); + let _ = std::fs::remove_dir_all(&dir); + } + /// #2658: a save replaces the file, but the document is the same one: it keeps its creation /// date where the system lets a program set it. #[cfg(any(windows, target_os = "macos"))] From 77480498b25f34028198c32f15afb50516664a73 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 17:04:07 +0000 Subject: [PATCH 07/22] fix(engine): low-resolution printing prints only the chosen content Pages printed as pictures were drawn with everything on them, so form fields only printed the page content, and the document with stamps printed every comment. The pictures are drawn from a copy holding only what the vector print draws. --- crates/engine/src/lib.rs | 3 +- crates/engine/src/print_raster.rs | 26 +++++++---- crates/engine/src/tests.rs | 77 +++++++++++++++++++++++++++++++ crates/print/src/lib.rs | 19 ++++++++ 4 files changed, 114 insertions(+), 11 deletions(-) diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index fc1a8d2074..817eb79a96 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -4541,8 +4541,7 @@ impl Session { distinct.push(p); } } - let hide_comments = matches!(settings.content, print::Content::Document | print::Content::FormFieldsOnly); - let bytes = print_raster::rasterize(doc, &distinct, raster, hide_comments).map_err(EditError::Print)?; + let bytes = print_raster::rasterize(doc, &distinct, raster, settings.content).map_err(EditError::Print)?; let pictures = pdfcraft_cos::Document::open(Arc::new(bytes)).map_err(|e| EditError::Print(e.to_string()))?; let remapped = print::Settings { pages: settings.pages.iter().map(|p| distinct.iter().position(|d| d == p).unwrap_or(0)).collect(), diff --git a/crates/engine/src/print_raster.rs b/crates/engine/src/print_raster.rs index 117c8e14cc..4ed5895860 100644 --- a/crates/engine/src/print_raster.rs +++ b/crates/engine/src/print_raster.rs @@ -1,25 +1,33 @@ //! Print as image (Acrobat's Advanced Print Setup ▸ Print As Image): every page //! sent to the printer as a picture at a chosen resolution, for printers that skip text, //! shading or transparency. The pages are rendered the way the viewer shows them (reading -//! colours off, the print's own colours), made into an image-only document, and that document -//! is imposed on the paper like any other. The print is no longer searchable text. +//! colours off, the print's own colours) with only what the print's content choice keeps, made +//! into an image-only document, and that document is imposed on the paper like any other. The +//! print is no longer searchable text. +use pdfcraft_cos::{SaveOptions, write_incremental}; use pdfcraft_create::ImageResolution; -use pdfcraft_print::Raster; +use pdfcraft_print::{Content, Raster}; use pdfcraft_render::{PRINT_TAG, PageRenderer, RenderRequest}; -use crate::Document; use crate::export::encode_png; +use crate::{Document, guard}; /// `doc`'s pages `pages` (0-based, each once, in the order given) as an image-only PDF, one -/// page per entry, the size of the source page as displayed. Pages too large for the renderer -/// at `raster.dpi` render at the largest resolution it allows. -pub fn rasterize(doc: &Document, pages: &[usize], raster: &Raster, hide_comments: bool) -> Result, String> { +/// page per entry, the size of the source page as displayed, drawing what `content` prints. +/// Pages too large for the renderer at `raster.dpi` render at the largest resolution it allows. +pub fn rasterize(doc: &Document, pages: &[usize], raster: &Raster, content: Content) -> Result, String> { let dpi = raster.dpi.clamp(72.0, 600.0); let src = doc.export_source(); let mut config = src.config.clone(); - config.hide_comments = hide_comments; - let mut renderer = PageRenderer::new(src.bytes, config); + // The copy below holds only what prints, so the viewer's own comment hiding plays no part. + config.hide_comments = false; + let printed = guard(|| -> Result, String> { + let mut cos = pdfcraft_cos::Document::open_with_password(src.bytes.clone(), config.password.as_deref()).map_err(|e| e.to_string())?; + pdfcraft_print::keep_printed(&mut cos, content); + write_incremental(&cos, &SaveOptions::default()).map_err(|e| e.to_string()) + })??; + let mut renderer = PageRenderer::new(std::sync::Arc::new(printed), config); let mut images: Vec<(String, Vec)> = Vec::with_capacity(pages.len()); let mut used: Option = None; for &page in pages { diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 217d8d166b..3d347e6492 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -1372,6 +1372,83 @@ fn low_resolution_printing_prints_pictures_of_at_most_150_dpi() { assert!(page_texts(&s2, id2).iter().any(|t| !t.trim().is_empty()), "the owner prints the pages themselves"); } +/// A 100 × 400 pt page with one band per kind of thing that prints, top to bottom: page content +/// (blue), a filled text field (green), a stamp (red) and a square comment (yellow). +fn print_content_pdf() -> Vec { + let content = "0 0 1 rg 0 300 100 100 re f"; + let ap = |colour: &str| { + let ops = format!("{colour} rg 0 0 100 100 re f"); + format!("<< /Type /XObject /Subtype /Form /BBox [0 0 100 100] /Length {} >> stream\n{ops}\nendstream", ops.len()) + }; + format!( + "%PDF-1.7\n\ + 1 0 obj << /Type /Catalog /Pages 2 0 R /AcroForm << /Fields [5 0 R] >> >> endobj\n\ + 2 0 obj << /Type /Pages /Kids [3 0 R] /Count 1 >> endobj\n\ + 3 0 obj << /Type /Page /Parent 2 0 R /MediaBox [0 0 100 400] /Contents 4 0 R /Annots [5 0 R 6 0 R 7 0 R] >> endobj\n\ + 4 0 obj << /Length {} >> stream\n{content}\nendstream endobj\n\ + 5 0 obj << /Type /Annot /Subtype /Widget /FT /Tx /T (Name) /V (Filled) /F 4 /P 3 0 R /Rect [0 200 100 300] /AP << /N 8 0 R >> >> endobj\n\ + 6 0 obj << /Type /Annot /Subtype /Stamp /F 4 /P 3 0 R /Rect [0 100 100 200] /AP << /N 9 0 R >> >> endobj\n\ + 7 0 obj << /Type /Annot /Subtype /Square /F 4 /P 3 0 R /Rect [0 0 100 100] /AP << /N 10 0 R >> >> endobj\n\ + 8 0 obj {} endobj\n\ + 9 0 obj {} endobj\n\ + 10 0 obj {} endobj\n\ + trailer << /Root 1 0 R >>\n%%EOF", + content.len(), + ap("0 1 0"), + ap("1 0 0"), + ap("1 1 0"), + ) + .into_bytes() +} + +/// Low-resolution printing prints what the chosen content prints when the pages themselves are +/// printed: form fields only leaves out the page content and every comment, the document alone +/// leaves out the comments, and the document with stamps keeps only the stamps. +#[test] +fn low_resolution_printing_prints_only_the_chosen_content() { + let mut cos = pdfcraft_cos::Document::open(Arc::new(print_content_pdf())).unwrap(); + cos.set_encryption(&pdfcraft_cos::NewEncryption { + algorithm: pdfcraft_cos::Algorithm::Aes256, + user_password: "", + owner_password: "owner", + permissions: -1 ^ (1 << 11), + encrypt_metadata: true, + seed: [7; 32], + }) + .unwrap(); + let bytes = Arc::new(pdfcraft_cos::write_full(&cos, &Default::default()).unwrap()); + let mut s = Session::new(); + let low = s.open("low.pdf", None, bytes.clone(), None).unwrap(); + let owner = s.open("low.pdf", None, bytes, Some("owner")).unwrap(); + // Each band's centre, in pixels from the top of the sheet at 72 dpi, and its colour. + let bands = [("content", 50, [0, 0, 255]), ("field", 150, [0, 255, 0]), ("stamp", 250, [255, 0, 0]), ("comment", 350, [255, 255, 0])]; + let all: &[&str] = &["content", "field", "stamp", "comment"]; + for (content, shown) in [ + (print::Content::DocumentAndMarkups, all), + (print::Content::Document, &["content", "field"][..]), + (print::Content::DocumentAndStamps, &["content", "field", "stamp"][..]), + (print::Content::FormFieldsOnly, &["field"][..]), + ] { + let settings = print::Settings { + pages: vec![0], + paper: (100.0, 400.0), + layout: print::Layout::Size(print::SizeMode::Actual), + content, + ..Default::default() + }; + for (id, how) in [(owner, "pages"), (low, "pictures")] { + let (w, h, rgba) = printed_pixels(s.print_pdf(id, &settings).unwrap()); + assert_eq!((w, h), (100, 400), "{content:?} as {how}"); + for (band, y, colour) in bands { + let px = rgba.get((y * w as usize + 50) * 4..).and_then(|p| p.get(..3)).unwrap(); + let near = |c: [u8; 3]| px.iter().zip(c).all(|(&a, b)| a.abs_diff(b) < 40); + let expected = shown.contains(&band); + assert!(if expected { near(colour) } else { near([255; 3]) }, "{content:?} as {how}: {band} drawn {expected} expected, pixel {px:?}"); + } + } + } +} + #[test] fn the_permissions_password_unlocks_an_open_document() { // Only a permissions password (#995, #1077): opens without asking. Changes are allowed, diff --git a/crates/print/src/lib.rs b/crates/print/src/lib.rs index e935cb938e..6839e5f6b0 100644 --- a/crates/print/src/lib.rs +++ b/crates/print/src/lib.rs @@ -696,6 +696,25 @@ fn decoded_contents(doc: &Document, page: &Dict) -> Vec { out } +/// Reduce `doc`'s pages to what [`impose`] draws of them under `content`: the annotations that +/// don't print are dropped, and with [`Content::FormFieldsOnly`] the page content too. For +/// printing pages as pictures, where a renderer draws each page as it stands. +pub fn keep_printed(doc: &mut Document, content: Content) { + for page in pdfcraft_model::pages(doc) { + let annots = page.dict.get(b"Annots").and_then(|a| doc.resolve(a).as_array().cloned()).unwrap_or_default(); + let kept: Vec = annots.into_iter().filter(|a| doc.resolve(a).as_dict().is_some_and(|d| prints(doc, d, content))).collect(); + // Fails only when the page object isn't a dictionary, and such a page draws nothing. + let _ = doc.update_dict(page.obj, |d| { + if content == Content::FormFieldsOnly { + d.remove(b"Contents"); + } + if d.contains(b"Annots") { + d.set(b"Annots".to_vec(), Object::Array(kept)); + } + }); + } +} + /// Does annotation `d` print under `content`? fn prints(doc: &Document, d: &Dict, content: Content) -> bool { const HIDDEN: i64 = 2; From 4aad36cf199aa7d5dd568a695dd810e6dc665a43 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 14:05:41 +0000 Subject: [PATCH 08/22] fix(create): refuse an indexed colour space whose base is indexed before following it Image export parsed an Indexed colour space's base before checking whether that base was itself Indexed. A base that refers back to the same array recursed until the stack overflowed. The base is now checked first; since Indexed is the only colour space that recurses, this also ends any cycle. --- crates/create/src/extract.rs | 11 +++++++++-- crates/create/src/tests.rs | 16 ++++++++++++++++ 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/crates/create/src/extract.rs b/crates/create/src/extract.rs index 768da4d1f1..bd5ab5271c 100644 --- a/crates/create/src/extract.rs +++ b/crates/create/src/extract.rs @@ -383,10 +383,17 @@ fn space(doc: &Document, cs: Option<&Object>) -> Result { Some(b"CalGray") => Ok(Space::Gray), Some(b"CalRGB") => Ok(Space::Rgb), Some(b"Indexed" | b"I") => { - let base = space(doc, a.get(1))?; - if matches!(base, Space::Indexed(..)) { + // Checked before recursing: Indexed is the only space that recurses, so refusing + // an Indexed base here also ends a cyclic base reference. + let nested = match a.get(1).map(|b| doc.resolve(b)).as_deref() { + Some(Object::Array(b)) => matches!(b.first().and_then(Object::as_name), Some(b"Indexed" | b"I")), + Some(Object::Name(n)) => matches!(n.as_slice(), b"Indexed" | b"I"), + _ => false, + }; + if nested { return Err("nested indexed colour spaces aren't valid".into()); } + let base = space(doc, a.get(1))?; let hival = match a.get(2).map(|value| doc.resolve(value)).as_deref() { Some(Object::Int(value)) if (0..=255).contains(value) => *value as u8, _ => return Err("an indexed colour space with an invalid hival".into()), diff --git a/crates/create/src/tests.rs b/crates/create/src/tests.rs index 6356105699..a214977675 100644 --- a/crates/create/src/tests.rs +++ b/crates/create/src/tests.rs @@ -1181,3 +1181,19 @@ fn a_jpeg_with_an_exif_orientation_becomes_an_upright_page() { let img = image_of(&doc, 1); assert_eq!((img.int(b"Width"), img.int(b"Height")), (Some(60), Some(20))); } + +#[test] +fn a_cyclic_indexed_colour_space_is_refused() { + let mut doc = from_images(&[("flat.png".into(), png_bytes(false))]).unwrap(); + // 5 0 obj [/Indexed 5 0 R 255 <00>]: the base is the indexed space itself. + let cs = doc.add(Object::Null); + doc.set(cs, Object::Array(vec![Object::name("Indexed"), Object::Ref(cs), Object::Int(255), Object::String(PdfString::literal(vec![0]))])); + with_image(&mut doc, |_| { + let mut dict = image_dict(1, 1, "DeviceGray", 8); + dict.set(b"ColorSpace".to_vec(), Object::Ref(cs)); + Stream::from_raw(dict, vec![0]) + }); + let out = extract_images(&doc, &[0], 0); + assert!(out.images.is_empty()); + assert_eq!(out.skipped.len(), 1, "{:?}", out.skipped); +} From 78a562bc081b8b2afad687e2399071c003ae9a19 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 14:08:32 +0000 Subject: [PATCH 09/22] fix(optimize): bound a merged fragment image's height before sizing its buffer The fragment merge multiplied the merged width by a height cast from a float without checking either fit. A tiny fragment rectangle gives a huge height: the multiplication overflowed (a panic in debug builds, a wrap past the pixel limit in release builds) and the u32 cast saturated, sizing an enormous allocation. The height must now fit u32 before the pixel limit is checked, and the buffer size uses checked multiplication. --- crates/optimize/src/fragments.rs | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/crates/optimize/src/fragments.rs b/crates/optimize/src/fragments.rs index b5967eee09..caf617f717 100644 --- a/crates/optimize/src/fragments.rs +++ b/crates/optimize/src/fragments.rs @@ -143,12 +143,16 @@ fn merge(doc: &Document, run: &[Fragment], n: usize) -> Option<(Vec, u32, u3 // Pixel rows per unit of height: the finest fragment's. let density = run.iter().map(|f| f.height as f64 / (f.rect[3] - f.rect[1])).fold(0.0, f64::max); let height = ((top - bottom) * density).round(); - if !(height.is_finite() && height >= 1.0) || (width as u64) * (height as u64) > MAX_PIXELS { + if !(height.is_finite() && height >= 1.0 && height <= f64::from(u32::MAX)) { return None; } + // In range and whole, so the cast is exact. let height = height as u32; - let row = width as usize * n; - let mut out = vec![255u8; row * height as usize]; + if u64::from(width) * u64::from(height) > MAX_PIXELS { + return None; + } + let row = (width as usize).checked_mul(n)?; + let mut out = vec![255u8; row.checked_mul(height as usize)?]; for f in run { let r0 = (((top - f.rect[3]) * density).round().max(0.0) as u32).min(height); let r1 = (((top - f.rect[1]) * density).round().max(0.0) as u32).min(height); @@ -297,3 +301,21 @@ pub(crate) fn run(doc: &mut Document, pages: &[ObjRef], report: &mut Report, pro } Ok(()) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn a_merged_height_beyond_u32_is_refused_before_allocating() { + // Two fragments, one pixel row each, the top one 2^-48 high: the merged image would be + // 2^16 × 2^48 pixels, whose product wraps to 0 in u64. + let image = ObjRef { num: 1, generation: 0 }; + let edge = 1.0 - (-48f64).exp2(); + let run = [ + Fragment { image, rect: [0.0, edge, 1.0, 1.0], width: 1 << 16, height: 1 }, + Fragment { image, rect: [0.0, 0.0, 1.0, edge], width: 1 << 16, height: 1 }, + ]; + assert!(merge(&Document::new_empty(), &run, 3).is_none()); + } +} From f0e3880c35327859f82228caaa8219d86b8955c0 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 14:12:52 +0000 Subject: [PATCH 10/22] fix(xfdf): never let a same-named imported comment replace a form widget Comment import replaces an existing annotation with the same /NM in place. The check for annotations it must keep covered locked comments and redaction marks but not widgets or field dictionaries, so an imported comment could overwrite a field's widget object, setting its value and validation entries without the form value setter while the AcroForm field list still pointed at it. Widgets and field dictionaries are now kept, and the imported comment is added beside them under no name. --- crates/xfdf/src/lib.rs | 11 ++++++----- crates/xfdf/src/tests.rs | 19 +++++++++++++++++++ 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/crates/xfdf/src/lib.rs b/crates/xfdf/src/lib.rs index ab258990e2..1e037b6795 100644 --- a/crates/xfdf/src/lib.rs +++ b/crates/xfdf/src/lib.rs @@ -1017,10 +1017,11 @@ const REDACT_UNSEEN: i64 = 1 | 2 | 32; /// `/F` Locked (ISO 32000-2 Table 167): the user protected this annotation from change. const LOCKED: i64 = 128; -/// An existing annotation an import must never replace, move or unlock: one the user locked, or a -/// redaction mark (replacing it would drop the region the user marked to remove). +/// An existing annotation an import must never replace, move or unlock: one the user locked, a +/// redaction mark (replacing it would drop the region the user marked to remove), or a form +/// widget or field (replacing it would set the field's value and policy around the form setter). fn import_protected(x: &Dict) -> bool { - x.int(b"F").unwrap_or(0) & LOCKED != 0 || x.name(b"Subtype") == Some(b"Redact".as_slice()) + x.int(b"F").unwrap_or(0) & LOCKED != 0 || matches!(x.name(b"Subtype"), Some(b"Redact" | b"Widget")) || x.contains(b"FT") } fn place(doc: &mut Document, page_ref: ObjRef, mut d: Dict) -> Result { @@ -1042,8 +1043,8 @@ fn place(doc: &mut Document, page_ref: ObjRef, mut d: Dict) -> Result = Vec::new(); - // Never let a same-name import replace a locked comment or a redaction mark: keep it, and add - // the imported dict beside it under no name of its own. + // Never let a same-name import replace a protected annotation (`import_protected`): keep it, + // and add the imported dict beside it under no name of its own. let mut clash = false; for a in annots { let existing = a.as_ref().and_then(|r| doc.get(r).as_dict().cloned()); diff --git a/crates/xfdf/src/tests.rs b/crates/xfdf/src/tests.rs index 6c9824a245..57155e4b7a 100644 --- a/crates/xfdf/src/tests.rs +++ b/crates/xfdf/src/tests.rs @@ -1660,3 +1660,22 @@ fn tabular_import_keeps_tsv_quotes_literal_and_csv_value_tabs() { import(&mut doc, b"Name,Other\r\n\"Ada\tLovelace\",other field\r\n").unwrap(); assert_eq!(fields(&doc).into_iter().find(|f| f.name == "Name").unwrap().value, vec!["Ada\tLovelace"]); } + +#[test] +fn a_same_named_comment_import_never_replaces_a_form_widget() { + let mut doc = blank(); + let page = pdfcraft_model::pages(&doc)[0].clone(); + let widget = doc + .resolve(page.dict.get(b"Annots").unwrap()) + .as_array() + .unwrap() + .iter() + .filter_map(Object::as_ref) + .find(|r| doc.get(*r).as_dict().is_some_and(|d| d.name(b"Subtype") == Some(b"Widget".as_slice()))) + .unwrap(); + doc.update_dict(widget, |d| d.set(b"NM".to_vec(), PdfString::text("w1"))).unwrap(); + let before = doc.get(widget); + let fdf = b"%FDF-1.2\n1 0 obj\n<< /FDF << /Annots [<< /Subtype /Text /NM (w1) /Page 0 /Rect [0 0 1 1] /FT /Tx /T (Name) /V (forged) >>] >> >>\nendobj\ntrailer\n<< /Root 1 0 R >>\n%%EOF\n"; + assert_eq!(import(&mut doc, fdf).unwrap().comments, 1); + assert_eq!(doc.get(widget), before, "the widget was overwritten by an imported comment"); +} From d266222339adf378f785c98fdd3ae65eb8831867 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 14:22:02 +0000 Subject: [PATCH 11/22] fix(xfdf): draw every imported redaction mark's review outline, opaque A redaction mark's outline is drawn from its /QuadPoints, but applying the mark falls back to its /Rect when it has none. An imported mark with only a /Rect, or with quads the outline can't draw, therefore had no visible outline while still removing content when applied, and an imported opacity of 0 hid the outline too. Imported marks now get quads from their /Rect when their own are unusable (or none, so they remove nothing), and their /CA and /ca are dropped. --- crates/xfdf/src/lib.rs | 21 +++++++++++++++++++++ crates/xfdf/src/tests.rs | 23 +++++++++++++++++++++++ 2 files changed, 44 insertions(+) diff --git a/crates/xfdf/src/lib.rs b/crates/xfdf/src/lib.rs index 1e037b6795..80c37b5e86 100644 --- a/crates/xfdf/src/lib.rs +++ b/crates/xfdf/src/lib.rs @@ -1032,6 +1032,27 @@ fn place(doc: &mut Document, page_ref: ObjRef, mut d: Dict) -> Result { + let (l, r, b, t) = (x0.min(x1), x0.max(x1), y0.min(y1), y0.max(y1)); + d.set(b"QuadPoints".to_vec(), arr(&[l, t, r, t, l, b, r, b])); + } + _ => { + d.remove(b"QuadPoints"); + } + } + } } let nm = d.get(b"NM").and_then(|n| n.as_string().map(|s| s.to_text())); let annots: Vec = doc diff --git a/crates/xfdf/src/tests.rs b/crates/xfdf/src/tests.rs index 57155e4b7a..7951bef4dd 100644 --- a/crates/xfdf/src/tests.rs +++ b/crates/xfdf/src/tests.rs @@ -1679,3 +1679,26 @@ fn a_same_named_comment_import_never_replaces_a_form_widget() { assert_eq!(import(&mut doc, fdf).unwrap().comments, 1); assert_eq!(doc.get(widget), before, "the widget was overwritten by an imported comment"); } + +#[test] +fn an_imported_rect_only_or_transparent_redaction_mark_is_drawn_for_review() { + let xfdf = br#""#; + let fdf = b"%FDF-1.2\n1 0 obj\n<< /FDF << /Annots [<< /Subtype /Redact /NM (r1) /Page 0 /Rect [60 600 560 620] /CA 0 /ca 0 >>] >> >>\nendobj\ntrailer\n<< /Root 1 0 R >>\n%%EOF\n"; + for data in [xfdf.as_slice(), fdf.as_slice()] { + let mut doc = blank(); + assert_eq!(import(&mut doc, data).unwrap().comments, 1); + let page = pdfcraft_model::pages(&doc)[0].clone(); + let mark = doc + .resolve(page.dict.get(b"Annots").unwrap()) + .as_array() + .unwrap() + .iter() + .filter_map(|a| a.as_ref().and_then(|r| doc.get(r).as_dict().cloned())) + .find(|d| d.name(b"Subtype") == Some(b"Redact".as_slice())) + .unwrap(); + assert_eq!(nums_of(&doc, mark.get(b"QuadPoints")), vec![60.0, 620.0, 560.0, 620.0, 60.0, 600.0, 560.0, 600.0]); + let ap = mark.get(b"AP").map(|a| doc.resolve(a)).and_then(|a| a.as_dict().cloned()); + assert!(ap.is_some_and(|ap| ap.get(b"N").is_some()), "the mark has no review appearance: {mark:?}"); + assert!(mark.get(b"CA").is_none() && mark.get(b"ca").is_none(), "the mark can be drawn transparent: {mark:?}"); + } +} From 8a8dab6c00ad27e9d3e1b66e42442c1fbd930ba7 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 14:37:40 +0000 Subject: [PATCH 12/22] fix(xfdf): keep merged spreadsheet cells from starting a formula Merge Data Files into Spreadsheet writes field names and values from other people's files as CSV cells, escaping only the delimiters. A cell starting with =, +, - or @ is evaluated as a formula when the CSV is opened in a spreadsheet program. Such cells, after any leading whitespace, are now written with a leading apostrophe; finite numbers such as -12.5 are left as they are. --- crates/xfdf/src/lib.rs | 18 +++++++++++++++++- crates/xfdf/src/tests.rs | 15 +++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/crates/xfdf/src/lib.rs b/crates/xfdf/src/lib.rs index 80c37b5e86..1eac1f96c5 100644 --- a/crates/xfdf/src/lib.rs +++ b/crates/xfdf/src/lib.rs @@ -1287,6 +1287,10 @@ pub fn data_values(bytes: &[u8]) -> Result)>, DataError /// Merge Data Files into Spreadsheet: one CSV row per file, one column per field name (in the /// order they first appear). +/// +/// The names and values come from other people's files, and spreadsheet programs evaluate a cell +/// that starts with `=`, `+`, `-` or `@` (after any leading whitespace) as a formula. Such a cell +/// is written with a leading `'`, which keeps it text; a plain number such as `-12.5` is not. pub fn merge_csv(files: &[Vec<(String, Vec)>]) -> String { let mut columns: Vec<&str> = Vec::new(); for f in files { @@ -1296,7 +1300,19 @@ pub fn merge_csv(files: &[Vec<(String, Vec)>]) -> String { } } } - let q = |s: &str| if s.contains([',', '"', '\n', '\r', '\t']) { format!("\"{}\"", s.replace('"', "\"\"")) } else { s.to_string() }; + let literal = |s: &str| { + let t = s.trim_start(); + let formula = match t.chars().next() { + Some('=' | '@') => true, + Some('+' | '-') => !t.parse::().is_ok_and(f64::is_finite), + _ => false, + }; + if formula { format!("'{s}") } else { s.to_string() } + }; + let q = |s: &str| { + let s = literal(s); + if s.contains([',', '"', '\n', '\r', '\t']) { format!("\"{}\"", s.replace('"', "\"\"")) } else { s } + }; let mut out = columns.iter().map(|c| q(c)).collect::>().join(","); out.push('\n'); for f in files { diff --git a/crates/xfdf/src/tests.rs b/crates/xfdf/src/tests.rs index 7951bef4dd..3e6dc58a5d 100644 --- a/crates/xfdf/src/tests.rs +++ b/crates/xfdf/src/tests.rs @@ -1702,3 +1702,18 @@ fn an_imported_rect_only_or_transparent_redaction_mark_is_drawn_for_review() { assert!(mark.get(b"CA").is_none() && mark.get(b"ca").is_none(), "the mark can be drawn transparent: {mark:?}"); } } + +#[test] +fn merged_spreadsheet_cells_never_start_a_formula() { + let row = vec![ + ("=HYPERLINK(\"http://x\")".to_string(), vec!["@SUM(A1)".to_string()]), + ("Total".to_string(), vec!["-2+3".to_string()]), + ("Plus".to_string(), vec!["\t+1+cmd".to_string()]), + ("Balance".to_string(), vec!["-12.5".to_string()]), + ("\tName".to_string(), vec!["Ada".to_string()]), + ]; + let csv = merge_csv(&[row]); + let lines: Vec<&str> = csv.lines().collect(); + assert_eq!(lines[0], "\"'=HYPERLINK(\"\"http://x\"\")\",Total,Plus,Balance,\"\tName\""); + assert_eq!(lines[1], "'@SUM(A1),'-2+3,\"'\t+1+cmd\",-12.5,Ada"); +} From 63f7932d06e19b1b9f9a1cb4baaab3550578bb4a Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 17:00:43 +0000 Subject: [PATCH 13/22] fix(xfdf): fall back to the /Rect when an imported redaction mark has a zero-area quad A quad whose corners all lie on one line (or on one point) encloses nothing, yet such /QuadPoints counted as drawable, replaced the mark's /Rect and left an imported redaction that covers nothing. Any quad without area now makes the mark's quads unusable, so its /Rect is used instead. --- crates/xfdf/src/lib.rs | 22 ++++++++++++++++------ crates/xfdf/src/tests.rs | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 6 deletions(-) diff --git a/crates/xfdf/src/lib.rs b/crates/xfdf/src/lib.rs index 1eac1f96c5..9582298b34 100644 --- a/crates/xfdf/src/lib.rs +++ b/crates/xfdf/src/lib.rs @@ -246,6 +246,14 @@ fn nums_of(doc: &Document, o: Option<&Object>) -> Vec { o.map(|o| doc.resolve(o)).and_then(|a| a.as_array().map(|a| a.iter().filter_map(|x| doc.resolve(x).as_f64()).collect())).unwrap_or_default() } +/// Whether a quad's four corners enclose any area: some three of them are not collinear. +fn quad_has_area(q: &[f64; 8]) -> bool { + let [ax, ay, bx, by, cx, cy, dx, dy] = *q; + let cross = |(px, py): (f64, f64), (qx, qy): (f64, f64), (rx, ry): (f64, f64)| (qx - px) * (ry - py) - (qy - py) * (rx - px); + let (a, b, c, d) = ((ax, ay), (bx, by), (cx, cy), (dx, dy)); + [cross(a, b, c), cross(a, b, d), cross(a, c, d), cross(b, c, d)].iter().any(|v| v.abs() > 0.0) +} + fn text_of(doc: &Document, d: &Dict, k: &[u8]) -> Option { d.get(k).and_then(|o| match &*doc.resolve(o) { Object::String(s) => Some(s.to_text()), @@ -1036,13 +1044,15 @@ fn place(doc: &mut Document, page_ref: ObjRef, mut d: Dict) -> Result = q.iter().filter_map(Object::as_f64).filter(|v| v.is_finite()).collect(); + let (quads, rest) = v.as_chunks::<8>(); + v.len() == q.len() && rest.is_empty() && !quads.is_empty() && quads.iter().all(quad_has_area) + }); if !quads_drawable { - // Quads the outline can't draw are replaced by the /Rect's, or dropped when there is - // no usable /Rect either; such a mark then removes nothing. + // Quads the outline can't draw, or any quad that encloses no area, are replaced by the + // /Rect's, or dropped when there is no usable /Rect either; such a mark then removes + // nothing. match nums_of(doc, d.get(b"Rect"))[..] { [x0, y0, x1, y1] if [x0, y0, x1, y1].iter().all(|v| v.is_finite()) => { let (l, r, b, t) = (x0.min(x1), x0.max(x1), y0.min(y1), y0.max(y1)); diff --git a/crates/xfdf/src/tests.rs b/crates/xfdf/src/tests.rs index 3e6dc58a5d..de880fe664 100644 --- a/crates/xfdf/src/tests.rs +++ b/crates/xfdf/src/tests.rs @@ -1717,3 +1717,37 @@ fn merged_spreadsheet_cells_never_start_a_formula() { assert_eq!(lines[0], "\"'=HYPERLINK(\"\"http://x\"\")\",Total,Plus,Balance,\"\tName\""); assert_eq!(lines[1], "'@SUM(A1),'-2+3,\"'\t+1+cmd\",-12.5,Ada"); } + +fn imported_redact_mark(data: &[u8]) -> Dict { + let mut doc = blank(); + assert_eq!(import(&mut doc, data).unwrap().comments, 1); + let page = pdfcraft_model::pages(&doc)[0].clone(); + doc.resolve(page.dict.get(b"Annots").unwrap()) + .as_array() + .unwrap() + .iter() + .filter_map(|a| a.as_ref().and_then(|r| doc.get(r).as_dict().cloned())) + .find(|d| d.name(b"Subtype") == Some(b"Redact".as_slice())) + .unwrap() +} + +#[test] +fn an_imported_redaction_mark_with_zero_area_quads_falls_back_to_its_rect() { + let rect_quads = vec![60.0, 620.0, 560.0, 620.0, 60.0, 600.0, 560.0, 600.0]; + let fdf = |quads: &str| { + format!( + "%FDF-1.2\n1 0 obj\n<< /FDF << /Annots [<< /Subtype /Redact /NM (r1) /Page 0 /Rect [60 600 560 620] /QuadPoints [{quads}] >>] >> >>\nendobj\ntrailer\n<< /Root 1 0 R >>\n%%EOF\n" + ) + }; + let cases = [ + fdf("100 100 100 100 100 100 100 100").into_bytes(), + // A good quad beside one that is a line (all four corners collinear). + fdf("60 620 560 620 60 600 560 600 10 10 20 20 30 30 40 40").into_bytes(), + br#""#.to_vec(), + ]; + for data in cases { + let mark = imported_redact_mark(&data); + let quads: Vec = mark.get(b"QuadPoints").and_then(Object::as_array).unwrap().iter().filter_map(Object::as_f64).collect(); + assert_eq!(quads, rect_quads, "zero-area quads were kept: {mark:?}"); + } +} From 256097023c57cc0574287d338051c88676b72096 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 17:01:55 +0000 Subject: [PATCH 14/22] fix(xfdf): use an imported redaction mark's /Rect only when it is four finite numbers The /Rect fallback dropped non-numbers before checking the length, so a malformed /Rect such as [0 0 /Bad 100 100] read as a valid rectangle and gave the mark quads. The array itself must now hold exactly four finite numbers. --- crates/xfdf/src/lib.rs | 13 +++++++++++-- crates/xfdf/src/tests.rs | 7 +++++++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/crates/xfdf/src/lib.rs b/crates/xfdf/src/lib.rs index 9582298b34..219b0de81b 100644 --- a/crates/xfdf/src/lib.rs +++ b/crates/xfdf/src/lib.rs @@ -246,6 +246,15 @@ fn nums_of(doc: &Document, o: Option<&Object>) -> Vec { o.map(|o| doc.resolve(o)).and_then(|a| a.as_array().map(|a| a.iter().filter_map(|x| doc.resolve(x).as_f64()).collect())).unwrap_or_default() } +/// A `/Rect` of exactly four finite numbers; any other entry makes the whole rectangle unusable. +fn rect_strict(doc: &Document, o: Option<&Object>) -> Option<[f64; 4]> { + let num = |x: &Object| doc.resolve(x).as_f64().filter(|v| v.is_finite()); + match doc.resolve(o?).as_array()?.as_slice() { + [x0, y0, x1, y1] => Some([num(x0)?, num(y0)?, num(x1)?, num(y1)?]), + _ => None, + } +} + /// Whether a quad's four corners enclose any area: some three of them are not collinear. fn quad_has_area(q: &[f64; 8]) -> bool { let [ax, ay, bx, by, cx, cy, dx, dy] = *q; @@ -1053,8 +1062,8 @@ fn place(doc: &mut Document, page_ref: ObjRef, mut d: Dict) -> Result { + match rect_strict(doc, d.get(b"Rect")) { + Some([x0, y0, x1, y1]) => { let (l, r, b, t) = (x0.min(x1), x0.max(x1), y0.min(y1), y0.max(y1)); d.set(b"QuadPoints".to_vec(), arr(&[l, t, r, t, l, b, r, b])); } diff --git a/crates/xfdf/src/tests.rs b/crates/xfdf/src/tests.rs index de880fe664..c98fab06ac 100644 --- a/crates/xfdf/src/tests.rs +++ b/crates/xfdf/src/tests.rs @@ -1751,3 +1751,10 @@ fn an_imported_redaction_mark_with_zero_area_quads_falls_back_to_its_rect() { assert_eq!(quads, rect_quads, "zero-area quads were kept: {mark:?}"); } } + +#[test] +fn an_imported_redaction_rect_with_a_non_number_is_not_used() { + let fdf = b"%FDF-1.2\n1 0 obj\n<< /FDF << /Annots [<< /Subtype /Redact /NM (r1) /Page 0 /Rect [0 0 /Bad 100 100] >>] >> >>\nendobj\ntrailer\n<< /Root 1 0 R >>\n%%EOF\n"; + let mark = imported_redact_mark(fdf); + assert!(mark.get(b"QuadPoints").is_none(), "quads were drawn from a malformed /Rect: {mark:?}"); +} From f1c56ef86534ec182cde58cc827a700e0a693f51 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 14:45:44 +0000 Subject: [PATCH 15/22] fix(redact): count a name tree's shared nodes once The hidden-information count followed every /Kids entry of the attachment and document-script name trees with only a depth limit, so a node listed twice by each level was read once per path: a few hundred bytes made the scan take 2^32 steps. Indirect nodes are now read once each, under a total node budget. --- crates/redact/src/sanitize.rs | 34 ++++++++++++++++++++++++++++------ crates/redact/src/tests.rs | 22 ++++++++++++++++++++++ 2 files changed, 50 insertions(+), 6 deletions(-) diff --git a/crates/redact/src/sanitize.rs b/crates/redact/src/sanitize.rs index 83db13d728..c9c2b182aa 100644 --- a/crates/redact/src/sanitize.rs +++ b/crates/redact/src/sanitize.rs @@ -87,14 +87,36 @@ fn sub(doc: &Document, d: &Dict, key: &[u8]) -> Option { d.get(key).and_then(|o| doc.resolve(o).as_dict().cloned()) } -/// Entries of a name tree (`/Names` leaves, `/Kids` recursion), bounded against cycles. -fn name_tree_len(doc: &Document, node: &Dict, depth: usize) -> usize { - if depth > 32 { +/// The most name-tree nodes one count reads. +const NAME_TREE_NODE_BUDGET: usize = 100_000; + +/// Entries of a name tree (`/Names` leaves, `/Kids` recursion). Each indirect node is read once, +/// however many `/Kids` arrays list it, so shared or cyclic kids cost one step per node rather +/// than one per path; depth and a total node budget bound the rest. +fn name_tree_len(doc: &Document, node: &Dict) -> usize { + let (mut seen, mut visited) = (std::collections::HashSet::new(), 0); + name_tree_len_seen(doc, node, 0, &mut seen, &mut visited) +} + +fn name_tree_len_seen(doc: &Document, node: &Dict, depth: usize, seen: &mut std::collections::HashSet, visited: &mut usize) -> usize { + *visited += 1; + if depth > 32 || *visited > NAME_TREE_NODE_BUDGET { return 0; } let leaves = node.get(b"Names").and_then(|n| doc.resolve(n).as_array().map(|a| a.len() / 2)).unwrap_or(0); let kids = node.get(b"Kids").and_then(|k| doc.resolve(k).as_array().cloned()).unwrap_or_default(); - leaves + kids.iter().filter_map(|k| doc.resolve(k).as_dict().cloned()).map(|k| name_tree_len(doc, &k, depth + 1)).sum::() + let mut n = leaves; + for k in &kids { + if let Object::Ref(r) = k + && !seen.insert(*r) + { + continue; + } + if let Some(kid) = doc.resolve(k).as_dict() { + n = n.saturating_add(name_tree_len_seen(doc, kid, depth + 1, seen, visited)); + } + } + n } /// The most distinct outline items one count reads, over all levels together. @@ -270,7 +292,7 @@ pub fn scan(doc: &Document) -> Vec<(Hidden, usize)> { let xmp = usize::from(cat.contains(b"Metadata")) + pages.iter().filter(|p| p.dict.contains(b"Metadata")).count(); info + xmp } - Hidden::Attachments => sub(doc, &names, b"EmbeddedFiles").map_or(0, |t| name_tree_len(doc, &t, 0)) + count_sub(b"FileAttachment"), + Hidden::Attachments => sub(doc, &names, b"EmbeddedFiles").map_or(0, |t| name_tree_len(doc, &t)) + count_sub(b"FileAttachment"), Hidden::Comments => annots.iter().filter(|a| is_comment(a.name(b"Subtype").unwrap_or(b""))).count(), Hidden::FormFields => pdfcraft_forms::fields(doc).len(), Hidden::HiddenText => content_pass(&mut doc2, true, false, false).map(|(n, _)| n).unwrap_or(0), @@ -285,7 +307,7 @@ pub fn scan(doc: &Document) -> Vec<(Hidden, usize)> { + usize::from(cat.contains(b"AA")) + pages.iter().filter(|p| p.dict.contains(b"AA")).count() + annots.iter().filter(|a| a.contains(b"AA") || (a.name(b"Subtype") == Some(b"Widget") && a.contains(b"A"))).count() - + sub(doc, &names, b"JavaScript").map_or(0, |t| name_tree_len(doc, &t, 0)) + + sub(doc, &names, b"JavaScript").map_or(0, |t| name_tree_len(doc, &t)) } Hidden::PrivateData => usize::from(cat.contains(b"PieceInfo")) + pages.iter().filter(|p| p.dict.contains(b"PieceInfo")).count(), }; diff --git a/crates/redact/src/tests.rs b/crates/redact/src/tests.rs index 286667937d..2a19014a9c 100644 --- a/crates/redact/src/tests.rs +++ b/crates/redact/src/tests.rs @@ -2472,3 +2472,25 @@ fn a_tj_surplus_arrays_undrawn_glyphs_are_not_seen_under_a_mark() { assert_eq!(under(&mut doc, 0, &[[45.0, 95.0, 50.0, 110.0]]), 0, "nothing is drawn past LASTARR's 35pt width"); assert_eq!(under(&mut doc, 0, &[[10.0, 95.0, 45.0, 110.0]]), 7, "LASTARR's 7 glyphs, the one the page draws"); } + +/// A name tree whose levels each list the next level twice: a walk that follows every path +/// visits 2^levels nodes. +#[test] +fn a_name_tree_whose_nodes_share_kids_counts_each_entry_once() { + use crate::sanitize::{Hidden, scan}; + let levels = 12; + let mut objs: Vec> = vec![ + b"<< /Type /Catalog /Pages 2 0 R /Names << /EmbeddedFiles 5 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 >>".to_vec(), + stream("", b"BT ET"), + ]; + for k in 0..levels { + let next = 6 + k; + objs.push(format!("<< /Kids [{next} 0 R {next} 0 R] >>").into_bytes()); + } + objs.push(b"<< /Names [(a.txt) << /Type /Filespec /F (a.txt) >>] >>".to_vec()); + let doc = pdf(objs); + let counts: std::collections::HashMap = scan(&doc).into_iter().collect(); + assert_eq!(counts[&Hidden::Attachments], 1, "one entry, reached through 2^{levels} paths"); +} From ee010fbbe5af7d7efcc8859b976a34121f3225d0 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 14:46:18 +0000 Subject: [PATCH 16/22] fix(redact): read content strictly before sanitizing or redacting it Page and Form content was decoded tolerantly, keeping what came before a corrupt spot and dropping the rest without saying so. A page whose content broke at its first bytes therefore read as empty: the hidden-text scan counted nothing, the removal was skipped and the original stream, which other viewers still draw, stayed in the file. Content is now decoded strictly; an unreadable page counts as something to remove, so the removal runs and refuses the page, and a Form read only in part is treated like an undecodable one. --- crates/redact/src/interp.rs | 3 ++- crates/redact/src/lib.rs | 4 +++- crates/redact/src/sanitize.rs | 3 +++ crates/redact/src/tests.rs | 20 ++++++++++++++++++++ 4 files changed, 28 insertions(+), 2 deletions(-) diff --git a/crates/redact/src/interp.rs b/crates/redact/src/interp.rs index c9a404b935..a608568f38 100644 --- a/crates/redact/src/interp.rs +++ b/crates/redact/src/interp.rs @@ -1201,7 +1201,8 @@ fn xobject( out.doomed.push(r); return Some(None); } - let data = match s.decoded() { + // Strict, like page content: a Form read only in part counts as undecodable. + let data = match s.decoded_strict() { Ok(d) => d, Err(_) if scope.mode == Mode::Apply => { scope.report.forms_removed += 1; diff --git a/crates/redact/src/lib.rs b/crates/redact/src/lib.rs index 6df5102afa..364f7cc193 100644 --- a/crates/redact/src/lib.rs +++ b/crates/redact/src/lib.rs @@ -231,7 +231,9 @@ pub(crate) fn page_streams(doc: &Document, page: &Dict, index: usize) -> Result< let mut data = Vec::new(); for o in &list { match &*doc.resolve(o) { - Object::Stream(s) => data.push(s.decoded().map_err(|_| RedactError::Unreadable(index + 1))?), + // Strict: a tolerant decode keeps what precedes corruption and drops the rest, which + // still draws in viewers that read further; anything not read can't be cleaned. + Object::Stream(s) => data.push(s.decoded_strict().map_err(|_| RedactError::Unreadable(index + 1))?), Object::Null => data.push(Vec::new()), _ => return Err(RedactError::Unreadable(index + 1)), } diff --git a/crates/redact/src/sanitize.rs b/crates/redact/src/sanitize.rs index c9c2b182aa..f4c880e30b 100644 --- a/crates/redact/src/sanitize.rs +++ b/crates/redact/src/sanitize.rs @@ -205,6 +205,9 @@ fn content_pass(doc: &mut Document, text: bool, layers: bool, write: bool) -> Re if write { return Err(RedactError::Unreadable(pi + 1)); } + // Content that can't be read may hide anything: count it so the removal runs and + // refuses the page, instead of a zero count skipping it. + total += 1; continue; }; let resources = page.dict.get(b"Resources").and_then(|r| doc.resolve(r).as_dict().cloned()).unwrap_or_default(); diff --git a/crates/redact/src/tests.rs b/crates/redact/src/tests.rs index 2a19014a9c..a0d68af2d7 100644 --- a/crates/redact/src/tests.rs +++ b/crates/redact/src/tests.rs @@ -2494,3 +2494,23 @@ fn a_name_tree_whose_nodes_share_kids_counts_each_entry_once() { let counts: std::collections::HashMap = scan(&doc).into_iter().collect(); assert_eq!(counts[&Hidden::Attachments], 1, "one entry, reached through 2^{levels} paths"); } + +/// Page content that stops decoding at its start (`z` inside an ASCII85 group) while the rest of +/// the stream draws hidden text. A tolerant decode reads nothing, counts nothing and keeps the +/// stream; sanitizing must refuse instead. +#[test] +fn hidden_text_in_content_that_stops_decoding_is_refused() { + use crate::sanitize::{Hidden, remove_hidden, sanitize}; + let encoded: &[u8] = br"!!z6<#'\7PQ#?0Ha>,+>Y->EZd(j+>PW)+B2ko-rWQ%;FOP6+B3(u78s~>"; + let fresh = || { + let mut doc = one_page(b"", "", vec![stream("/Filter /ASCII85Decode", encoded)]); + doc.update_dict(ObjRef::new(3, 0), |d| d.set(b"Contents".to_vec(), Object::Ref(ObjRef::new(7, 0)))).unwrap(); + doc + }; + let mut doc = fresh(); + let out = remove_hidden(&mut doc, &[Hidden::HiddenText]); + assert!(matches!(out, Err(RedactError::Unreadable(1))), "{out:?}"); + let mut doc = fresh(); + let out = sanitize(&mut doc); + assert!(matches!(out, Err(RedactError::Unreadable(1))), "{out:?}"); +} From 3fe9fe218ecbc4fb6ab2f9eba346115001bf0368 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 14:46:38 +0000 Subject: [PATCH 17/22] fix(redact): remove the widgets flattening keeps when removing form fields Removing form fields flattens them and then drops the interactive form, but flattening keeps hidden and no-view widgets (they draw nothing) and widgets with no appearance. The pages still listed them, so their values and their parent fields' values survived the full rewrite. Every widget left after flattening now leaves the pages and the structure tree, inline annotations included. --- crates/redact/src/sanitize.rs | 11 ++++++++--- crates/redact/src/tests.rs | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 3 deletions(-) diff --git a/crates/redact/src/sanitize.rs b/crates/redact/src/sanitize.rs index f4c880e30b..41ef96cccf 100644 --- a/crates/redact/src/sanitize.rs +++ b/crates/redact/src/sanitize.rs @@ -341,7 +341,8 @@ pub fn remove_hidden(doc: &mut Document, which: &[Hidden]) -> Result 0 { + let fields = which.contains(&Hidden::FormFields) && count(Hidden::FormFields) > 0; + if fields { // Fields without appearances get one first, so their values stay visible. for f in pdfcraft_forms::fields(doc) { if f.widgets.iter().any(|w| !doc.get(w.obj).as_dict().is_some_and(|d| d.contains(b"AP"))) { @@ -354,10 +355,12 @@ pub fn remove_hidden(doc: &mut Document, which: &[Hidden]) -> Result bool { let s = a.name(b"Subtype").unwrap_or(b""); (which.contains(&Hidden::Comments) && is_comment(s)) + || (fields && s == b"Widget") || (which.contains(&Hidden::Attachments) && s == b"FileAttachment") || (which.contains(&Hidden::LinksActionsScripts) && s == b"Link") }; @@ -374,7 +377,9 @@ pub fn remove_hidden(doc: &mut Document, which: &[Hidden]) -> Result Object { + Object::Array(nums.iter().map(|&n| Object::Ref(ObjRef::new(n, 0))).collect()) +} + +/// An interactive form dictionary listing fields `nums`. +fn fields_dict(nums: &[u32]) -> Object { + let mut d = Dict::new(); + d.set(b"Fields".to_vec(), ref_array(nums)); + Object::Dict(d) +} + +/// Flattening keeps hidden widgets (they draw nothing); removing form fields must still take them +/// and their values, and their parent field's value, out of the file. +#[test] +fn removing_form_fields_takes_hidden_widgets_and_their_values_out_of_the_file() { + use crate::sanitize::{Hidden, remove_hidden}; + let mut doc = one_page( + b"BT /F1 10 Tf 10 100 Td (Visible) Tj ET", + "", + vec![ + // 7 a hidden terminal field merged with its widget, 8 a parent field, 9 its hidden widget + b"<< /Type /Annot /Subtype /Widget /FT /Tx /T (own) /V (LEAK_WIDGET_VALUE) /F 2 /Rect [10 200 150 220] /AP << /N 10 0 R >> >>".to_vec(), + b"<< /FT /Tx /T (parent) /V (LEAK_PARENT_VALUE) /Kids [9 0 R] >>".to_vec(), + b"<< /Type /Annot /Subtype /Widget /Parent 8 0 R /F 32 /Rect [10 230 150 250] /AP << /N 10 0 R >> >>".to_vec(), + stream("/Type /XObject /Subtype /Form /BBox [0 0 140 20]", b"0 g"), + ], + ); + doc.update_dict(doc.root().unwrap(), |d| d.set(b"AcroForm".to_vec(), fields_dict(&[7, 8]))).unwrap(); + doc.update_dict(ObjRef::new(3, 0), |d| d.set(b"Annots".to_vec(), ref_array(&[7, 9]))).unwrap(); + remove_hidden(&mut doc, &[Hidden::FormFields]).unwrap(); + assert_no_secret(&doc, b"LEAK_"); + assert!(annots_of(&doc, &pdfcraft_model::pages(&doc).swap_remove(0).dict).is_empty()); +} From 750bbe27f1864bcb42b1cc1745256ae91d6db729 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 14:47:21 +0000 Subject: [PATCH 18/22] fix(redact): clean the structure tree after removing hidden content Removing hidden text or hidden layers rewrote page and Form content but left the structure tree alone, unlike applying redactions. An element's ActualText, Alt or expansion text still described the removed content, and a marked-content reference into a Form kept the original Form, with the removed text, in the full rewrite. The hidden-content pass now cleans the tags of what each page no longer shows and moves Form references to the rewritten copy, refusing when the tree can't be cleaned completely. --- crates/redact/src/sanitize.rs | 13 ++++++++++++- crates/redact/src/tests.rs | 28 ++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/crates/redact/src/sanitize.rs b/crates/redact/src/sanitize.rs index 41ef96cccf..fea0449512 100644 --- a/crates/redact/src/sanitize.rs +++ b/crates/redact/src/sanitize.rs @@ -254,7 +254,7 @@ fn content_pass(doc: &mut Document, text: bool, layers: bool, write: bool) -> Re let mut cleaned_page = page.dict.clone(); cleaned_page.set(b"Contents".to_vec(), Object::Array(new_list.clone())); let (_, page_content) = page_streams(doc, &cleaned_page, pi)?; - let mut res = resources; + let mut res = resources.clone(); if !out.xobjects.is_empty() || !out.replaced.is_empty() { let mut xo = res.get(b"XObject").and_then(|x| doc.resolve(x).as_dict().cloned()).unwrap_or_default(); // A full save keeps whatever is still referenced: unlink the originals this page no @@ -270,6 +270,17 @@ fn content_pass(doc: &mut Document, text: bool, layers: bool, write: bool) -> Re d.set(b"Contents".to_vec(), Object::Array(new_list)); d.set(b"Resources".to_vec(), Object::Dict(res)); })?; + // Tags must not describe or point at what the page no longer shows. + let (changed_ids, empty_ids) = crate::tags::touched( + doc, + &resources, + pdfcraft_content::Pieces::join(&data).data(), + pdfcraft_content::Pieces::join(&page_content).data(), + ); + crate::tags::clean(doc, page.obj, None, &changed_ids, &empty_ids)?; + } + for (old, new, changed, empty) in out.tag_changes { + crate::tags::clean(doc, page.obj, Some((old, new)), &changed, &empty)?; } } // What a page no longer draws must not stay reachable through an inherited or shared diff --git a/crates/redact/src/tests.rs b/crates/redact/src/tests.rs index 4d2cc76017..5c0eece859 100644 --- a/crates/redact/src/tests.rs +++ b/crates/redact/src/tests.rs @@ -2549,3 +2549,31 @@ fn removing_form_fields_takes_hidden_widgets_and_their_values_out_of_the_file() assert_no_secret(&doc, b"LEAK_"); assert!(annots_of(&doc, &pdfcraft_model::pages(&doc).swap_remove(0).dict).is_empty()); } + +/// Removing hidden text cleans the structure tree as redaction does: the elements describing the +/// removed text lose their descriptions, and a reference into a rewritten Form points at the copy. +#[test] +fn removing_hidden_text_cleans_the_structure_tree() { + use crate::sanitize::{Hidden, remove_hidden}; + let mut doc = one_page( + b"/P <> BDC BT /F1 10 Tf 3 Tr 10 200 Td (SECRET) Tj ET EMC /P <> BDC BT /F1 10 Tf 0 Tr 10 100 Td (Public) Tj ET EMC /Fm Do", + "/XObject << /Fm 7 0 R >>", + vec![ + stream( + "/Type /XObject /Subtype /Form /BBox [0 0 300 300] /Resources << /Font << /F1 5 0 R >> >>", + b"/P <> BDC BT /F1 10 Tf 3 Tr 10 150 Td (FORMSECRET) Tj ET EMC", + ), + b"<< /Type /StructTreeRoot /K 9 0 R >>".to_vec(), + b"<< /S /Document /P 8 0 R /K [10 0 R 11 0 R 12 0 R] >>".to_vec(), + b"<< /S /P /P 9 0 R /Pg 3 0 R /ActualText (LEAK_PAGE_TAG) /K 0 >>".to_vec(), + b"<< /S /P /P 9 0 R /Pg 3 0 R /ActualText (Public) /K 1 >>".to_vec(), + b"<< /S /P /P 9 0 R /Pg 3 0 R /Alt (LEAK_FORM_TAG) /K << /Type /MCR /Pg 3 0 R /Stm 7 0 R /MCID 0 >> >>".to_vec(), + ], + ); + doc.update_dict(doc.root().unwrap(), |d| d.set(b"StructTreeRoot".to_vec(), Object::Ref(ObjRef::new(8, 0)))).unwrap(); + doc.update_dict(ObjRef::new(3, 0), |d| d.set(b"StructParents".to_vec(), Object::Int(0))).unwrap(); + remove_hidden(&mut doc, &[Hidden::HiddenText]).unwrap(); + assert_no_secret(&doc, b"LEAK_"); + assert_no_secret(&doc, b"SECRET"); + assert!(doc.get(ObjRef::new(11, 0)).as_dict().unwrap().contains(b"ActualText"), "untouched content keeps its tags"); +} From 822179ccf2334418c8a711b993528da31d0977ed Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 14:51:06 +0000 Subject: [PATCH 19/22] fix(redact): remove hidden data from every object that owns it Metadata, private application data, associated files and additional actions were removed from the catalog and pages only. The same entries on an image or font (XMP), a Form XObject (PieceInfo), a page or annotation (/AF with its embedded file) or a form field (/AA scripts) survived the full rewrite, and the scan did not count them, so removing only such a category reported nothing to remove. A walk of every object reachable from the catalog now counts and strips these entries wherever they are, leaving name maps such as resource dictionaries alone (their keys are names a producer chose), and refuses a graph nested too deeply to walk in full. --- crates/redact/src/lib.rs | 4 + crates/redact/src/sanitize.rs | 193 +++++++++++++++++++++++++++++----- crates/redact/src/tests.rs | 94 +++++++++++++++++ 3 files changed, 265 insertions(+), 26 deletions(-) diff --git a/crates/redact/src/lib.rs b/crates/redact/src/lib.rs index 364f7cc193..968773b91d 100644 --- a/crates/redact/src/lib.rs +++ b/crates/redact/src/lib.rs @@ -64,6 +64,10 @@ pub enum RedactError { "the document's tags can't be cleaned completely (an element written in place, or a structure tree too large or too deep to follow); the operation was refused" )] Tags, + /// Objects written in place nest deeper than hidden information is followed, so its removal + /// could not be shown complete. + #[error("the document's objects nest too deeply to remove hidden information completely; the operation was refused")] + TooDeep, #[error(transparent)] Edit(#[from] pdfcraft_edit::EditError), #[error(transparent)] diff --git a/crates/redact/src/sanitize.rs b/crates/redact/src/sanitize.rs index fea0449512..d9edfbde95 100644 --- a/crates/redact/src/sanitize.rs +++ b/crates/redact/src/sanitize.rs @@ -15,7 +15,7 @@ use crate::{RedactError, Report, annots_of, listed_annotations, page_streams}; pub enum Hidden { /// Document information (`/Info`) and XMP metadata streams. Metadata, - /// Embedded files and file attachment annotations. + /// Embedded files, associated files (`/AF`) and file attachment annotations. Attachments, /// Comments and markup annotations (with their pop-ups). Comments, @@ -163,6 +163,148 @@ fn outline_len_seen( n } +/// Entries that hold a category's data wherever they appear, not only on the catalog and pages: +/// XMP on images, fonts and Forms, private data on Forms, associated files on pages and +/// annotations, additional actions on fields. +const OWNED: [(Hidden, &[u8]); 4] = + [(Hidden::Metadata, b"Metadata"), (Hidden::PrivateData, b"PieceInfo"), (Hidden::Attachments, b"AF"), (Hidden::LinksActionsScripts, b"AA")]; + +/// Keys whose dictionary values map names a producer chooses to objects (resources, glyph +/// procedures, named destinations, role and class maps, private data by application). A name +/// there may be spelt like an owned entry (a font called `/AF`), so these maps are walked but +/// never stripped. +const NAME_MAPS: [&[u8]; 14] = [ + b"Font", + b"XObject", + b"ExtGState", + b"ColorSpace", + b"Pattern", + b"Shading", + b"Properties", + b"CharProcs", + b"Dests", + b"RoleMap", + b"ClassMap", + b"Colorants", + b"Schema", + b"PieceInfo", +]; + +/// How deep the owner sweep follows dictionaries and arrays written in place. +const MAX_DIRECT_DEPTH: usize = 64; + +/// What a dictionary is to the owner sweep. +#[derive(Clone, Copy, PartialEq, Eq, Hash)] +enum Held { + /// An object that may own an entry of [`OWNED`]. + Owner, + /// A [`NAME_MAPS`] map: its keys are names, its values owners. + Map, + /// An appearance dictionary (`/AP`): its dictionary values map appearance states. + Appearances, +} + +impl Held { + fn child(self, key: &[u8]) -> Held { + match self { + Held::Owner if NAME_MAPS.contains(&key) => Held::Map, + Held::Owner if key == b"AP" => Held::Appearances, + Held::Appearances => Held::Map, + _ => Held::Owner, + } + } +} + +/// The objects reachable from the catalog that hold one of `keys` outside a name map, and how +/// many dictionaries hold each key. Each object is read at most once per [`Held`] role, so a +/// shared or cyclic graph costs one step per object. +fn owners(doc: &Document, keys: &[&[u8]]) -> Result<(Vec, Vec<(ObjRef, Held)>), RedactError> { + let mut held = vec![0; keys.len()]; + let mut holders = Vec::new(); + let Some(root) = doc.root() else { return Ok((held, holders)) }; + let mut next = vec![(root, Held::Owner)]; + let mut seen = std::collections::HashSet::new(); + while let Some((r, role)) = next.pop() { + if !seen.insert((r, role)) { + continue; + } + if find_owned(&doc.get(r), role, keys, &mut held, &mut next, 0)? { + holders.push((r, role)); + } + } + Ok((held, holders)) +} + +/// Counts the dictionaries in `o` (and what it holds in place) that hold one of `keys`, and +/// queues the indirect objects it names. Returns whether any key was found. +fn find_owned(o: &Object, role: Held, keys: &[&[u8]], held: &mut [usize], next: &mut Vec<(ObjRef, Held)>, depth: usize) -> Result { + if depth > MAX_DIRECT_DEPTH { + return Err(RedactError::TooDeep); + } + let (d, role) = match o { + Object::Ref(r) => { + next.push((*r, role)); + return Ok(false); + } + Object::Array(a) => { + let mut found = false; + for v in a { + found |= find_owned(v, Held::Owner, keys, held, next, depth + 1)?; + } + return Ok(found); + } + Object::Dict(d) => (d, role), + // A stream's dictionary is never a name map. + Object::Stream(s) => (&s.dict, Held::Owner), + _ => return Ok(false), + }; + let mut found = false; + if role != Held::Map { + for (i, k) in keys.iter().enumerate() { + if d.contains(k) + && let Some(n) = held.get_mut(i) + { + *n = n.saturating_add(1); + found = true; + } + } + } + for (k, v) in d.iter() { + found |= find_owned(v, role.child(k), keys, held, next, depth + 1)?; + } + Ok(found) +} + +/// Removes `keys` from `o` and from what it holds in place, outside name maps. [`owners`] has +/// already refused anything nested past the depth limit. +fn strip_owned(o: &mut Object, role: Held, keys: &[&[u8]], depth: usize) { + if depth > MAX_DIRECT_DEPTH { + return; + } + let (d, role) = match o { + Object::Array(a) => { + for v in a { + strip_owned(v, Held::Owner, keys, depth + 1); + } + return; + } + Object::Dict(d) => (d, role), + Object::Stream(s) => (&mut s.dict, Held::Owner), + _ => return, + }; + if role != Held::Map { + for k in keys { + d.remove(k); + } + } + let names: Vec> = d.iter().filter(|(_, v)| matches!(v, Object::Array(_) | Object::Dict(_))).map(|(k, _)| k.clone()).collect(); + for k in names { + if let Some(v) = d.get_mut(&k) { + strip_owned(v, role.child(&k), keys, depth + 1); + } + } +} + /// A comment as the comments list sees it, less file attachments (removed with Attachments) and /// redaction marks. fn is_comment(subtype: &[u8]) -> bool { @@ -296,6 +438,11 @@ pub fn scan(doc: &Document) -> Vec<(Hidden, usize)> { let annots: Vec = pages.iter().flat_map(|p| annots_of(doc, &p.dict)).filter_map(|a| doc.resolve(&a).as_dict().cloned()).collect(); let count_sub = |s: &[u8]| annots.iter().filter(|a| a.name(b"Subtype") == Some(s)).count(); let names = sub(doc, &cat, b"Names").unwrap_or_default(); + let owned_keys: Vec<&[u8]> = OWNED.iter().map(|o| o.1).collect(); + // A graph nested too deeply to walk counts as holding everything, so the removal runs and + // refuses it. + let held = owners(doc, &owned_keys).map_or_else(|_| vec![1; OWNED.len()], |o| o.0); + let owned = |h: Hidden| OWNED.iter().position(|o| o.0 == h).and_then(|i| held.get(i)).copied().unwrap_or(0); let mut doc2 = doc.clone(); HIDDEN .into_iter() @@ -303,10 +450,13 @@ pub fn scan(doc: &Document) -> Vec<(Hidden, usize)> { let n = match h { Hidden::Metadata => { let info = doc.trailer().get(b"Info").and_then(|i| doc.resolve(i).as_dict().map(|d| d.len())).unwrap_or(0); - let xmp = usize::from(cat.contains(b"Metadata")) + pages.iter().filter(|p| p.dict.contains(b"Metadata")).count(); - info + xmp + info + owned(Hidden::Metadata) + } + Hidden::Attachments => { + sub(doc, &names, b"EmbeddedFiles").map_or(0, |t| name_tree_len(doc, &t)) + + count_sub(b"FileAttachment") + + owned(Hidden::Attachments) } - Hidden::Attachments => sub(doc, &names, b"EmbeddedFiles").map_or(0, |t| name_tree_len(doc, &t)) + count_sub(b"FileAttachment"), Hidden::Comments => annots.iter().filter(|a| is_comment(a.name(b"Subtype").unwrap_or(b""))).count(), Hidden::FormFields => pdfcraft_forms::fields(doc).len(), Hidden::HiddenText => content_pass(&mut doc2, true, false, false).map(|(n, _)| n).unwrap_or(0), @@ -318,12 +468,11 @@ pub fn scan(doc: &Document) -> Vec<(Hidden, usize)> { Hidden::LinksActionsScripts => { count_sub(b"Link") + usize::from(cat.get(b"OpenAction").is_some_and(|a| doc.resolve(a).as_dict().is_some())) - + usize::from(cat.contains(b"AA")) - + pages.iter().filter(|p| p.dict.contains(b"AA")).count() - + annots.iter().filter(|a| a.contains(b"AA") || (a.name(b"Subtype") == Some(b"Widget") && a.contains(b"A"))).count() + + owned(Hidden::LinksActionsScripts) + + annots.iter().filter(|a| !a.contains(b"AA") && a.name(b"Subtype") == Some(b"Widget") && a.contains(b"A")).count() + sub(doc, &names, b"JavaScript").map_or(0, |t| name_tree_len(doc, &t)) } - Hidden::PrivateData => usize::from(cat.contains(b"PieceInfo")) + pages.iter().filter(|p| p.dict.contains(b"PieceInfo")).count(), + Hidden::PrivateData => owned(Hidden::PrivateData), }; (h, n) }) @@ -406,24 +555,12 @@ pub fn remove_hidden(doc: &mut Document, which: &[Hidden]) -> Result Result { - cat.remove(b"Metadata"); doc.trailer_mut().remove(b"Info"); } Hidden::Attachments => { if let Some(n) = names.as_mut() { n.remove(b"EmbeddedFiles"); } - cat.remove(b"AF"); } Hidden::Bookmarks => { cat.remove(b"Outlines"); @@ -457,14 +592,10 @@ pub fn remove_hidden(doc: &mut Document, which: &[Hidden]) -> Result { - cat.remove(b"PieceInfo"); - } Hidden::HiddenLayers if !off.is_empty() => { // The off layers leave the configuration (their content is gone already). if let Some(mut oc) = sub(doc, &cat, b"OCProperties") { @@ -494,6 +625,16 @@ pub fn remove_hidden(doc: &mut Document, which: &[Hidden]) -> Result = OWNED.iter().filter(|o| which.contains(&o.0)).map(|o| o.1).collect(); + if !keys.is_empty() { + for (r, role) in owners(doc, &keys)?.1 { + let mut o = (*doc.get(r)).clone(); + strip_owned(&mut o, role, &keys, 0); + doc.set(r, o); + } + } if done.is_empty() { return Err(RedactError::NothingToApply); } diff --git a/crates/redact/src/tests.rs b/crates/redact/src/tests.rs index 5c0eece859..603fede2c7 100644 --- a/crates/redact/src/tests.rs +++ b/crates/redact/src/tests.rs @@ -2577,3 +2577,97 @@ fn removing_hidden_text_cleans_the_structure_tree() { assert_no_secret(&doc, b"SECRET"); assert!(doc.get(ObjRef::new(11, 0)).as_dict().unwrap().contains(b"ActualText"), "untouched content keeps its tags"); } + +/// One page drawing image /Im (object 7) and form /Fm (object 8), with `extra` objects from 9 on. +fn owners_fixture(image: &str, form: &str, extra: Vec>) -> Document { + let mut objs = vec![ + stream(&format!("/Type /XObject /Subtype /Image /Width 1 /Height 1 /ColorSpace /DeviceGray /BitsPerComponent 8 {image}"), b"\x80"), + stream(&format!("/Type /XObject /Subtype /Form /BBox [0 0 10 10] {form}"), b"0 0 5 5 re f"), + ]; + objs.extend(extra); + one_page(b"BT /F1 10 Tf 10 100 Td (Visible) Tj ET q 10 0 0 10 0 0 cm /Im Do Q /Fm Do", "/XObject << /Im 7 0 R /Fm 8 0 R >>", objs) +} + +/// Removing one category and saving leaves none of `secret` in the file, and the scan counted it. +fn removes_from_every_owner(mut doc: Document, h: crate::sanitize::Hidden, secret: &[u8]) { + use crate::sanitize::{remove_hidden, scan}; + let counts: std::collections::HashMap<_, usize> = scan(&doc).into_iter().collect(); + assert!(counts[&h] > 0, "{h:?} is counted"); + remove_hidden(&mut doc, &[h]).unwrap(); + assert_no_secret(&doc, secret); + assert!(scan(&reopen(&doc)).into_iter().all(|(k, n)| k != h || n == 0 || h == crate::sanitize::Hidden::Metadata)); +} + +#[test] +fn private_data_of_a_form_xobject_is_removed() { + let doc = owners_fixture("", "/PieceInfo << /App << /Private (LEAK_PRIVATE) >> >>", vec![]); + removes_from_every_owner(doc, crate::sanitize::Hidden::PrivateData, b"LEAK_"); +} + +#[test] +fn metadata_of_an_image_is_removed() { + let doc = owners_fixture("/Metadata 9 0 R", "", vec![stream("/Type /Metadata /Subtype /XML", b"LEAK_XMP")]); + removes_from_every_owner(doc, crate::sanitize::Hidden::Metadata, b"LEAK_"); +} + +#[test] +fn a_pages_associated_files_are_removed_with_attachments() { + let mut doc = owners_fixture( + "", + "", + vec![b"<< /Type /Filespec /F (a.txt) /AFRelationship /Data /EF << /F 10 0 R >> >>".to_vec(), stream("/Type /EmbeddedFile", b"LEAK_ATTACHED")], + ); + doc.update_dict(ObjRef::new(3, 0), |d| d.set(b"AF".to_vec(), ref_array(&[9]))).unwrap(); + removes_from_every_owner(doc, crate::sanitize::Hidden::Attachments, b"LEAK_"); +} + +#[test] +fn a_fields_own_actions_are_removed_with_links_actions_and_scripts() { + let mut doc = owners_fixture( + "", + "", + vec![ + b"<< /FT /Tx /T (total) /AA << /C << /S /JavaScript /JS (LEAK_SCRIPT) >> >> /Kids [10 0 R] >>".to_vec(), + b"<< /Type /Annot /Subtype /Widget /Parent 9 0 R /Rect [10 200 150 220] >>".to_vec(), + ], + ); + doc.update_dict(doc.root().unwrap(), |d| d.set(b"AcroForm".to_vec(), fields_dict(&[9]))).unwrap(); + doc.update_dict(ObjRef::new(3, 0), |d| d.set(b"Annots".to_vec(), ref_array(&[10]))).unwrap(); + removes_from_every_owner(doc, crate::sanitize::Hidden::LinksActionsScripts, b"LEAK_"); +} + +/// Resource names are chosen by the producer: a font named like an owned entry stays. +#[test] +fn a_resource_named_like_an_owned_entry_is_kept() { + let mut doc = one_page( + b"BT /AF 10 Tf 10 100 Td (Visible) Tj ET", + "", + vec![b"<< /Type /Filespec /F (a.txt) /EF << /F 8 0 R >> >>".to_vec(), stream("/Type /EmbeddedFile", b"LEAK_ATTACHED")], + ); + doc.update_dict(ObjRef::new(3, 0), |d| { + let mut fonts = Dict::new(); + fonts.set(b"AF".to_vec(), Object::Ref(ObjRef::new(5, 0))); + let mut res = Dict::new(); + res.set(b"Font".to_vec(), Object::Dict(fonts)); + d.set(b"Resources".to_vec(), Object::Dict(res)); + d.set(b"AF".to_vec(), ref_array(&[7])); + }) + .unwrap(); + removes_from_every_owner(doc.clone(), crate::sanitize::Hidden::Attachments, b"LEAK_"); + crate::sanitize::remove_hidden(&mut doc, &[crate::sanitize::Hidden::Attachments]).unwrap(); + let page = pdfcraft_model::pages(&doc).swap_remove(0); + let res = doc.resolve(page.dict.get(b"Resources").unwrap()).as_dict().cloned().unwrap(); + assert!(res.get(b"Font").and_then(|f| f.as_dict()).is_some_and(|f| f.contains(b"AF"))); + assert!(!page.dict.contains(b"AF")); +} + +/// Owners written in place deeper than the sweep follows are refused, not skipped. +#[test] +fn owners_nested_too_deeply_are_refused() { + use crate::sanitize::{Hidden, remove_hidden}; + let nested = format!("{}<< /AA << /O << /S /JavaScript /JS (LEAK_DEEP) >> >> >>{}", "[".repeat(100), "]".repeat(100)); + let mut doc = one_page(b"BT ET", "", vec![format!("<< /Deep {nested} >>").into_bytes()]); + doc.update_dict(ObjRef::new(3, 0), |d| d.set(b"PieceInfo".to_vec(), Object::Ref(ObjRef::new(7, 0)))).unwrap(); + let out = remove_hidden(&mut doc, &[Hidden::LinksActionsScripts]); + assert!(matches!(out, Err(RedactError::TooDeep)), "{out:?}"); +} From 94563649e604acbd3fb2a8c3cca0185b63dd2493 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 16:22:48 +0000 Subject: [PATCH 20/22] fix(redact): name the owner sweep's holder list for clippy --- crates/redact/src/sanitize.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/redact/src/sanitize.rs b/crates/redact/src/sanitize.rs index d9edfbde95..7349c433ae 100644 --- a/crates/redact/src/sanitize.rs +++ b/crates/redact/src/sanitize.rs @@ -215,10 +215,13 @@ impl Held { } } +/// Indirect objects that hold an owned entry, each with the role it was reached in. +type Holders = Vec<(ObjRef, Held)>; + /// The objects reachable from the catalog that hold one of `keys` outside a name map, and how /// many dictionaries hold each key. Each object is read at most once per [`Held`] role, so a /// shared or cyclic graph costs one step per object. -fn owners(doc: &Document, keys: &[&[u8]]) -> Result<(Vec, Vec<(ObjRef, Held)>), RedactError> { +fn owners(doc: &Document, keys: &[&[u8]]) -> Result<(Vec, Holders), RedactError> { let mut held = vec![0; keys.len()]; let mut holders = Vec::new(); let Some(root) = doc.root() else { return Ok((held, holders)) }; From 4d64ca8e2da819dff1d0e08c63939560d3b62a78 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 16:23:26 +0000 Subject: [PATCH 21/22] docs(redact): say which objects hidden-information removal reaches --- crates/redact/README.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/redact/README.md b/crates/redact/README.md index 9c2793e248..2caf067b5b 100644 --- a/crates/redact/README.md +++ b/crates/redact/README.md @@ -71,7 +71,11 @@ sanitize::sanitize(&mut doc)?; // all of them Categories: metadata (`/Info`, XMP), file attachments, comments, form fields (flattened so the values stay visible), hidden text (render modes 3 and 7, or wholly off the page), hidden layers (content of off optional-content groups, then the groups), bookmarks, links/actions/JavaScript, -and `/PieceInfo` private data. Both make the next save a full rewrite. +and `/PieceInfo` private data. XMP metadata, private data, associated files (`/AF`) and +additional actions (`/AA`) are removed from every object reachable from the catalog that owns +them (images, fonts, Forms, annotations and fields as well as the catalog and pages), never from +name maps such as resource dictionaries. Content is read strictly: a page whose content can't be +read in full is refused, not cleaned in part. Both make the next save a full rewrite. `patterns` finds phone numbers, e-mail addresses, credit card numbers (Luhn-checked), US Social Security numbers and dates for Search & Redact. From 241c5512d646e9be2964ccba96a69be5ab579bb5 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 17:04:40 +0000 Subject: [PATCH 22/22] fix(redact): charge repeated name-tree kids to the node budget The name-tree count cloned each node's /Kids array and skipped repeated references without charging the budget, so a node listing the same kid many times cost work proportional to every entry while the budget saw one step. The walk now borrows the array, charges every entry it does not descend into, and stops once the budget is spent. --- crates/redact/src/sanitize.rs | 30 ++++++++++++++++++++---------- crates/redact/src/tests.rs | 20 ++++++++++++++++++++ 2 files changed, 40 insertions(+), 10 deletions(-) diff --git a/crates/redact/src/sanitize.rs b/crates/redact/src/sanitize.rs index 7349c433ae..5b2918f01c 100644 --- a/crates/redact/src/sanitize.rs +++ b/crates/redact/src/sanitize.rs @@ -92,28 +92,38 @@ const NAME_TREE_NODE_BUDGET: usize = 100_000; /// Entries of a name tree (`/Names` leaves, `/Kids` recursion). Each indirect node is read once, /// however many `/Kids` arrays list it, so shared or cyclic kids cost one step per node rather -/// than one per path; depth and a total node budget bound the rest. +/// than one per path. Every `/Kids` entry the walk does not descend into (a repeated reference or +/// a non-dictionary) costs a step as well, so a long `/Kids` array of repeats cannot outrun the +/// budget. Depth and a total step budget bound the rest. fn name_tree_len(doc: &Document, node: &Dict) -> usize { let (mut seen, mut visited) = (std::collections::HashSet::new(), 0); name_tree_len_seen(doc, node, 0, &mut seen, &mut visited) } fn name_tree_len_seen(doc: &Document, node: &Dict, depth: usize, seen: &mut std::collections::HashSet, visited: &mut usize) -> usize { - *visited += 1; + *visited = visited.saturating_add(1); if depth > 32 || *visited > NAME_TREE_NODE_BUDGET { return 0; } let leaves = node.get(b"Names").and_then(|n| doc.resolve(n).as_array().map(|a| a.len() / 2)).unwrap_or(0); - let kids = node.get(b"Kids").and_then(|k| doc.resolve(k).as_array().cloned()).unwrap_or_default(); let mut n = leaves; - for k in &kids { - if let Object::Ref(r) = k - && !seen.insert(*r) - { - continue; + let kids = node.get(b"Kids").map(|k| doc.resolve(k)); + for k in kids.as_deref().and_then(Object::as_array).map_or(&[][..], Vec::as_slice) { + if *visited > NAME_TREE_NODE_BUDGET { + break; } - if let Some(kid) = doc.resolve(k).as_dict() { - n = n.saturating_add(name_tree_len_seen(doc, kid, depth + 1, seen, visited)); + let held; + let kid = match k { + Object::Ref(r) if !seen.insert(*r) => None, + Object::Ref(r) => { + held = doc.get(*r); + held.as_dict() + } + other => other.as_dict(), + }; + match kid { + Some(kid) => n = n.saturating_add(name_tree_len_seen(doc, kid, depth + 1, seen, visited)), + None => *visited = visited.saturating_add(1), } } n diff --git a/crates/redact/src/tests.rs b/crates/redact/src/tests.rs index 603fede2c7..a9902495df 100644 --- a/crates/redact/src/tests.rs +++ b/crates/redact/src/tests.rs @@ -2671,3 +2671,23 @@ fn owners_nested_too_deeply_are_refused() { let out = remove_hidden(&mut doc, &[Hidden::LinksActionsScripts]); assert!(matches!(out, Err(RedactError::TooDeep)), "{out:?}"); } + +/// A name tree node that lists the same kid more times than the node budget: each repeat costs a +/// step, so the walk stops at the budget instead of scanning every repeat, and the distinct kid +/// listed after them is never reached. +#[test] +fn repeated_name_tree_kids_count_towards_the_node_budget() { + use crate::sanitize::{Hidden, scan}; + let repeats = "6 0 R ".repeat(100_010); + let doc = pdf(vec![ + b"<< /Type /Catalog /Pages 2 0 R /Names << /EmbeddedFiles 5 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 >>".to_vec(), + stream("", b"BT ET"), + format!("<< /Kids [{repeats}7 0 R] >>").into_bytes(), + b"<< /Names [(a.txt) << /Type /Filespec /F (a.txt) >>] >>".to_vec(), + b"<< /Names [(b.txt) << /Type /Filespec /F (b.txt) >>] >>".to_vec(), + ]); + let counts: std::collections::HashMap = scan(&doc).into_iter().collect(); + assert_eq!(counts[&Hidden::Attachments], 1, "the walk went past its node budget to the last kid"); +}