fix(sign): judge a shared signature value once per listing; dedupe DSS evidence by object number (#6041)

* fix(sign): judge a shared signature value once per listing; dedupe DSS evidence by object number

* address review: shared signature values are judged by output index without copying the dictionary; a dead evidence reference does not hide a live one

* address review: repeated evidence references are skipped before they are resolved

* address review: each full evidence reference is tried once

crates/sign/src/pdf/listing.rs

@@ -431,8 +431,12 @@ pub fn list(doc: &Document, bytes: &[u8], trust: &TrustStore) -> Vec<SignatureIn
 /// structure and validation against `bytes` and `trust` are read again on every call.
 pub fn list_cached(doc: &Document, bytes: &[u8], trust: &TrustStore, cache: &DigestCache) -> Vec<SignatureInfo> {
     let pages = annot_pages(doc);
-    let mut out = Vec::new();
+    let mut out: Vec<SignatureInfo> = Vec::new();
     let mut field_values: HashSet<ObjRef> = HashSet::new();
+    // Fields that name one signature value are judged once: validation does not depend on the
+    // field, and repeating it per field would repeat the digest, chain and evidence work.
+    // The row of `out` that holds each value's verdict: a later field copies it, not a map of copies.
+    let mut judged: HashMap<ObjRef, usize> = HashMap::new();
     let (fields, whole) = walk_sig_fields(doc);
     for f in fields {
         if let Some(v_ref) = f.dict.get(b"V").and_then(Object::as_ref) {
@@ -445,6 +449,13 @@ pub fn list_cached(doc: &Document, bytes: &[u8], trust: &TrustStore, cache: &Dig
             .filter(|r| r.len() == 4)
             .map(|r| [r[0].min(r[2]), r[1].min(r[3]), r[0].max(r[2]), r[1].max(r[3])]);
         let visible = widget.is_some_and(|(_, w)| widget_shown(doc, w));
+        // A repeat of a judged value reads nothing of it: the signature dictionary is not copied.
+        let shared = f.dict.get(b"V").and_then(Object::as_ref).filter(|_| f.dict.get(b"V").is_some_and(|v| doc.resolve(v).as_dict().is_some()));
+        if let Some(first) = shared.and_then(|r| judged.get(&r)).and_then(|&i| out.get(i)) {
+            let own = SignatureInfo { field: f.name.clone(), signed: true, page, rect, visible, ..first.clone() };
+            out.push(own);
+            continue;
+        }
         let v = f.signature(doc);
         let mut info = SignatureInfo {
             field: f.name.clone(),
@@ -477,6 +488,9 @@ pub fn list_cached(doc: &Document, bytes: &[u8], trust: &TrustStore, cache: &Dig
         if let Some(v) = v {
             validate_into(doc, bytes, trust, &v, &mut info, cache);
         }
+        if let Some(r) = shared {
+            judged.insert(r, out.len());
+        }
         out.push(info);
     }
     // Standalone document timestamps (ISO 32000-2 §12.8.2.2) live outside AcroForm fields.

crates/sign/src/pdf/validate.rs

@@ -371,7 +371,11 @@ pub(crate) const MAX_DSS_BYTES: usize = 64 << 20;
 pub(crate) struct DssEvidence<'a> {
     doc: &'a Document,
     items: std::vec::IntoIter<Object>,
-    seen: HashSet<ObjRef>,
+    /// Object numbers read: `Document::get` resolves a reference by number, so `5 0 R` and `5 1 R`
+    /// are one stream.
+    seen: HashSet<u32>,
+    /// Full references already tried, resolved or not.
+    tried: HashSet<ObjRef>,
     taken: usize,
     left: usize,
     /// A cap stopped the array being read in full.
@@ -381,7 +385,7 @@ pub(crate) struct DssEvidence<'a> {
 impl<'a> DssEvidence<'a> {
     pub(crate) fn new(doc: &'a Document, dss: &Dict, key: &[u8]) -> Self {
         let items = dss.get(key).map(|o| doc.resolve(o)).and_then(|o| o.as_array().cloned()).unwrap_or_default();
-        DssEvidence { doc, items: items.into_iter(), seen: HashSet::new(), taken: 0, left: MAX_DSS_BYTES, cut: false }
+        DssEvidence { doc, items: items.into_iter(), seen: HashSet::new(), tried: HashSet::new(), taken: 0, left: MAX_DSS_BYTES, cut: false }
     }
 }
 
@@ -395,11 +399,21 @@ impl Iterator for DssEvidence<'_> {
         // Repeats are skipped before the caps count anything, so repeating one stream cannot use
         // them up; the scan of the array itself is bounded by the parser's cap on array length.
         for item in self.items.by_ref() {
-            if item.as_ref().is_some_and(|r| !self.seen.insert(r)) {
+            // Resolved first: a reference to a generation the slot doesn't hold reads as null and
+            // must not make a later, valid reference to the same object number a repeat.
+            // A repeat is skipped before anything is resolved: a long chain of repeats costs no
+            // lookups. The number is recorded only once the reference gave a stream.
+            // Each full reference is tried once, so one that never reaches a stream (a cycle) costs
+            // its lookups once, however often it is listed; `seen` keeps object numbers apart so a
+            // dead generation can't hide a live one.
+            if item.as_ref().is_some_and(|r| self.seen.contains(&r.num) || !self.tried.insert(r)) {
                 continue;
             }
             let o = self.doc.resolve(&item);
             let Object::Stream(s) = o.as_ref() else { continue };
+            if let Some(r) = item.as_ref() {
+                self.seen.insert(r.num);
+            }
             if self.taken >= MAX_DSS_ENTRIES {
                 self.cut = true;
                 return None;

crates/sign/src/pdf_discovery_tests.rs

@@ -373,6 +373,19 @@ fn only_grew_compares_huge_arrays_without_quadratic_work() {
     assert!(start.elapsed() < std::time::Duration::from_secs(10), "{:?}", start.elapsed());
 }
 
+/// A reference to a generation the object doesn't have reads as null: it is no repeat of the valid
+/// reference to the same object number that follows it.
+#[test]
+fn dss_evidence_skips_a_dead_reference_without_hiding_the_live_one() {
+    let mut doc = fixture(&[]);
+    let live = doc.add(Object::Stream(Stream::flate(Dict::new(), b"evidence")));
+    let dead = ObjRef::new(live.num, live.generation + 1);
+    let mut dss = Dict::new();
+    dss.set(b"CRLs".to_vec(), Object::Array(vec![Object::Ref(dead), Object::Ref(live)]));
+    let read: Vec<Vec<u8>> = DssEvidence::new(&doc, &dss, b"CRLs").collect();
+    assert_eq!(read, vec![b"evidence".to_vec()]);
+}
+
 #[test]
 fn dss_evidence_decodes_each_stream_once_within_its_caps() {
     // One small stream listed 1000 times, then streams that inflate past the evidence cap.

crates/sign/tests/it/pdf.rs

@@ -3369,6 +3369,38 @@ fn signing_a_field_with_a_null_value_after_another_signature_keeps_the_first_val
     );
 }
 
+/// Fields that name one signature value are listed each, under their own name, with the verdict
+/// the value earned once.
+#[test]
+fn fields_sharing_a_signature_value_are_listed_with_one_verdict() {
+    let id = pkcs12::open(&data("rsa-aes.p12"), "test").unwrap();
+    let trust = TrustStore { certs: vec![id.certificate.clone()] };
+    let signed = pdfcraft_sign::sign(&open(&fixture()), &id, &SignOptions { field: Some("Approval".into()), ..opts() }).unwrap();
+    let v = open(&signed).get(pdfcraft_cos::ObjRef { num: 6, generation: 0 }).as_dict().and_then(|d| d.get(b"V").cloned()).unwrap();
+    let shared = edit_after(&signed, |doc| {
+        let mut w = pdfcraft_cos::Dict::new();
+        w.set(b"Type".to_vec(), Object::name("Annot"));
+        w.set(b"Subtype".to_vec(), Object::name("Widget"));
+        w.set(b"FT".to_vec(), Object::name("Sig"));
+        w.set(b"T".to_vec(), PdfString::text("Twin"));
+        w.set(b"V".to_vec(), v.clone());
+        w.set(b"Rect".to_vec(), Object::Array(vec![Object::Int(0); 4]));
+        let w = doc.add(Object::Dict(w));
+        let root = doc.root().unwrap();
+        let af = doc.get(root).as_dict().and_then(|c| c.get(b"AcroForm").cloned()).unwrap();
+        let af = doc.resolve(&af).as_dict().cloned().unwrap();
+        let mut fields = af.get(b"Fields").map(|f| doc.resolve(f)).and_then(|f| f.as_array().cloned()).unwrap();
+        fields.push(Object::Ref(w));
+        let mut af = af;
+        af.set(b"Fields".to_vec(), Object::Array(fields));
+        doc.update_dict(root, |c| c.set(b"AcroForm".to_vec(), Object::Dict(af))).unwrap();
+    });
+    let list = signatures(&open(&shared), &shared, &trust);
+    let (a, b) = (list.iter().find(|s| s.field == "Approval").unwrap(), list.iter().find(|s| s.field == "Twin").unwrap());
+    assert!(a.signed && b.signed);
+    assert_eq!((a.status, &a.modification, &a.signer, a.signed_len), (b.status, &b.modification, &b.signer, b.signed_len));
+}
+
 #[test]
 fn the_catalogs_perms_do_not_change_under_no_changes() {
     // /Perms names the certification signature (DocMDP). Under P=1 nothing may change, so a later