Merge PR #4132: fix(sign): judge a page's indirect /Annots array by the annotations it gains

crates/sign/src/pdf.rs

@@ -1357,6 +1357,11 @@ fn classify_changes(doc: &Document, old: Option<Document>, p: Option<u8>) -> Mod
     // attacker-written version carries (relabelling a signed page /Type /Sig, or dropping its
     // /Type and adding /ModDate, must not reclassify it away from page content).
     let signed_pages: HashSet<ObjRef> = pdfcraft_annot::page_refs(&old).unwrap_or_default().into_iter().collect();
+    // The signed pages' /Annots arrays that are objects of their own. Adding an annotation updates
+    // such an array in place (the page dictionary does not change), so it is judged like a
+    // page's inline /Annots: by the annotations it gained.
+    let signed_annots: HashSet<ObjRef> =
+        signed_pages.iter().filter_map(|pg| old.get(*pg).as_dict().and_then(|d| d.get(b"Annots").and_then(Object::as_ref))).collect();
     // What each signed page draws from, by the role it held when signed (#1787).
     let signed_resources = signed_page_resources(&old, &signed_pages);
     let old_info = old.trailer().get(b"Info").and_then(Object::as_ref);
@@ -1425,6 +1430,24 @@ fn classify_changes(doc: &Document, old: Option<Document>, p: Option<u8>) -> Mod
             (Some(k), _) => k,
             // The Document Security Store's own arrays and dictionaries, when they only grew.
             (_, _) if dss.contains(&num) => kind::SECURITY_STORE,
+            // A signed page's indirect /Annots array, still an array: what it lost, then what it
+            // gained. Dropping a signed annotation is never hidden behind an added signature.
+            (_, Object::Array(now)) if signed_annots.contains(&r) && before.as_deref().is_some_and(|b| b.as_array().is_some()) => {
+                let was: HashSet<ObjRef> =
+                    before.as_deref().and_then(Object::as_array).map(|a| a.iter().filter_map(Object::as_ref).collect()).unwrap_or_default();
+                let kept: HashSet<ObjRef> = now.iter().filter_map(Object::as_ref).collect();
+                let removed: Vec<ObjRef> = was.iter().filter(|r| !kept.contains(r)).copied().collect();
+                if removed.is_empty() {
+                    let added: Vec<ObjRef> = now.iter().filter_map(Object::as_ref).filter(|r| !was.contains(r)).collect();
+                    added_annots(doc, &added)
+                } else if removed.iter().any(|r| old.get(*r).as_dict().is_some_and(|d| d.name(b"Subtype") == Some(b"Widget"))) {
+                    // A form field or signature widget taken off the page: no permission covers it.
+                    kind::OTHER
+                } else {
+                    // Deleting comments is a comment change (permitted at P 3 only).
+                    kind::COMMENTS
+                }
+            }
             (_, Object::Dict(d)) => {
                 let was = before.as_deref().and_then(Object::as_dict);
                 if signed_pages.contains(&r) {

crates/sign/tests/it/pdf.rs

@@ -144,6 +144,71 @@ fn signing_an_existing_field_and_counter_signing() {
     ));
 }
 
+/// A page whose /Annots is an object of its own: signing and commenting update that array in
+/// place, not the page. A second signature is a signature change for the first, as with an
+/// inline /Annots, and a comment is still a comment (refused under a form-fill certification).
+#[test]
+fn an_indirect_annots_array_is_judged_by_the_annotations_it_gains() {
+    let page = pdfcraft_cos::ObjRef { num: 3, generation: 0 };
+    let unsigned = edit_after(&fixture(), |doc| {
+        let annots = doc.add(Object::Array(vec![Object::Ref(pdfcraft_cos::ObjRef { num: 6, generation: 0 })]));
+        doc.update_dict(page, |p| p.set(b"Annots".to_vec(), Object::Ref(annots))).unwrap();
+    });
+    let annots_ref = |bytes: &[u8]| open(bytes).get(page).as_dict().and_then(|d| d.get(b"Annots").and_then(Object::as_ref));
+    let annots = annots_ref(&unsigned).unwrap();
+    let id = pkcs12::open(&data("rsa-aes.p12"), "test").unwrap();
+    let trust = TrustStore { certs: vec![id.certificate.clone()] };
+    let first = pdfcraft_sign::sign(&open(&unsigned), &id, &opts()).unwrap();
+    let second = pdfcraft_sign::sign(&open(&first), &id, &SignOptions { rect: Some([20.0, 160.0, 220.0, 210.0]), ..opts() }).unwrap();
+    assert_eq!(annots_ref(&second), Some(annots), "the signer kept the page's array indirect");
+    let doc = open(&second);
+    let sigs = signatures(&doc, &second, &trust);
+    let s = sigs.iter().find(|s| s.field == "Signature1").unwrap();
+    assert_eq!(s.modification, Modification::Allowed(vec![kind::SIGNATURE]), "{:?}", s.details);
+    assert_eq!(s.status, Status::Valid);
+    // A comment appended to the indirect array.
+    let comment = |doc: &mut Document| {
+        let mut d = pdfcraft_cos::Dict::new();
+        d.set(b"Type".to_vec(), Object::name("Annot"));
+        d.set(b"Subtype".to_vec(), Object::name("Text"));
+        d.set(b"Rect".to_vec(), Object::Array(vec![Object::Int(10), Object::Int(10), Object::Int(30), Object::Int(30)]));
+        let r = doc.add(Object::Dict(d));
+        let mut a = doc.get(annots).as_array().cloned().unwrap();
+        a.push(Object::Ref(r));
+        doc.set(annots, Object::Array(a));
+    };
+    let check = |bytes: &[u8]| signatures(&open(bytes), bytes, &trust).into_iter().find(|s| s.signed).unwrap();
+    let s = check(&edit_after(&first, comment));
+    assert_eq!((s.status, s.modification), (Status::Valid, Modification::Allowed(vec![kind::COMMENTS])));
+    let certified = pdfcraft_sign::sign(&open(&unsigned), &id, &SignOptions { certify: Some(2), ..opts() }).unwrap();
+    let s = check(&edit_after(&certified, comment));
+    assert_eq!((s.status, s.modification), (Status::Invalid, Modification::Disallowed(vec![kind::COMMENTS])));
+    // Under that certification, a signed comment swapped for a new signature widget is still a
+    // deleted comment, and a form field taken off the page is permitted by no level.
+    let commented = pdfcraft_sign::sign(&open(&edit_after(&unsigned, comment)), &id, &SignOptions { certify: Some(2), ..opts() }).unwrap();
+    let swap = |drop: &dyn Fn(&Document, &Object) -> bool| {
+        edit_after(&commented, |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("Later"));
+            w.set(b"Rect".to_vec(), Object::Array(vec![Object::Int(0); 4]));
+            let w = doc.add(Object::Dict(w));
+            let mut a = doc.get(annots).as_array().cloned().unwrap();
+            a.retain(|o| !drop(doc, o));
+            a.push(Object::Ref(w));
+            doc.set(annots, Object::Array(a));
+        })
+    };
+    let subtype =
+        |doc: &Document, o: &Object, t: &[u8]| o.as_ref().is_some_and(|r| doc.get(r).as_dict().is_some_and(|d| d.name(b"Subtype") == Some(t)));
+    let s = check(&swap(&|doc, o| subtype(doc, o, b"Text")));
+    assert_eq!((s.status, s.modification), (Status::Invalid, Modification::Disallowed(vec![kind::COMMENTS])));
+    let s = check(&swap(&|doc, o| subtype(doc, o, b"Widget") && o.as_ref().is_some_and(|r| r.num == 6)));
+    assert_eq!((s.status, s.modification), (Status::Invalid, Modification::Disallowed(vec![kind::OTHER])));
+}
+
 /// Issue #1328: a signature field whose /V is null, directly or through a reference, is unsigned:
 /// it is listed as unsigned and can be signed by name.
 #[test]