From dad86bf9f55840eb6350e181bc306a24286cfb4a Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Fri, 23 Oct 2026 09:21:00 -0400 Subject: [PATCH 1/3] fix(redact,sign,cos): close high-severity findings from the agent scan Redact: refuse Redact marks written in place; remove replies and pop-ups listed on other pages; delete form fields by object, not name; resolve Do by its last operand like the renderer; sanitize drops PCAdded and flattens widgets before hidden-content cleanup. Sign: freed signed objects and edits to an already-signed widget are modifications. Cos: only reading trusts /Type /XRef to skip encryption. --- crates/content/src/lib.rs | 6 +++ crates/cos/src/security.rs | 5 ++- crates/cos/tests/it/encryption.rs | 17 +++++++ crates/forms/src/author.rs | 13 +++++- crates/forms/src/lib.rs | 4 +- crates/redact/src/interp.rs | 10 ++--- crates/redact/src/lib.rs | 75 ++++++++++++++++++++++++++----- crates/redact/src/sanitize.rs | 17 ++++--- crates/redact/src/tests.rs | 59 ++++++++++++++++++++++++ crates/sign/src/pdf.rs | 21 +++++++++ crates/sign/tests/it/pdf.rs | 26 +++++++++++ 11 files changed, 226 insertions(+), 27 deletions(-) diff --git a/crates/content/src/lib.rs b/crates/content/src/lib.rs index bb478d18e..c52916d33 100644 --- a/crates/content/src/lib.rs +++ b/crates/content/src/lib.rs @@ -52,6 +52,12 @@ impl Op { pub fn name(&self, i: usize) -> Option<&[u8]> { self.operands.get(i).and_then(Object::as_name) } + + /// The last operand as a name: the one `Do` and the other one-operand operators act on, since + /// the renderer takes operands from the end and ignores any surplus before them. + pub fn last_name(&self) -> Option<&[u8]> { + self.operands.last().and_then(Object::as_name) + } } /// A parsed stream. diff --git a/crates/cos/src/security.rs b/crates/cos/src/security.rs index 12d08b8dc..22c85c7f6 100644 --- a/crates/cos/src/security.rs +++ b/crates/cos/src/security.rs @@ -178,7 +178,10 @@ fn walk(o: &Object, strings: &dyn Fn(&PdfString) -> PdfString, h: &SecurityHandl Object::Stream(s) => { let mut dict: Dict = s.dict.iter().map(|(k, v)| (k.clone(), walk(v, strings, h, num, generation, pass))).collect(); let ty = s.dict.name(b"Type"); - if ty == Some(b"XRef") || (ty == Some(b"Metadata") && !h.encrypts_metadata()) { + // Cross-reference streams are never encrypted (§7.5.8), but only reading may trust the + // label: a stream a producer calls /XRef stays plaintext if writing believes it, and the + // writer builds its own cross-reference streams outside this transform. + if (decrypt && ty == Some(b"XRef")) || (ty == Some(b"Metadata") && !h.encrypts_metadata()) { return Object::Stream(Stream { dict, raw: s.raw.clone() }); } let mut named = named_crypt_filter(&s.dict); diff --git a/crates/cos/tests/it/encryption.rs b/crates/cos/tests/it/encryption.rs index 2c6aad244..2ee33afc1 100644 --- a/crates/cos/tests/it/encryption.rs +++ b/crates/cos/tests/it/encryption.rs @@ -477,3 +477,20 @@ fn removing_an_inline_encrypt_dictionary_leaves_an_unencrypted_file() { assert!(hayro_syntax::Pdf::new(plain).is_ok(), "{alg:?}"); } } + +/// A content stream that calls itself `/Type /XRef` is still encrypted: only reading trusts the label. +#[test] +fn a_stream_labelled_xref_is_encrypted_when_saved() { + let mut doc = Document::open(Arc::new(fixture_with(" /Type /XRef"))).unwrap(); + doc.set_encryption(&NewEncryption { + algorithm: Algorithm::Aes256, + user_password: "u", + owner_password: "o", + permissions: -4, + encrypt_metadata: true, + seed: [9; 32], + }) + .unwrap(); + let bytes = write_full(&doc, &SaveOptions { object_streams: false, ..SaveOptions::default() }).unwrap(); + assert!(!bytes.windows(SECRET.len()).any(|w| w == SECRET.as_bytes()), "the labelled stream was written in the clear"); +} diff --git a/crates/forms/src/author.rs b/crates/forms/src/author.rs index 544d0a7f8..c3424050d 100644 --- a/crates/forms/src/author.rs +++ b/crates/forms/src/author.rs @@ -1239,8 +1239,17 @@ fn rename_in_calculations(doc: &mut Document, all: &[Field], old: &str, new: &st /// Delete a field: its widgets leave their pages and the field leaves the form. pub fn delete_field(doc: &mut Document, name: &str) -> Result<(), FormError> { - let all = fields(doc); - let f = all.iter().find(|f| f.name == name).ok_or_else(|| FormError::NoSuchField(name.into()))?.clone(); + let f = fields(doc).into_iter().find(|f| f.name == name).ok_or_else(|| FormError::NoSuchField(name.into()))?; + delete_found_field(doc, f) +} + +/// [`delete_field`] for the field object `obj`, when separate fields share one name. +pub fn delete_field_object(doc: &mut Document, obj: ObjRef) -> Result<(), FormError> { + let f = fields(doc).into_iter().find(|f| f.obj == obj).ok_or_else(|| FormError::NoSuchField(format!("{obj:?}")))?; + delete_found_field(doc, f) +} + +fn delete_found_field(doc: &mut Document, f: Field) -> Result<(), FormError> { let widgets: std::collections::HashSet = f.widgets.iter().map(|w| w.obj).collect(); for p in page_refs(doc) { let Some(a) = doc.get(p).as_dict().and_then(|d| d.get(b"Annots").cloned()) else { continue }; diff --git a/crates/forms/src/lib.rs b/crates/forms/src/lib.rs index 82c1c8e74..d120c3cd3 100644 --- a/crates/forms/src/lib.rs +++ b/crates/forms/src/lib.rs @@ -26,8 +26,8 @@ mod scripting; pub mod table; pub use actions::{FieldAction, Trigger, field_actions, set_field_actions}; pub use author::{ - BorderStyle, CheckStyle, FieldFont, FieldProps, Look, LookPatch, MIN_FIELD_SIDE, NewField, add_field, check_style, delete_field, duplicate_field, - look, redraw_field, set_button_icon, set_props, + BorderStyle, CheckStyle, FieldFont, FieldProps, Look, LookPatch, MIN_FIELD_SIDE, NewField, add_field, check_style, delete_field, + delete_field_object, duplicate_field, look, redraw_field, set_button_icon, set_props, }; pub use calc_order::{CalculatedField, CalculationIssue, CalculationOrder, calculation_order, set_calculation_order}; pub use scripting::{ diff --git a/crates/redact/src/interp.rs b/crates/redact/src/interp.rs index 9851c2e8c..fcd2ebe83 100644 --- a/crates/redact/src/interp.rs +++ b/crates/redact/src/interp.rs @@ -415,7 +415,7 @@ pub(crate) fn prune(xo: &mut Dict, replaced: &[Vec], content: &[Vec]) { } let joined = Pieces::join(content); let ops = joined.parse().ops; - let still_drawn = |n: &[u8]| ops.iter().any(|op| op.is("Do") && op.name(0) == Some(n)); + let still_drawn = |n: &[u8]| ops.iter().any(|op| op.is("Do") && op.last_name() == Some(n)); for n in replaced { if !still_drawn(n) { xo.remove(n); @@ -430,7 +430,7 @@ fn content_draws(doc: &Document, content: &[Vec], xo: &Dict, name: &[u8], de let joined = Pieces::join(content); let ops = joined.parse().ops; for op in ops.iter().filter(|op| op.is("Do")) { - let Some(n) = op.name(0) else { continue }; + let Some(n) = op.last_name() else { continue }; if n == name { return Some(true); } @@ -672,7 +672,7 @@ fn resource_ops(doc: &Document, res: &Dict, content: &[Vec]) -> Result]) -> let mut xo = res_dict(doc, res, b"XObject"); let names: Vec> = xo.iter().map(|(n, _)| n.to_vec()).collect(); for name in names { - if !ops.iter().any(|op| op.is("Do") && op.name(0) == Some(&name)) { + if !ops.iter().any(|op| op.is("Do") && op.last_name() == Some(&name)) { xo.remove(&name); } } @@ -1204,7 +1204,7 @@ fn xobject( xobjects: &mut Dict, out: &mut Output, ) -> Option> { - let name = op.name(0)?.to_vec(); + let name = op.last_name()?.to_vec(); let r = xobjects.get(&name)?.as_ref()?; let obj = doc.get(r); let Object::Stream(s) = &*obj else { return None }; diff --git a/crates/redact/src/lib.rs b/crates/redact/src/lib.rs index 00fbbbd17..81b975a0c 100644 --- a/crates/redact/src/lib.rs +++ b/crates/redact/src/lib.rs @@ -185,7 +185,14 @@ pub(crate) fn marks_checked(doc: &Document) -> (Vec, Vec) { let (mut out, mut unreadable) = (Vec::new(), Vec::new()); for (pi, p) in pdfcraft_model::pages(doc).iter().enumerate() { for a in annots_of(doc, &p.dict) { - let Some(r) = a.as_ref() else { continue }; + let Some(r) = a.as_ref() else { + // A mark written in place has no object to find, apply or remove: refuse it + // rather than apply the others as if it were not there. + if doc.resolve(&a).as_dict().is_some_and(|d| d.name(b"Subtype") == Some(b"Redact")) { + unreadable.push(pi); + } + continue; + }; let obj = doc.get(r); let Some(d) = obj.as_dict() else { continue }; if d.name(b"Subtype") != Some(b"Redact") { @@ -400,13 +407,14 @@ fn apply_to(doc: &mut Document, pages: Option<&[usize]>) -> Result, _>>()?; report.pages = by_page.len(); localize_resources(doc, &by_page)?; - let mut doomed_fields: Vec = Vec::new(); + let mut doomed_fields: Vec = Vec::new(); // The original image and form XObjects the rewrite drops or replaces, on any page. let mut doomed_xobjects: Vec = Vec::new(); let mut retired_properties = Vec::new(); - let widget_owner: Vec<(ObjRef, String)> = - pdfcraft_forms::fields(doc).into_iter().flat_map(|f| f.widgets.iter().map(|w| (w.obj, f.name.clone())).collect::>()).collect(); + let widget_owner: Vec<(ObjRef, ObjRef)> = + pdfcraft_forms::fields(doc).into_iter().flat_map(|f| f.widgets.iter().map(|w| (w.obj, f.obj)).collect::>()).collect(); let listed_before = listed_annotations(doc); + let mut removed_everywhere: HashSet = HashSet::new(); for &(pi, page_marks) in &groups { let page = pdfcraft_model::pages(doc).swap_remove(pi); @@ -521,10 +529,10 @@ fn apply_to(doc: &mut Document, pages: Option<&[usize]>) -> Result) -> Result = annots .into_iter() .filter(|a| { @@ -578,10 +587,56 @@ fn apply_to(doc: &mut Document, pages: Option<&[usize]>) -> Result Re _ => Dict::new(), }; dict.remove(b"Length"); + // The source text of an added item (Edit ▸ Add content) would keep what was removed. + dict.remove(b"PCAdded"); new_list[i] = Object::Ref(doc.add(Object::Stream(pdfcraft_cos::Stream::flate(dict, &bytes)))); changed = true; } @@ -505,13 +507,6 @@ pub fn remove_hidden(doc: &mut Document, which: &[Hidden]) -> Result 0) || (layers && count(Hidden::HiddenLayers) > 0) { - content_pass(doc, text, layers, true)?.1 - } else { - std::collections::HashSet::new() - }; // Every annotation the pages list now: whatever is no longer listed at the end (flattened // widgets included) leaves the structure tree too. let listed_before = listed_annotations(doc); @@ -529,6 +524,14 @@ pub fn remove_hidden(doc: &mut Document, which: &[Hidden]) -> Result