From ce3857c2e5ec61dd6a5cf8565c7e699acc7618a7 Mon Sep 17 00:00:00 2001 From: Martin Ball Date: Thu, 5 Nov 2026 20:55:45 +0000 Subject: [PATCH 1/2] sign: classify changes by signed object role Judge each object changed after signing by the role it held in the signed revision, keyed by object number, and compare a slot's accepted generations across revisions. Adds `Document::accepted_generations` to cos. Co-Authored-By: Claude Opus 5.5 --- crates/cos/README.md | 4 +- crates/cos/src/document/objects.rs | 24 ++ crates/cos/tests/it/generations.rs | 35 +++ crates/sign/src/pdf.rs | 3 + crates/sign/src/pdf/changes.rs | 168 ++++++++------ .../src/pdf/revision_classification_tests.rs | 207 ++++++++++++++++++ crates/sign/tests/it/main.rs | 1 + .../sign/tests/it/revision_classification.rs | 158 +++++++++++++ 8 files changed, 536 insertions(+), 64 deletions(-) create mode 100644 crates/sign/src/pdf/revision_classification_tests.rs create mode 100644 crates/sign/tests/it/revision_classification.rs diff --git a/crates/cos/README.md b/crates/cos/README.md index 774f2c4367..9f5fb56725 100644 --- a/crates/cos/README.md +++ b/crates/cos/README.md @@ -62,7 +62,9 @@ another. Edits never touch the original bytes. not expand compact xref ranges; the winning xref entry remains authoritative for free slots. Compressed objects require generation zero. Explicit `set` overlays accept only the supplied generation; `set_existing` and `update_dict` retain both accepted generations of a - damaged live slot, including through snapshots, and require full rewrite. Headers beyond + damaged live slot, including through snapshots, and require full rewrite. + `accepted_generations(num)` lists the generations a slot accepts, so code comparing two + revisions can tell whether a reference still reaches the same slot. Headers beyond this bounded probe are not additionally recovered. Materialized primary and hybrid rows are diagnosed before folding, so a newer row does not hide a retained disagreement. - **Tolerant, never fatal.** The lexer works on a byte slice and a position; it allocates nothing diff --git a/crates/cos/src/document/objects.rs b/crates/cos/src/document/objects.rs index 8d9917d749..458840de3a 100644 --- a/crates/cos/src/document/objects.rs +++ b/crates/cos/src/document/objects.rs @@ -175,6 +175,30 @@ impl Document { } } + /// The generations a reference to `num` may carry and still resolve (see [`Self::get`]), + /// sorted: none for a free or unknown slot, else the slot's generation, and for a damaged + /// file slot (or a repair-aware edit of one) also the generation its object header carries. + /// Code comparing revisions (signature validation) uses it to tell whether the references + /// that reached an object in one revision still reach the same slot in another. + pub fn accepted_generations(&self, num: u32) -> Vec { + // Mirrors `reference_matches`, which stays allocation-free for every lookup. + let mut generations = match self.overlay.get(&num) { + Some(Slot::Set(generation, _)) => vec![*generation], + Some(Slot::RepairedSet(generation, alias, _)) => vec![*generation, *alias], + Some(Slot::Freed(_)) => Vec::new(), + None => match self.entries.lookup(num, &self.data) { + Some(XrefEntry::InFile { offset, generation }) => { + std::iter::once(generation).chain(self.indexed_header_reference(num, offset).map(|h| h.generation)).collect() + } + Some(XrefEntry::InStream { .. }) => vec![0], + None | Some(XrefEntry::Free { .. }) => Vec::new(), + }, + }; + generations.sort_unstable(); + generations.dedup(); + generations + } + pub(super) fn try_get_reference(&self, r: ObjRef) -> Result, CosError> { if !self.reference_matches(r) { return Ok(Arc::new(Object::Null)); diff --git a/crates/cos/tests/it/generations.rs b/crates/cos/tests/it/generations.rs index 3d56628c93..bbe2ff0b33 100644 --- a/crates/cos/tests/it/generations.rs +++ b/crates/cos/tests/it/generations.rs @@ -473,3 +473,38 @@ fn restoring_explicit_sets_removes_old_generation_aliases() { } } } + +/// `accepted_generations` lists exactly the generations `get` resolves, for every kind of slot: +/// a clean file slot, a damaged one (row and header disagree), a repair-aware edit, a plain +/// edit, a freed slot, a free row, an object stream member and an unknown number. +#[test] +fn accepted_generations_are_the_generations_get_resolves() { + let agrees = |doc: &Document, num: u32, expected: &[u16]| { + assert_eq!(doc.accepted_generations(num), expected, "object {num}"); + for generation in 0..8 { + let live = !matches!(*doc.get(ObjRef::new(num, generation)), Object::Null); + assert_eq!(live, expected.contains(&generation), "object {num} generation {generation}"); + } + }; + let doc = Document::open(contradictory_entry(false)).unwrap(); + agrees(&doc, 1, &[0]); + agrees(&doc, 3, &[0, 1]); + agrees(&doc, 9, &[]); + let mut repaired = doc.clone(); + repaired.set_existing(ObjRef::new(3, 0), Object::Int(77)).unwrap(); + agrees(&repaired, 3, &[0, 1]); + let mut narrowed = doc.clone(); + narrowed.set(ObjRef::new(3, 2), Object::Int(99)); + agrees(&narrowed, 3, &[2]); + let mut freed = doc.clone(); + freed.free(ObjRef::new(3, 0)); + agrees(&freed, 3, &[]); + agrees(&Document::open(contradictory_entry(true)).unwrap(), 3, &[]); + let mut doc = + Document::open(object_file(&[(1, 0, "<< /Type /Catalog /Pages 2 0 R >>"), (2, 0, "<< /Type /Pages /Kids [] /Count 0 >>")])).unwrap(); + let member = doc.add(Object::Dict(Default::default())); + doc.update_dict(ObjRef::new(1, 0), |c| c.set(b"Extra".to_vec(), Object::Ref(member))).unwrap(); + let packed = Document::open(Arc::new(write_full(&doc, &SaveOptions { object_streams: true, ..SaveOptions::default() }).unwrap())).unwrap(); + assert!(matches!(packed.xref_entry(member.num), Some(XrefEntry::InStream { .. })), "the member is in an object stream"); + agrees(&packed, member.num, &[0]); +} diff --git a/crates/sign/src/pdf.rs b/crates/sign/src/pdf.rs index 75b42f9905..8fafff7f01 100644 --- a/crates/sign/src/pdf.rs +++ b/crates/sign/src/pdf.rs @@ -45,6 +45,9 @@ use validate::{DssEvidence, MAX_DSS_BYTES, MAX_DSS_ENTRIES}; #[cfg(test)] #[path = "pdf_discovery_tests.rs"] mod discovery_tests; +#[cfg(test)] +#[path = "pdf/revision_classification_tests.rs"] +mod revision_classification_tests; /// One signature field. #[derive(Clone, Debug)] diff --git a/crates/sign/src/pdf/changes.rs b/crates/sign/src/pdf/changes.rs index 3a10a74e41..81467542e6 100644 --- a/crates/sign/src/pdf/changes.rs +++ b/crates/sign/src/pdf/changes.rs @@ -7,7 +7,11 @@ pub(super) fn classify_changes(doc: &Document, old: Option, p: Option< let Some(old) = old else { return Modification::Disallowed(vec![kind::UNREADABLE]); }; - let old_content: HashSet = pdfcraft_annot::page_refs(&old) + // Every role below is keyed by object number, not by the reference as written: a damaged + // file's slot accepts two generations (#1143), and a page signed as `3 5 R` and edited as + // `3 0 R` is still that page. The loop over changed objects refuses any slot whose accepted + // generations changed, so a number names the same slot in both revisions. + let old_content: HashSet = pdfcraft_annot::page_refs(&old) .unwrap_or_default() .into_iter() .flat_map(|pg| { @@ -21,24 +25,34 @@ pub(super) fn classify_changes(doc: &Document, old: Option, p: Option< _ => Vec::new(), } }) + .map(|r| r.num) .collect(); // The signed revision's document-level XMP stream. - let old_metadata = old.root().and_then(|root| old.get(root).as_dict().and_then(|c| c.get(b"Metadata").and_then(Object::as_ref))); + let old_metadata = old.root().and_then(|root| old.get(root).as_dict().and_then(|c| c.get(b"Metadata").and_then(Object::as_ref))).map(|r| r.num); // The signed revision's XFA data packets: filling the form rewrites them. let old_xfa_data = xfa_data_packets(&old); // What each object was in the signed revision: its pages, and its trailer /Info. A changed // object is judged by the role it held when signed, never by the labels or keys its new, // 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 = pdfcraft_annot::page_refs(&old).unwrap_or_default().into_iter().collect(); + let signed_page_refs = pdfcraft_annot::page_refs(&old).unwrap_or_default(); + let signed_pages: HashSet = signed_page_refs.iter().map(|r| r.num).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 = - signed_pages.iter().filter_map(|pg| old.get(*pg).as_dict().and_then(|d| d.get(b"Annots").and_then(Object::as_ref))).collect(); + let signed_annots: HashSet = signed_page_refs + .iter() + .filter_map(|pg| old.get(*pg).as_dict().and_then(|d| d.get(b"Annots").and_then(Object::as_ref))) + .map(|r| r.num) + .collect(); + // The annotations the signed pages listed. One of them listed again elsewhere was moved or + // copied there, not added. + let placed_annots: HashSet = + signed_page_refs.iter().flat_map(|pg| refs(&old, old.get(*pg).as_dict().and_then(|d| d.get(b"Annots")))).map(|r| r.num).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); + let signed_resources = signed_page_resources(&old, &signed_page_refs); + let old_info = old.trailer().get(b"Info").and_then(Object::as_ref).map(|r| r.num); + let signed_role = |num: u32| old_content.contains(&num) || signed_pages.contains(&num) || signed_resources.contains_key(&num); let (mut allowed, mut disallowed): (Vec<&'static str>, Vec<&'static str>) = (Vec::new(), Vec::new()); let dss = dss_parts(doc, &old); // Stored at the same place in both (and not edited since): the same bytes, unchanged. @@ -61,8 +75,7 @@ pub(super) fn classify_changes(doc: &Document, old: Option, p: Option< // while every comparison here finds the signed object untouched. No writer has a reason to // add such an entry after signing: it is never a permitted change. for (_, num) in doc.xref_oddities().iter().filter(|o| !old.xref_oddities().contains(o)) { - let r = ObjRef { num: *num, generation: doc.generation(*num) }; - let what = if old_content.contains(&r) || signed_pages.contains(&r) { kind::PAGE_CONTENT } else { kind::OTHER }; + let what = if old_content.contains(num) || signed_pages.contains(num) { kind::PAGE_CONTENT } else { kind::OTHER }; if !disallowed.contains(&what) { disallowed.push(what); } @@ -80,6 +93,7 @@ pub(super) fn classify_changes(doc: &Document, old: Option, p: Option< old_xfa_data: &old_xfa_data, signed_pages: &signed_pages, signed_annots: &signed_annots, + placed_annots: &placed_annots, signed_resources: &signed_resources, old_info, dss: &dss, @@ -88,6 +102,19 @@ pub(super) fn classify_changes(doc: &Document, old: Option, p: Option< if same_place(num) { continue; } + // The signed revision's references reached this slot by the generations it accepted. If + // it accepts others now, those references may reach nothing here while another reader + // follows them back through /Prev to an intermediate revision, or to another alias, and + // shows what that holds: the object compared below is not provably what is displayed. + // No writer changes a signed object's generation. + let signed_generations = old.accepted_generations(num); + if !signed_generations.is_empty() && signed_generations != doc.accepted_generations(num) { + let what = if signed_role(num) { kind::PAGE_CONTENT } else { kind::OTHER }; + if !disallowed.contains(&what) { + disallowed.push(what); + } + continue; + } let r = ObjRef { num, generation: doc.generation(num) }; let new = doc.get(r); let before = old.try_get(num).ok().filter(|o| !matches!(**o, Object::Null)); @@ -124,9 +151,8 @@ pub(super) fn classify_changes(doc: &Document, old: Option, p: Option< // rewritten by every save and is not a change. let current: HashSet = doc.object_numbers().into_iter().collect(); for num in old.object_numbers().into_iter().filter(|n| !current.contains(n)) { - let r = ObjRef { num, generation: old.generation(num) }; - let was = old.get(r); - let signed_role = old_content.contains(&r) || signed_pages.contains(&r) || signed_resources.contains_key(&r); + let was = old.get(ObjRef { num, generation: old.generation(num) }); + let signed_role = signed_role(num); // A signed page's content or resource is judged by that role, whatever /Type its stream carried. if !signed_role && matches!(&*was, Object::Stream(s) if matches!(s.dict.name(b"Type"), Some(b"XRef" | b"ObjStm"))) { continue; @@ -142,13 +168,14 @@ pub(super) fn classify_changes(doc: &Document, old: Option, p: Option< struct ChangedObjectClassifier<'a> { doc: &'a Document, old: &'a Document, - old_content: &'a HashSet, - old_metadata: Option, - old_xfa_data: &'a HashSet, - signed_pages: &'a HashSet, - signed_annots: &'a HashSet, - signed_resources: &'a HashMap, - old_info: Option, + old_content: &'a HashSet, + old_metadata: Option, + old_xfa_data: &'a HashSet, + signed_pages: &'a HashSet, + signed_annots: &'a HashSet, + placed_annots: &'a HashSet, + signed_resources: &'a HashMap, + old_info: Option, dss: &'a HashSet, } @@ -170,7 +197,7 @@ impl ChangedObjectClassifier<'_> { } fn resource_kind(&self, r: ObjRef, new: &Object, before: Option<&Object>) -> Option<&'static str> { - match (self.signed_resources.get(&r), new) { + match (self.signed_resources.get(&r.num), new) { (Some(false), _) => Some(kind::PAGE_CONTENT), (Some(true), Object::Dict(dict)) if self.keeps_previous_entries(dict, before) => None, (Some(true), _) => Some(kind::PAGE_CONTENT), @@ -183,32 +210,40 @@ impl ChangedObjectClassifier<'_> { } fn is_signed_annotation_array(&self, r: ObjRef, before: Option<&Object>) -> bool { - self.signed_annots.contains(&r) && before.is_some_and(|object| object.as_array().is_some()) + self.signed_annots.contains(&r.num) && before.is_some_and(|object| object.as_array().is_some()) } fn annotation_array_change(&self, now: &[Object], before: Option<&Object>) -> &'static str { - let was: HashSet = - before.and_then(Object::as_array).map(|items| items.iter().filter_map(Object::as_ref).collect()).unwrap_or_default(); - let kept: HashSet = now.iter().filter_map(Object::as_ref).collect(); - let removed: Vec = was.iter().filter(|reference| !kept.contains(reference)).copied().collect(); - if removed.is_empty() { - let added: Vec = now.iter().filter_map(Object::as_ref).filter(|reference| !was.contains(reference)).collect(); - added_annots(self.doc, &added) - } else if removed.iter().any(|reference| self.old.get(*reference).as_dict().is_some_and(|dict| dict.name(b"Subtype") == Some(b"Widget"))) { - // A form field or signature widget taken off the page: no permission covers it. + let was: Vec = before.and_then(Object::as_array).map(|items| items.iter().filter_map(Object::as_ref).collect()).unwrap_or_default(); + self.annots_change(&was, &now.iter().filter_map(Object::as_ref).collect::>()) + } + + /// A signed page's annotations going from `was` to `now`, compared by object number. + fn annots_change(&self, was: &[ObjRef], now: &[ObjRef]) -> &'static str { + let had: HashSet = was.iter().map(|r| r.num).collect(); + let kept: HashSet = now.iter().map(|r| r.num).collect(); + let removed: Vec = had.iter().filter(|num| !kept.contains(num)).copied().collect(); + let added: Vec = now.iter().filter(|r| !had.contains(&r.num)).copied().collect(); + let was_widget = |num: u32| self.old.try_get(num).ok().is_some_and(|o| o.as_dict().is_some_and(|d| d.name(b"Subtype") == Some(b"Widget"))); + // A signed annotation listed on another page was moved or copied there. + let moved: Vec = added.iter().map(|r| r.num).filter(|num| self.placed_annots.contains(num)).collect(); + if removed.iter().chain(&moved).any(|num| was_widget(*num)) { + // A form field or signature widget taken off its page, or onto another: no permission covers it. kind::OTHER - } else { - // Deleting comments is a comment change (permitted at P 3 only). + } else if !removed.is_empty() || !moved.is_empty() { + // Deleting or moving comments is a comment change (permitted at P 3 only). kind::COMMENTS + } else { + added_annots(self.doc, &added) } } fn dictionary_change(&self, r: ObjRef, dict: &Dict, before: Option<&Object>) -> &'static str { let previous = before.and_then(Object::as_dict); - if self.signed_pages.contains(&r) { + if self.signed_pages.contains(&r.num) { // A signed page is judged by what the page shows, whatever its new labels say. - page_change_kind(self.doc, self.old, dict, previous) - } else if previous.is_some() && Some(r) == self.old_info { + self.page_change_kind(dict, previous) + } else if previous.is_some() && Some(r.num) == self.old_info { // Only the signed revision's own /Info dictionary is a metadata change. kind::METADATA } else { @@ -216,17 +251,29 @@ impl ChangedObjectClassifier<'_> { } } + /// A changed object that was a page in the signed revision, judged by what the page shows + /// rather than by any `/Type` its new version carries. Only its annotations may change (see + /// [`Self::annots_change`]); any other difference (content, resources, a relabelled or dropped + /// `/Type`) is a page content change. + fn page_change_kind(&self, d: &Dict, before: Option<&Dict>) -> &'static str { + match before { + Some(b) if without(d, &[b"Annots"]) != without(b, &[b"Annots"]) => kind::PAGE_CONTENT, + Some(b) => self.annots_change(&refs(self.old, b.get(b"Annots")), &refs(self.doc, d.get(b"Annots"))), + None => kind::PAGE_CONTENT, + } + } + fn stream_change(&self, r: ObjRef, stream: &pdfcraft_cos::Stream, before: Option<&Object>) -> Option<&'static str> { // File plumbing is the stream's role when signed; a later /Type label cannot change it. let was_plumbing = matches!(before, Some(Object::Stream(previous)) if matches!(previous.dict.name(b"Type"), Some(b"XRef" | b"ObjStm"))); - if self.old_content.contains(&r) { + if self.old_content.contains(&r.num) { Some(kind::PAGE_CONTENT) } else if before.is_none() || was_plumbing { None - } else if stream.dict.name(b"Type") == Some(b"Metadata") && Some(r) == self.old_metadata { + } else if stream.dict.name(b"Type") == Some(b"Metadata") && Some(r.num) == self.old_metadata { // Only the signed catalog's XMP stream is metadata. Some(kind::METADATA) - } else if self.old_xfa_data.contains(&r) { + } else if self.old_xfa_data.contains(&r.num) { // Signed XFA data and state packets are changed by a form fill. Some(kind::FORM_FILL) } else { @@ -238,7 +285,7 @@ impl ChangedObjectClassifier<'_> { /// The streams of `doc`'s XFA form that hold its data and state: the `datasets` and `form` /// packets of the AcroForm's `/XFA` array (name, stream, name, stream, …). A form given as one /// XFA stream holds its template too, and is not listed. -fn xfa_data_packets(doc: &Document) -> HashSet { +fn xfa_data_packets(doc: &Document) -> HashSet { let Some(form) = acroform(doc) else { return HashSet::new() }; let Some(xfa) = form.get(b"XFA").map(|x| doc.resolve(x)) else { return HashSet::new() }; let Object::Array(xfa) = &*xfa else { return HashSet::new() }; @@ -247,6 +294,7 @@ fn xfa_data_packets(doc: &Document) -> HashSet { .iter() .filter(|[name, _]| doc.resolve(name).as_string().is_some_and(|s| matches!(s.to_text().as_str(), "datasets" | "form"))) .filter_map(|[_, packet]| packet.as_ref()) + .map(|r| r.num) .collect() } @@ -442,23 +490,24 @@ fn is_signature_dict(d: &Dict) -> bool { /// properties), where any change changes what the page shows. Resources are the page's own or /// inherited. Bounded by depth and by the number of objects visited; past the bound the rest is /// judged as before. -fn signed_page_resources(old: &Document, pages: &HashSet) -> HashMap { +fn signed_page_resources(old: &Document, pages: &[ObjRef]) -> HashMap { const CATEGORIES: [&[u8]; 7] = [b"Font", b"XObject", b"ExtGState", b"ColorSpace", b"Pattern", b"Shading", b"Properties"]; const MAX_VISITED: usize = 50_000; const MAX_DEPTH: u8 = 32; const MAX_INHERITANCE: usize = 64; /// Everything reachable from `o` is content the page draws (a leaf role, overriding a - /// container one reached by another path). A signed page or the catalog reached this way - /// (a `/P` or `/Parent` back-reference) keeps the role of its own: the walk stops there. - fn reach(old: &Document, o: &Object, depth: u8, stop: &HashSet, roles: &mut HashMap) { + /// container one reached by another path, under any generation alias). A signed page or the + /// catalog reached this way (a `/P` or `/Parent` back-reference) keeps the role of its own: + /// the walk stops there. + fn reach(old: &Document, o: &Object, depth: u8, stop: &HashSet, roles: &mut HashMap) { if roles.len() >= MAX_VISITED || depth > MAX_DEPTH { return; } let resolved; let o = match o { Object::Ref(r) => { - if stop.contains(r) || roles.insert(*r, false).is_some() { + if stop.contains(&r.num) || roles.insert(r.num, false).is_some() { return; } resolved = old.get(*r); @@ -490,14 +539,14 @@ fn signed_page_resources(old: &Document, pages: &HashSet) -> HashMap = pages.iter().map(|r| r.num).collect(); + stop.extend(old.root().map(|r| r.num)); for page in pages { let Some(res) = resources(old, *page) else { continue }; - let container = |o: &Object, roles: &mut HashMap| -> Arc { + let container = |o: &Object, roles: &mut HashMap| -> Arc { match o { Object::Ref(r) => { - roles.entry(*r).or_insert(true); + roles.entry(r.num).or_insert(true); old.get(*r) } other => Arc::new(other.clone()), @@ -516,21 +565,6 @@ fn signed_page_resources(old: &Document, pages: &HashSet) -> HashMap) -> &'static str { - match before { - Some(b) if without(d, &[b"Annots"]) != without(b, &[b"Annots"]) => kind::PAGE_CONTENT, - Some(b) => { - let was = refs(old, b.get(b"Annots")); - let added: Vec = refs(doc, d.get(b"Annots")).into_iter().filter(|r| !was.contains(r)).collect(); - added_annots(doc, &added) - } - None => kind::PAGE_CONTENT, - } -} - /// Whether the catalog `d` keeps every developer extension the signed catalog `b` declared, each /// with the same value: it may only declare more. fn extensions_kept(doc: &Document, old: &Document, d: &Dict, b: &Dict) -> bool { @@ -557,6 +591,14 @@ fn change_kind(doc: &Document, old: &Document, d: &Dict, before: Option<&Dict>) { return kind::PAGE_CONTENT; } + // A signed form field or widget keeps that role: its value, appearance or signature may + // change, but turning it into a comment, another kind of annotation or another type of + // field is no form fill. + if before.is_some_and(|b| { + (b.name(b"Subtype") == Some(b"Widget") || b.contains(b"FT")) && (d.name(b"Subtype") != b.name(b"Subtype") || d.name(b"FT") != b.name(b"FT")) + }) { + return kind::OTHER; + } match ty { // Only a new signature value or policy: rewriting a signed one is not a signature (#1788). Some(b"Sig" | b"DocTimeStamp" | b"SigRef" | b"TransformParams") if before.is_none() => return kind::SIGNATURE, diff --git a/crates/sign/src/pdf/revision_classification_tests.rs b/crates/sign/src/pdf/revision_classification_tests.rs new file mode 100644 index 0000000000..ed338c24de --- /dev/null +++ b/crates/sign/src/pdf/revision_classification_tests.rs @@ -0,0 +1,207 @@ +//! Changes made after signing are judged against what a reader resolves and by the role each +//! object held when signed: a newer generation, a repaired generation alias, or a widget +//! replaced, removed or moved never passes as a permitted change. + +use std::sync::Arc; + +use pdfcraft_cos::{Document, ObjRef, Object}; + +use super::Modification; +use super::changes::classify_changes; + +/// One cross-reference row: an object written with header generation `.0` under row generation `.1`. +enum Row<'a> { + Obj(u16, u16, &'a str), +} + +/// A classic-xref file built one revision at a time; each later revision points back with /Prev. +#[derive(Clone)] +struct File { + bytes: Vec, + size: u32, + xref: Option, +} + +impl File { + fn new() -> Self { + File { bytes: b"%PDF-1.7\n".to_vec(), size: 1, xref: None } + } + + fn revision(mut self, rows: &[(u32, Row<'_>)]) -> Self { + let mut table = Vec::new(); + for (num, row) in rows { + self.size = self.size.max(num + 1); + let Row::Obj(header, generation, body) = row; + table.push(format!("{num} 1\n{:010} {generation:05} n \n", self.bytes.len())); + self.bytes.extend_from_slice(format!("{num} {header} obj\n{body}\nendobj\n").as_bytes()); + } + let at = self.bytes.len(); + self.bytes.extend_from_slice(b"xref\n"); + if self.xref.is_none() { + self.bytes.extend_from_slice(b"0 1\n0000000000 65535 f \n"); + } + self.bytes.extend_from_slice(table.concat().as_bytes()); + let prev = self.xref.map(|p| format!(" /Prev {p}")).unwrap_or_default(); + self.bytes.extend_from_slice(format!("trailer\n<< /Size {} /Root 1 0 R{prev} >>\nstartxref\n{at}\n%%EOF\n", self.size).as_bytes()); + self.xref = Some(at); + self + } + + fn open(&self) -> Document { + Document::open(Arc::new(self.bytes.clone())).unwrap() + } +} + +const PAGES: &str = "<< /Type /Pages /Kids [3 0 R 4 0 R] /Count 2 >>"; +const TEXT_FIELD: &str = "<< /Type /Annot /Subtype /Widget /FT /Tx /T (Name) /V (Ann) /Rect [20 20 120 40] /P 3 0 R >>"; +const COMMENT: &str = "<< /Type /Annot /Subtype /Text /Rect [20 20 40 40] /Contents (note) >>"; + +fn page(extra: &str) -> String { + format!("<< /Type /Page /Parent 2 0 R /MediaBox [0 0 300 300] {extra} >>") +} + +fn r(num: u32, generation: u16) -> ObjRef { + ObjRef { num, generation } +} + +fn assert_refused(doc: &Document, old: Document, p: u8) { + let verdict = classify_changes(doc, Some(old), Some(p)); + assert!(matches!(verdict, Modification::Disallowed(_)), "P={p}: main returns {verdict:?}"); +} + +/// Two pages and a text field on page 3, listed in /Fields and in the page's inline /Annots. +fn form(page3: &str) -> File { + File::new().revision(&[ + (1, Row::Obj(0, 0, "<< /Type /Catalog /Pages 2 0 R /AcroForm << /Fields [5 0 R] >> >>")), + (2, Row::Obj(0, 0, PAGES)), + (3, Row::Obj(0, 0, &page(page3))), + (4, Row::Obj(0, 0, &page("/Annots []"))), + (5, Row::Obj(0, 0, TEXT_FIELD)), + (6, Row::Obj(0, 0, "[5 0 R]")), + ]) +} + +#[test] +fn content_stream_intermediate_generation_cover() { + let signed = File::new().revision(&[ + (1, Row::Obj(0, 0, "<< /Type /Catalog /Pages 2 0 R >>")), + (2, Row::Obj(0, 0, PAGES)), + (3, Row::Obj(0, 0, &page(""))), + (4, Row::Obj(0, 0, &page("/Contents 5 0 R"))), + (5, Row::Obj(0, 0, "<< /Length 3 >>\nstream\nq Q\nendstream")), + ]); + let now = signed + .clone() + .revision(&[(5, Row::Obj(0, 0, "<< /Length 3 >>\nstream\nQ q\nendstream"))]) + .revision(&[(5, Row::Obj(1, 1, "<< /Length 3 >>\nstream\nq Q\nendstream"))]); + let (doc, old) = (now.open(), signed.open()); + assert_eq!(doc.revisions().len(), 3); + assert!(doc.xref_oddities().is_empty()); + assert_eq!(doc.get(r(5, 1)), old.get(r(5, 0)), "the newest generation holds the signed bytes"); + assert_refused(&doc, old, 2); +} + +#[test] +fn widget_removed_from_inline_annots() { + let signed = form("/Annots [5 0 R]"); + let mut doc = signed.open(); + doc.update_dict(r(3, 0), |p| p.set(b"Annots".to_vec(), Object::Array(Vec::new()))).unwrap(); + assert_refused(&doc, signed.open(), 3); +} + +#[test] +fn widget_moved_between_pages() { + let signed = form("/Annots [5 0 R]"); + let mut doc = signed.open(); + doc.update_dict(r(3, 0), |p| p.set(b"Annots".to_vec(), Object::Array(Vec::new()))).unwrap(); + doc.update_dict(r(4, 0), |p| p.set(b"Annots".to_vec(), Object::Array(vec![Object::Ref(r(5, 0))]))).unwrap(); + doc.update_dict(r(5, 0), |w| w.set(b"P".to_vec(), Object::Ref(r(4, 0)))).unwrap(); + assert_refused(&doc, signed.open(), 3); +} + +#[test] +fn annots_moved_to_new_object_dropping_widget() { + let signed = form("/Annots 6 0 R"); + let mut doc = signed.open(); + let empty = doc.add(Object::Array(Vec::new())); + doc.update_dict(r(3, 0), |p| p.set(b"Annots".to_vec(), Object::Ref(empty))).unwrap(); + assert_refused(&doc, signed.open(), 3); +} + +#[test] +fn repaired_page_relabelled_as_field() { + let signed = File::new().revision(&[ + (1, Row::Obj(0, 0, "<< /Type /Catalog /Pages 2 0 R >>")), + (2, Row::Obj(0, 0, "<< /Type /Pages /Kids [3 5 R 4 0 R] /Count 2 >>")), + (3, Row::Obj(5, 0, &page(""))), + (4, Row::Obj(0, 0, &page(""))), + ]); + let mut doc = signed.open(); + doc.update_dict(r(3, 0), |p| { + p.remove(b"Type"); + p.set(b"FT".to_vec(), Object::name("Tx")); + p.set(b"Rotate".to_vec(), Object::Int(180)); + }) + .unwrap(); + assert_refused(&doc, signed.open(), 2); +} + +#[test] +fn resource_container_and_drawn_alias() { + let signed = File::new().revision(&[ + (1, Row::Obj(0, 0, "<< /Type /Catalog /Pages 2 0 R >>")), + (2, Row::Obj(0, 0, "<< /Type /Pages /Kids [3 0 R] /Count 1 >>")), + (3, Row::Obj(0, 0, &page("/Contents 6 0 R /Resources << /Font 4 0 R /ExtGState << /GS1 4 5 R >> >>"))), + (4, Row::Obj(5, 0, "<< /F1 5 0 R /CA 1 >>")), + (5, Row::Obj(0, 0, "<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica >>")), + (6, Row::Obj(0, 0, "<< /Length 3 >>\nstream\nq Q\nendstream")), + ]); + let mut doc = signed.open(); + doc.update_dict(r(4, 0), |d| d.set(b"FT".to_vec(), Object::name("Tx"))).unwrap(); + assert_refused(&doc, signed.open(), 2); +} + +/// A field signed as `4 5 R` / `4 0 R` (header 5, row 0) that a later revision rewrites. +fn aliased_field(fields: &str, annots: &str) -> File { + File::new().revision(&[ + (1, Row::Obj(0, 0, &format!("<< /Type /Catalog /Pages 2 0 R /AcroForm << /Fields [{fields}] >> >>"))), + (2, Row::Obj(0, 0, "<< /Type /Pages /Kids [3 0 R] /Count 1 >>")), + (3, Row::Obj(0, 0, &page(&format!("/Annots [{annots}]")))), + (4, Row::Obj(5, 0, TEXT_FIELD)), + ]) +} + +fn set_annots(doc: &mut Document, annot: ObjRef) { + doc.update_dict(r(3, 0), |p| p.set(b"Annots".to_vec(), Object::Array(vec![Object::Ref(annot)]))).unwrap(); +} + +#[test] +fn repaired_alias_set_changes_widget_to_comment() { + let signed = aliased_field("4 0 R", "4 0 R"); + let now = signed.clone().revision(&[(4, Row::Obj(7, 0, COMMENT))]); + let mut doc = now.open(); + set_annots(&mut doc, r(4, 7)); + assert_refused(&doc, signed.open(), 3); +} + +#[test] +fn repaired_slot_becomes_disjoint_generation() { + let signed = aliased_field("4 0 R", "4 0 R"); + let now = signed.clone().revision(&[(4, Row::Obj(7, 7, COMMENT))]); + let mut doc = now.open(); + set_annots(&mut doc, r(4, 7)); + assert_refused(&doc, signed.open(), 3); +} + +#[test] +fn hidden_shared_alias_form_fill() { + let signed = aliased_field("4 5 R", "4 0 R"); + let filled = TEXT_FIELD.replace("(Ann)", "(Bob)"); + let now = signed.clone().revision(&[(4, Row::Obj(5, 1, &filled))]); + let mut doc = now.open(); + set_annots(&mut doc, r(4, 1)); + // At P=3 a relisted annotation is permitted whatever it is: only the generations refuse it. + for p in [2, 3] { + assert_refused(&doc, signed.open(), p); + } +} diff --git a/crates/sign/tests/it/main.rs b/crates/sign/tests/it/main.rs index d526b01e7c..791b2b503b 100644 --- a/crates/sign/tests/it/main.rs +++ b/crates/sign/tests/it/main.rs @@ -7,4 +7,5 @@ mod cms; mod crypto; mod encrypted_signing; mod pdf; +mod revision_classification; mod store_keys; diff --git a/crates/sign/tests/it/revision_classification.rs b/crates/sign/tests/it/revision_classification.rs new file mode 100644 index 0000000000..9d36c3f57e --- /dev/null +++ b/crates/sign/tests/it/revision_classification.rs @@ -0,0 +1,158 @@ +//! A certification signature stays valid only while what a reader shows is what was signed: +//! revisions appended by hand that reach back to an older generation, or that replace a signed +//! form field in place, are refused. + +use std::sync::Arc; + +use pdfcraft_cos::{Document, Object}; +use pdfcraft_sign::{Modification, SignOptions, Status, TrustStore, pkcs12, signatures}; + +fn data(name: &str) -> Vec { + std::fs::read(format!("{}/tests/data/{name}", env!("CARGO_MANIFEST_DIR"))).unwrap() +} + +fn open(b: &[u8]) -> Document { + Document::open(Arc::new(b.to_vec())).unwrap() +} + +/// A classic-xref PDF of `objs`, numbered from 1, with object 1 as the catalog. +fn write_pdf(objs: &[&str]) -> Vec { + let mut out = b"%PDF-1.7\n".to_vec(); + let mut offsets = Vec::new(); + for (i, o) in objs.iter().enumerate() { + offsets.push(out.len()); + out.extend_from_slice(format!("{} 0 obj\n{o}\nendobj\n", i + 1).as_bytes()); + } + let xref = out.len(); + out.extend_from_slice(format!("xref\n0 {}\n0000000000 65535 f \n", objs.len() + 1).as_bytes()); + for o in offsets { + out.extend_from_slice(format!("{o:010} 00000 n \n").as_bytes()); + } + out.extend_from_slice(format!("trailer\n<< /Size {} /Root 1 0 R >>\nstartxref\n{xref}\n%%EOF\n", objs.len() + 1).as_bytes()); + out +} + +/// One row of a hand-written revision: `num header obj body` under row generation `row`, or a +/// free row with its next generation. +enum Row<'a> { + Obj { num: u32, header: u16, row: u16, body: &'a str }, + Free { num: u32, next: u16 }, +} + +/// `base` with one more revision of `rows`, written as a classic table pointing back with /Prev. +fn append(base: &[u8], rows: &[Row<'_>]) -> Vec { + let doc = open(base); + let size = doc.trailer().get(b"Size").and_then(Object::as_int).unwrap(); + let root = doc.root().unwrap(); + let tail = String::from_utf8_lossy(&base[base.len().saturating_sub(64)..]).into_owned(); + let prev: usize = tail.rsplit("startxref").next().unwrap().split_whitespace().next().unwrap().parse().unwrap(); + let mut out = base.to_vec(); + let mut table = String::new(); + for row in rows { + match row { + Row::Obj { num, header, row, body } => { + table.push_str(&format!("{num} 1\n{:010} {row:05} n \n", out.len())); + out.extend_from_slice(format!("{num} {header} obj\n{body}\nendobj\n").as_bytes()); + } + Row::Free { num, next } => table.push_str(&format!("{num} 1\n0000000000 {next:05} f \n")), + } + } + let xref = out.len(); + out.extend_from_slice( + format!("xref\n{table}trailer\n<< /Size {size} /Root {} {} R /Prev {prev} >>\nstartxref\n{xref}\n%%EOF\n", root.num, root.generation) + .as_bytes(), + ); + out +} + +fn opts(certify: u8) -> SignOptions { + SignOptions { + page: 0, + rect: Some([20.0, 100.0, 220.0, 150.0]), + date: "D:20261002120000+01'00'".into(), + certify: Some(certify), + ..SignOptions::default() + } +} + +/// The certification signature of `bytes`, which must be refused. +fn assert_refused(bytes: &[u8]) { + let id = pkcs12::open(&data("rsa-aes.p12"), "test").unwrap(); + let trust = TrustStore { certs: vec![id.certificate.clone()] }; + let s = signatures(&open(bytes), bytes, &trust).into_iter().find(|s| s.signed).unwrap(); + assert!(matches!(s.modification, Modification::Disallowed(_)) && s.status != Status::Valid, "main returns {:?}, {:?}", s.status, s.modification); +} + +fn certify(unsigned: &[u8], p: u8) -> Vec { + let id = pkcs12::open(&data("rsa-aes.p12"), "test").unwrap(); + pdfcraft_sign::sign(&open(unsigned), &id, &opts(p)).unwrap() +} + +const PAGE2: &str = "<< /Type /Page /Parent 2 0 R /MediaBox [0 0 300 300] /Contents 7 0 R /Resources << /Font << /F1 6 0 R >> >> >>"; + +/// Revision 2 rotates page 2 (`4 0 obj`); revision 3 writes the signed page back as `4 1 obj`. +/// The page tree still says `4 0 R`. +fn intermediate_generation_cover(p: u8) { + let unsigned = write_pdf(&[ + "<< /Type /Catalog /Pages 2 0 R >>", + "<< /Type /Pages /Kids [3 0 R 4 0 R] /Count 2 >>", + "<< /Type /Page /Parent 2 0 R /MediaBox [0 0 300 300] /Contents 5 0 R /Resources << /Font << /F1 6 0 R >> >> >>", + PAGE2, + "<< /Length 41 >>\nstream\nBT /F1 14 Tf 20 250 Td (Page one) Tj ET\nendstream", + "<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica >>", + "<< /Length 41 >>\nstream\nBT /F1 14 Tf 20 250 Td (Page two) Tj ET\nendstream", + ]); + let signed = certify(&unsigned, p); + let rotated = PAGE2.replace(" >> >> >>", " >> >> /Rotate 180 >>"); + let two = append(&signed, &[Row::Obj { num: 4, header: 0, row: 0, body: &rotated }]); + let three = append(&two, &[Row::Obj { num: 4, header: 1, row: 1, body: PAGE2 }]); + let doc = open(&three); + assert!(doc.xref_oddities().is_empty()); + assert_eq!(doc.revisions().len(), open(&signed).revisions().len() + 2); + assert_eq!(doc.get(pdfcraft_cos::ObjRef { num: 4, generation: 1 }), open(&signed).get(pdfcraft_cos::ObjRef { num: 4, generation: 0 })); + assert_refused(&three); +} + +#[test] +fn intermediate_generation_cover_p1() { + intermediate_generation_cover(1); +} + +#[test] +fn intermediate_generation_cover_p2() { + intermediate_generation_cover(2); +} + +#[test] +fn intermediate_generation_cover_p3() { + intermediate_generation_cover(3); +} + +const COMMENT: &str = "<< /Type /Annot /Subtype /Text /Rect [20 200 40 220] /Contents (note) >>"; + +/// A page with text field `4 0 R`, listed in /Fields and in the page's inline /Annots, certified +/// allowing comments. +fn certified_form() -> Vec { + certify( + &write_pdf(&[ + "<< /Type /Catalog /Pages 2 0 R /AcroForm << /Fields [4 0 R] >> >>", + "<< /Type /Pages /Kids [3 0 R] /Count 1 >>", + "<< /Type /Page /Parent 2 0 R /MediaBox [0 0 300 300] /Annots [4 0 R] >>", + "<< /Type /Annot /Subtype /Widget /FT /Tx /T (Name) /V (Ann) /Rect [20 200 120 220] /P 3 0 R >>", + ]), + 3, + ) +} + +#[test] +fn widget_replaced_in_place_by_comment() { + let signed = certified_form(); + assert_refused(&append(&signed, &[Row::Obj { num: 4, header: 0, row: 0, body: COMMENT }])); +} + +#[test] +fn widget_freed_then_reused_as_comment() { + let signed = certified_form(); + let freed = append(&signed, &[Row::Free { num: 4, next: 1 }]); + assert_refused(&append(&freed, &[Row::Obj { num: 4, header: 0, row: 0, body: COMMENT }])); +} From de344a4bb1dd02bad3a9cffd13cf69a5ac73292b Mon Sep 17 00:00:00 2001 From: Martin Ball Date: Thu, 5 Nov 2026 22:44:26 +0000 Subject: [PATCH 2/2] sign: compare annotation lists by what resolves, within a bound An annotation list is compared by the entries that resolve in each revision, and an unchanged list is no change. Expanding annotation lists is bounded per classification. A damaged slot rewritten with fewer accepted generations is the same slot when nothing signed refers to a generation it gave up. Co-Authored-By: Claude Opus 5.5 --- crates/sign/src/pdf/changes.rs | 142 +++++++++++++++--- .../src/pdf/revision_classification_tests.rs | 77 ++++++++++ 2 files changed, 196 insertions(+), 23 deletions(-) diff --git a/crates/sign/src/pdf/changes.rs b/crates/sign/src/pdf/changes.rs index 81467542e6..ef4b8868c6 100644 --- a/crates/sign/src/pdf/changes.rs +++ b/crates/sign/src/pdf/changes.rs @@ -1,5 +1,11 @@ use super::types::{Modification, kind}; use super::{Arc, Dict, Document, HashMap, HashSet, ObjRef, Object}; +use std::cell::{Cell, OnceCell}; + +/// The most annotation-list entries one classification expands. Pages sharing one long /Annots +/// array, or many pages rewritten to list one, would otherwise cost pages × entries; past this +/// the lists are not compared, and the change is refused. +const MAX_ANNOT_ENTRIES: usize = 1 << 22; /// What later revisions changed, classified as Acrobat reports it, under DocMDP `p` (or none: /// an approval signature permits form fill, comments and further signatures). @@ -47,13 +53,31 @@ pub(super) fn classify_changes(doc: &Document, old: Option, p: Option< .collect(); // The annotations the signed pages listed. One of them listed again elsewhere was moved or // copied there, not added. - let placed_annots: HashSet = - signed_page_refs.iter().flat_map(|pg| refs(&old, old.get(*pg).as_dict().and_then(|d| d.get(b"Annots")))).map(|r| r.num).collect(); + let annot_budget = Cell::new(MAX_ANNOT_ENTRIES); + let (mut placed_annots, mut expanded, mut annots_unbounded) = (HashSet::new(), HashSet::new(), false); + for pg in &signed_page_refs { + let page = old.get(*pg); + let Some(list) = page.as_dict().and_then(|d| d.get(b"Annots")) else { continue }; + // Pages sharing one array object list the same annotations. + if list.as_ref().is_some_and(|r| !expanded.insert(r.num)) { + continue; + } + let Some(found) = annot_refs(&old, Some(list), &annot_budget) else { + annots_unbounded = true; + break; + }; + placed_annots.extend(found.into_iter().map(|r| r.num)); + } // What each signed page draws from, by the role it held when signed (#1787). let signed_resources = signed_page_resources(&old, &signed_page_refs); let old_info = old.trailer().get(b"Info").and_then(Object::as_ref).map(|r| r.num); let signed_role = |num: u32| old_content.contains(&num) || signed_pages.contains(&num) || signed_resources.contains_key(&num); let (mut allowed, mut disallowed): (Vec<&'static str>, Vec<&'static str>) = (Vec::new(), Vec::new()); + if annots_unbounded { + disallowed.push(kind::OTHER); + } + // Every reference the signed revision makes, read only when a slot's generations narrowed. + let signed_references = OnceCell::new(); let dss = dss_parts(doc, &old); // Stored at the same place in both (and not edited since): the same bytes, unchanged. let same_place = |num: u32| -> bool { @@ -94,6 +118,7 @@ pub(super) fn classify_changes(doc: &Document, old: Option, p: Option< signed_pages: &signed_pages, signed_annots: &signed_annots, placed_annots: &placed_annots, + annot_budget: &annot_budget, signed_resources: &signed_resources, old_info, dss: &dss, @@ -106,9 +131,19 @@ pub(super) fn classify_changes(doc: &Document, old: Option, p: Option< // it accepts others now, those references may reach nothing here while another reader // follows them back through /Prev to an intermediate revision, or to another alias, and // shows what that holds: the object compared below is not provably what is displayed. - // No writer changes a signed object's generation. + // No writer changes a signed object's generation. A damaged slot rewritten cleanly + // accepts fewer: that is the same slot when nothing signed refers to it by a generation + // it gave up. let signed_generations = old.accepted_generations(num); - if !signed_generations.is_empty() && signed_generations != doc.accepted_generations(num) { + let generations = doc.accepted_generations(num); + let narrowed_unreferenced = || { + !generations.is_empty() + && generations.iter().all(|g| signed_generations.contains(g)) + && signed_references.get_or_init(|| references(&old)).as_ref().is_some_and(|found: &HashSet| { + signed_generations.iter().filter(|g| !generations.contains(g)).all(|g| !found.contains(&ObjRef { num, generation: *g })) + }) + }; + if !signed_generations.is_empty() && signed_generations != generations && !narrowed_unreferenced() { let what = if signed_role(num) { kind::PAGE_CONTENT } else { kind::OTHER }; if !disallowed.contains(&what) { disallowed.push(what); @@ -174,6 +209,7 @@ struct ChangedObjectClassifier<'a> { signed_pages: &'a HashSet, signed_annots: &'a HashSet, placed_annots: &'a HashSet, + annot_budget: &'a Cell, signed_resources: &'a HashMap, old_info: Option, dss: &'a HashSet, @@ -188,8 +224,8 @@ impl ChangedObjectClassifier<'_> { return Some(kind::SECURITY_STORE); } match new { - Object::Array(now) if self.is_signed_annotation_array(r, before) => Some(self.annotation_array_change(now, before)), - Object::Dict(dict) => Some(self.dictionary_change(r, dict, before)), + Object::Array(_) if self.is_signed_annotation_array(r, before) => self.annotation_array_change(new, before), + Object::Dict(dict) => self.dictionary_change(r, dict, before), Object::Stream(stream) => self.stream_change(r, stream, before), _ if before.is_none() => None, _ => Some(kind::OTHER), @@ -213,13 +249,21 @@ impl ChangedObjectClassifier<'_> { self.signed_annots.contains(&r.num) && before.is_some_and(|object| object.as_array().is_some()) } - fn annotation_array_change(&self, now: &[Object], before: Option<&Object>) -> &'static str { - let was: Vec = before.and_then(Object::as_array).map(|items| items.iter().filter_map(Object::as_ref).collect()).unwrap_or_default(); - self.annots_change(&was, &now.iter().filter_map(Object::as_ref).collect::>()) + fn annotation_array_change(&self, now: &Object, before: Option<&Object>) -> Option<&'static str> { + match (annot_refs(self.old, before, self.annot_budget), annot_refs(self.doc, Some(now), self.annot_budget)) { + (Some(was), Some(now)) => self.annots_change(&was, &now), + _ => Some(kind::OTHER), + } } - /// A signed page's annotations going from `was` to `now`, compared by object number. - fn annots_change(&self, was: &[ObjRef], now: &[ObjRef]) -> &'static str { + /// A signed page's annotations going from `was` to `now`, compared by object number. Only an + /// entry that resolves is on the page: one rewritten to a generation its slot does not accept + /// draws nothing, whatever its number. `None` when the page shows the same annotations in + /// the same order. + fn annots_change(&self, was: &[ObjRef], now: &[ObjRef]) -> Option<&'static str> { + let live = + |doc: &Document, list: &[ObjRef]| -> Vec { list.iter().filter(|r| !matches!(*doc.get(**r), Object::Null)).copied().collect() }; + let (was, now) = (live(self.old, was), live(self.doc, now)); let had: HashSet = was.iter().map(|r| r.num).collect(); let kept: HashSet = now.iter().map(|r| r.num).collect(); let removed: Vec = had.iter().filter(|num| !kept.contains(num)).copied().collect(); @@ -227,27 +271,31 @@ impl ChangedObjectClassifier<'_> { let was_widget = |num: u32| self.old.try_get(num).ok().is_some_and(|o| o.as_dict().is_some_and(|d| d.name(b"Subtype") == Some(b"Widget"))); // A signed annotation listed on another page was moved or copied there. let moved: Vec = added.iter().map(|r| r.num).filter(|num| self.placed_annots.contains(num)).collect(); + // The annotations kept, in their new order: another order draws another one on top. + let reordered = !now.iter().map(|r| r.num).filter(|num| had.contains(num)).eq(was.iter().map(|r| r.num)); if removed.iter().chain(&moved).any(|num| was_widget(*num)) { // A form field or signature widget taken off its page, or onto another: no permission covers it. - kind::OTHER - } else if !removed.is_empty() || !moved.is_empty() { - // Deleting or moving comments is a comment change (permitted at P 3 only). - kind::COMMENTS + Some(kind::OTHER) + } else if !removed.is_empty() || !moved.is_empty() || reordered { + // Deleting, moving or restacking comments is a comment change (permitted at P 3 only). + Some(kind::COMMENTS) + } else if added.is_empty() { + None } else { - added_annots(self.doc, &added) + Some(added_annots(self.doc, &added)) } } - fn dictionary_change(&self, r: ObjRef, dict: &Dict, before: Option<&Object>) -> &'static str { + fn dictionary_change(&self, r: ObjRef, dict: &Dict, before: Option<&Object>) -> Option<&'static str> { let previous = before.and_then(Object::as_dict); if self.signed_pages.contains(&r.num) { // A signed page is judged by what the page shows, whatever its new labels say. self.page_change_kind(dict, previous) } else if previous.is_some() && Some(r.num) == self.old_info { // Only the signed revision's own /Info dictionary is a metadata change. - kind::METADATA + Some(kind::METADATA) } else { - change_kind(self.doc, self.old, dict, previous) + Some(change_kind(self.doc, self.old, dict, previous)) } } @@ -255,11 +303,14 @@ impl ChangedObjectClassifier<'_> { /// rather than by any `/Type` its new version carries. Only its annotations may change (see /// [`Self::annots_change`]); any other difference (content, resources, a relabelled or dropped /// `/Type`) is a page content change. - fn page_change_kind(&self, d: &Dict, before: Option<&Dict>) -> &'static str { + fn page_change_kind(&self, d: &Dict, before: Option<&Dict>) -> Option<&'static str> { match before { - Some(b) if without(d, &[b"Annots"]) != without(b, &[b"Annots"]) => kind::PAGE_CONTENT, - Some(b) => self.annots_change(&refs(self.old, b.get(b"Annots")), &refs(self.doc, d.get(b"Annots"))), - None => kind::PAGE_CONTENT, + Some(b) if without(d, &[b"Annots"]) != without(b, &[b"Annots"]) => Some(kind::PAGE_CONTENT), + Some(b) => match (annot_refs(self.old, b.get(b"Annots"), self.annot_budget), annot_refs(self.doc, d.get(b"Annots"), self.annot_budget)) { + (Some(was), Some(now)) => self.annots_change(&was, &now), + _ => Some(kind::OTHER), + }, + None => Some(kind::PAGE_CONTENT), } } @@ -439,6 +490,51 @@ fn added_annots(doc: &Document, added: &[ObjRef]) -> &'static str { } } +/// The references in the annotation list `o` (inline or indirect), its length spent from +/// `budget`. `None` past the budget. +fn annot_refs(doc: &Document, o: Option<&Object>, budget: &Cell) -> Option> { + let held; + let items = match o { + Some(Object::Ref(_)) => { + held = o.map(|o| doc.resolve(o)); + held.as_deref().and_then(Object::as_array) + } + other => other.and_then(Object::as_array), + }; + let Some(items) = items else { return Some(Vec::new()) }; + budget.set(budget.get().checked_sub(items.len())?); + Some(items.iter().filter_map(Object::as_ref).collect()) +} + +/// Every indirect reference in `doc`'s objects and trailer. `None` (nothing can be shown +/// unreferenced) when an object can't be read, or past a bound on references or nesting. +fn references(doc: &Document) -> Option> { + const MAX_REFERENCES: usize = 1 << 22; + const MAX_DEPTH: u8 = 64; + fn walk(o: &Object, depth: u8, found: &mut HashSet) -> bool { + if depth > MAX_DEPTH { + return false; + } + match o { + Object::Ref(r) => { + found.insert(*r); + found.len() <= MAX_REFERENCES + } + Object::Dict(d) => d.iter().all(|(_, v)| walk(v, depth + 1, found)), + Object::Stream(s) => s.dict.iter().all(|(_, v)| walk(v, depth + 1, found)), + Object::Array(a) => a.iter().all(|v| walk(v, depth + 1, found)), + _ => true, + } + } + let mut found = HashSet::new(); + for num in doc.object_numbers() { + if !walk(&*doc.try_get(num).ok()?, 0, &mut found) { + return None; + } + } + doc.trailer().iter().all(|(_, v)| walk(v, 0, &mut found)).then_some(found) +} + fn refs(doc: &Document, o: Option<&Object>) -> Vec { o.map(|o| doc.resolve(o)).and_then(|a| a.as_array().map(|a| a.iter().filter_map(Object::as_ref).collect())).unwrap_or_default() } diff --git a/crates/sign/src/pdf/revision_classification_tests.rs b/crates/sign/src/pdf/revision_classification_tests.rs index ed338c24de..b5e72fcae0 100644 --- a/crates/sign/src/pdf/revision_classification_tests.rs +++ b/crates/sign/src/pdf/revision_classification_tests.rs @@ -205,3 +205,80 @@ fn hidden_shared_alias_form_fill() { assert_refused(&doc, signed.open(), p); } } + +fn verdict(doc: &Document, old: Document, p: Option) -> Modification { + classify_changes(doc, Some(old), p) +} + +/// A widget whose /Annots entry is rewritten to a generation its slot never had no longer +/// resolves: it is taken off the page, inline or through an indirect array. +#[test] +fn widget_dropped_through_a_dead_generation() { + for (page3, num) in [("/Annots 6 0 R", 6), ("/Annots [5 0 R]", 3)] { + let signed = form(page3); + let body = if num == 6 { "[5 3 R]".to_owned() } else { page("/Annots [5 3 R]") }; + let now = signed.clone().revision(&[(num, Row::Obj(0, 0, &body))]); + for p in [Some(2), Some(3), None] { + let v = verdict(&now.open(), signed.open(), p); + assert!(matches!(v, Modification::Disallowed(_)), "{page3} P={p:?}: {v:?}"); + } + } +} + +/// The same annotations, listed in a new array object, change nothing on the page; listed in +/// another order they change which is drawn on top. +#[test] +fn an_unchanged_annotation_list_is_no_change_and_a_reordered_one_is_a_comment_change() { + let signed = File::new().revision(&[ + (1, Row::Obj(0, 0, "<< /Type /Catalog /Pages 2 0 R >>")), + (2, Row::Obj(0, 0, "<< /Type /Pages /Kids [3 0 R] /Count 1 >>")), + (3, Row::Obj(0, 0, &page("/Annots [4 0 R 5 0 R]"))), + (4, Row::Obj(0, 0, COMMENT)), + (5, Row::Obj(0, 0, COMMENT)), + ]); + let moved = signed.clone().revision(&[(3, Row::Obj(0, 0, &page("/Annots 6 0 R"))), (6, Row::Obj(0, 0, "[4 0 R 5 0 R]"))]); + assert_eq!(verdict(&moved.open(), signed.open(), Some(2)), Modification::Allowed(Vec::new())); + let reordered = signed.clone().revision(&[(3, Row::Obj(0, 0, &page("/Annots [5 0 R 4 0 R]")))]); + assert_eq!(verdict(&reordered.open(), signed.open(), Some(2)), Modification::Disallowed(vec![super::kind::COMMENTS])); +} + +/// Many signed pages sharing one long /Annots array, all rewritten to list it through a new +/// object, are judged in bounded time. +#[test] +fn shared_annotation_arrays_are_judged_in_bounded_time() { + const PAGES: u32 = 30_000; + let first_page = 10; + let kids: String = (0..PAGES).map(|i| format!("{} 0 R ", first_page + i)).collect(); + let annots: String = (0..PAGES).map(|i| format!("{} 0 R ", first_page + i)).collect(); + let mut rows = vec![ + (1, "<< /Type /Catalog /Pages 2 0 R >>".to_owned()), + (2, format!("<< /Type /Pages /Kids [{kids}] /Count {PAGES} >>")), + (3, format!("[{annots}]")), + ]; + rows.extend((0..PAGES).map(|i| (first_page + i, page("/Annots 3 0 R")))); + let rows: Vec<(u32, Row<'_>)> = rows.iter().map(|(n, b)| (*n, Row::Obj(0, 0, b))).collect(); + let signed = File::new().revision(&rows); + let repointed: Vec = (0..PAGES).map(|_| page("/Annots 4 0 R")).collect(); + let mut later: Vec<(u32, Row<'_>)> = repointed.iter().enumerate().map(|(i, b)| (first_page + i as u32, Row::Obj(0, 0, b))).collect(); + let copy = format!("[{annots}]"); + later.push((4, Row::Obj(0, 0, ©))); + let now = signed.clone().revision(&later); + let (doc, old) = (now.open(), signed.open()); + let started = std::time::Instant::now(); + let _ = verdict(&doc, old, Some(3)); + assert!(started.elapsed() < std::time::Duration::from_secs(20), "took {:?}", started.elapsed()); +} + +/// A damaged signed slot (header 5, row 0) rewritten cleanly as `4 0 obj` keeps the generation +/// every signed reference uses: a form fill. Had anything signed referred to it as `4 5 R`, that +/// reference would no longer resolve, and the change is refused. +#[test] +fn a_repaired_slot_rewritten_cleanly_is_judged_by_its_references() { + let filled = TEXT_FIELD.replace("(Ann)", "(Bob)"); + let signed = aliased_field("4 0 R", "4 0 R"); + let now = signed.clone().revision(&[(4, Row::Obj(0, 0, &filled))]); + assert_eq!(verdict(&now.open(), signed.open(), Some(2)), Modification::Allowed(vec![super::kind::FORM_FILL])); + let signed = aliased_field("4 5 R", "4 0 R"); + let now = signed.clone().revision(&[(4, Row::Obj(0, 0, &filled))]); + assert!(matches!(verdict(&now.open(), signed.open(), Some(2)), Modification::Disallowed(_))); +}