diff --git a/crates/sign/src/pdf/listing.rs b/crates/sign/src/pdf/listing.rs index f0b14f5ba2..e4a34763af 100644 --- a/crates/sign/src/pdf/listing.rs +++ b/crates/sign/src/pdf/listing.rs @@ -207,17 +207,26 @@ const MAX_FIELD_NAME_BYTES: usize = 32 << 20; /// Every signature field, depth first in `/Fields` order. pub(super) fn sig_fields(doc: &Document) -> Vec { + sig_fields_bounded(doc).0 +} + +/// [`sig_fields`], with whether the walk was cut short by [`MAX_FIELD_DEPTH`] or +/// [`MAX_FIELD_NAME_BYTES`]: the fields it could not reach are then unknown to the caller, which +/// is what lets a permission lookup fail closed instead of guessing. +pub(super) fn sig_fields_bounded(doc: &Document) -> (Vec, bool) { let mut out = Vec::new(); - let Some(root) = doc.root() else { return out }; + let mut cut = false; + let Some(root) = doc.root() else { return (out, cut) }; let Some(af) = doc.get(root).as_dict().and_then(|c| c.get(b"AcroForm").map(|a| doc.resolve(a))).and_then(|a| a.as_dict().cloned()) else { - return out; + return (out, cut); }; - let Some(fields) = af.get(b"Fields").map(|f| doc.resolve(f)).and_then(|f| f.as_array().cloned()) else { return out }; + let Some(fields) = af.get(b"Fields").map(|f| doc.resolve(f)).and_then(|f| f.as_array().cloned()) else { return (out, cut) }; let mut seen = HashSet::new(); let mut stack: Vec<(Object, String, Option>, usize)> = fields.into_iter().rev().map(|f| (f, String::new(), None, 0)).collect(); let mut name_bytes = 0usize; while let Some((o, prefix, inherited_ft, depth)) = stack.pop() { if depth > MAX_FIELD_DEPTH { + cut = true; continue; } if let Some(r) = o.as_ref() @@ -239,6 +248,7 @@ pub(super) fn sig_fields(doc: &Document) -> Vec { // this name, and the clone of it each queued sub-field is about to hold name_bytes = name_bytes.saturating_add(name.len().saturating_mul(sub.len() + 1)); if name_bytes > MAX_FIELD_NAME_BYTES { + cut = true; break; } for k in sub.into_iter().rev() { @@ -253,6 +263,44 @@ pub(super) fn sig_fields(doc: &Document) -> Vec { out.push(Field { name, r: o.as_ref(), dict: d, widgets: ws }); } } + (out, cut) +} + +/// Every `/V` an AcroForm field holds, however deep: a standalone document timestamp must not be +/// confused with a signature a field already carries. No qualified names are built here, so the +/// name budget that can stop [`sig_fields`] cannot hide a value from it. +pub(super) fn field_values(doc: &Document) -> HashSet { + let mut out = HashSet::new(); + let Some(root) = doc.root() else { return out }; + let Some(fields) = doc + .get(root) + .as_dict() + .and_then(|c| c.get(b"AcroForm").map(|a| doc.resolve(a))) + .and_then(|a| a.as_dict().cloned()) + .and_then(|a| a.get(b"Fields").map(|f| doc.resolve(f).as_array().cloned()).unwrap_or_default()) + else { + return out; + }; + let mut seen = HashSet::new(); + let mut stack: Vec<(Object, usize)> = fields.into_iter().rev().map(|f| (f, 0)).collect(); + while let Some((o, depth)) = stack.pop() { + if depth > MAX_FIELD_DEPTH { + continue; + } + if let Some(r) = o.as_ref() + && !seen.insert(r) + { + continue; + } + let Some(d) = doc.resolve(&o).as_dict().cloned() else { continue }; + if let Some(v) = d.get(b"V").and_then(Object::as_ref) { + out.insert(v); + } + let kids: Vec = d.get(b"Kids").map(|k| doc.resolve(k)).and_then(|k| k.as_array().cloned()).unwrap_or_default(); + for k in kids.into_iter().rev() { + stack.push((k, depth + 1)); + } + } out } @@ -283,11 +331,10 @@ pub fn list(doc: &Document, bytes: &[u8], trust: &TrustStore) -> Vec Vec { let pages = annot_pages(doc); let mut out = Vec::new(); - let mut field_values: HashSet = HashSet::new(); + // Every value a field holds, listed or not: a signature below the listing's bounds still + // owns its `/V`, which is then not a standalone document timestamp. + let field_values = field_values(doc); for f in sig_fields(doc) { - if let Some(v_ref) = f.dict.get(b"V").and_then(Object::as_ref) { - field_values.insert(v_ref); - } let widget = f.widgets.first(); let page = widget.and_then(|(r, w)| r.and_then(|r| pages.get(&r).copied()).or_else(|| w.reference(b"P").and_then(|p| page_index(doc, p)))); let rect = widget diff --git a/crates/sign/src/pdf/validate.rs b/crates/sign/src/pdf/validate.rs index 08b482c4b4..cee5d7a4b1 100644 --- a/crates/sign/src/pdf/validate.rs +++ b/crates/sign/src/pdf/validate.rs @@ -1,5 +1,5 @@ use super::changes::classify_changes; -use super::listing::{DigestCache, list, nums, sig_fields, text}; +use super::listing::{DigestCache, list, nums, sig_fields, sig_fields_bounded, text}; use super::types::{Detail, Fill, Modification, Status, TrustStore}; use super::{ Certificate, Dict, DigestAlg, Document, HashSet, ObjRef, Object, SignError, SignatureInfo, SignedData, Time, build_chain, build_chain_noted, @@ -246,13 +246,21 @@ fn finish_validation(context: SignatureValidation<'_>, verified: VerifiedSignatu // The permissions are the signed revision's: a later one can rewrite `/P` (#1788). This // signature is the one whose byte range ends that revision. let old = cache.revision(doc, bytes, covered); - if let Some(old) = &old - && let Some(v) = sig_fields(old) - .iter() - .filter_map(|f| f.signature(old)) - .find(|v| nums(old, v, b"ByteRange").is_some_and(|b| b.len() == 4 && b[2] + b[3] == covered as f64)) - { - info.certify = docmdp(old, &v); + // The permissions are the signed revision's: a later one can rewrite `/P` (#1788). + if let Some(old) = &old { + // When the field walk cannot reach every field, the signature that ends this revision + // is unknown, so no permission is claimed: keeping `docmdp(doc, v)`, read from the + // current revision, would let a later `/P` rewrite speak for the signed one. + info.certify = None; + let (fields, cut) = sig_fields_bounded(old); + if !cut + && let Some(v) = fields + .iter() + .filter_map(|f| f.signature(old)) + .find(|v| nums(old, v, b"ByteRange").is_some_and(|b| b.len() == 4 && b[2] + b[3] == covered as f64)) + { + info.certify = docmdp(old, &v); + } } classify_changes(doc, old, info.certify) }; diff --git a/crates/sign/src/pdf_discovery_tests.rs b/crates/sign/src/pdf_discovery_tests.rs index 6c0456ac5b..d001a21b15 100644 --- a/crates/sign/src/pdf_discovery_tests.rs +++ b/crates/sign/src/pdf_discovery_tests.rs @@ -397,3 +397,27 @@ fn dss_evidence_decodes_each_stream_once_within_its_caps() { assert!(ocsps.cut); assert!(DssEvidence::new(&doc, &dss, b"OCSPs").count() <= MAX_DSS_ENTRIES); } + +/// A field tree deeper than the walk follows is reported as cut: a permission lookup that cannot +/// reach every field must see that, so it fails closed instead of trusting a partial list. +#[test] +fn a_field_tree_past_the_walk_depth_is_reported_as_cut() { + let mut doc = fixture(&[STAMP]); + let mut kid = None; + for i in 0..80 { + let mut f = Dict::new(); + f.set(b"FT".to_vec(), Object::name("Sig")); + f.set(b"T".to_vec(), Object::String(PdfString::literal(format!("f{i}")))); + if let Some(k) = kid { + f.set(b"Kids".to_vec(), Object::Array(vec![Object::Ref(k)])); + } + let id = doc.add(f); + kid = Some(id); + } + let mut form = Dict::new(); + form.set(b"Fields".to_vec(), Object::Array(vec![Object::Ref(kid.unwrap())])); + doc.update_dict(doc.root().unwrap(), |d| d.set(b"AcroForm".to_vec(), Object::Dict(form))).unwrap(); + let (fields, cut) = super::listing::sig_fields_bounded(&doc); + assert!(cut, "the walk stopped at its depth limit without saying so"); + assert!(fields.len() < 80, "a cut walk lists what it reached: {}", fields.len()); +}