fix(sign): a signed page's resources are page content whatever keys a later revision writes (#1787) (#2115)

A rewritten resources dictionary, font, XObject or graphics state of a
signed page changes what the page shows. It was judged by the keys its
new version carried, so adding /FT or /Subtype /Widget made it a
permitted form fill. The roles now come from the signed revision's own
object graph, walked from each page's resources with bounds; a category
dictionary that only gained a name is judged as before.


Claude-Session: https://claude.ai/code/session_01RUNQSPVXx3dDBabRZHAnjz

Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
Co-authored-by: Brandon Thomas <echelon@gmail.com>
This commit is contained in:
mtlcl410
2026-10-15 02:45:35 -04:00
committed by GitHub
co-authored by Claude Fable 5.1 Brandon Thomas
parent a7fe7a20c4
commit 716a957186
4 changed files with 245 additions and 15 deletions
+1
View File
@@ -175,6 +175,7 @@ Ranked; detail and the full milestone table in [docs/roadmap.md](docs/roadmap.md
- **2026-10-14 (M3, Open in Full Screen mode honoured):** a document with `/PageMode /FullScreen` now asks (Full Screen / Not now) before the app enters full screen, as Acrobat does; before, the setting was written and read but never acted on when opening.
- **2026-10-14 (M3, control-click candidate):** Coordinate clicks retain hover warmup and pair press/release in one input pass so slow frames do not turn a click into a hold. Text multi-click selection accepts the actual press event when egui has already cleared its origin. Regression cases cover slow-frame button activation, disabled controls and coalesced text selection; estimates unchanged.
- **2026-10-14 (community, M7, #128 Hebrew in Add text):** Add text and `page_add_text` draw Hebrew with the craft-fonts Hebr face (Noto Sans Hebrew), through the same shaped path as Arabic and Thai: right-to-left display order with numbers and Latin in place, points and cantillation kept on their letter as one Type 3 glyph, ToUnicode and per-line ActualText so extraction, search and the editor get typing order. Builds without the face refuse Hebrew rather than write `?`. Regular style only. Estimates unchanged.
- **2026-10-14 (M9, #1787):** Changes after signing are judged by the role an object held when signed for a page's resources too: a rewritten font, XObject, graphics state or resources dictionary is page content however its new version is labelled, so a `/FT` or `/Subtype /Widget` added by the attacker no longer reads as a permitted form fill. A category dictionary that only gained a name is judged as before.
- **2026-10-13 (M3, native-opening candidate):** Chosen tools survive preparation, conversion and password retries; startup intake waits for pending preparation. Regression tests cover completion, cancellation and document identity. Estimates unchanged.
- **2026-10-13 (community, M3, #531 touchpad momentum):** On Wayland a two-finger swipe keeps gliding after the fingers lift and slows to a stop, measured from the gesture's last 0.1 s and decaying with a 0.35 s time constant through the #759 1:1 path. Any wheel input, click, key, zoom or edge ends it (fingers resting on the touchpad don't yet: the vendored winit binds no Wayland hold gestures); single-page paging, macOS (which sends its own momentum), the web, and Animation effects Off never glide. Unit tests for the physics and two headless view tests. Estimates unchanged.
- **2026-10-13 (M3, native-open admission):** Later prepared replies stay queued while a modal or native picker owns the foreground. Picker completion, cancellation and abandonment wake retained opens. Regressions cover recipient ownership, exactly-once admission, source/history preservation and one-shot repaint requests. Estimates unchanged.
+101 -4
View File
@@ -1205,6 +1205,8 @@ 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();
// 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 (mut allowed, mut disallowed): (Vec<&'static str>, Vec<&'static str>) = (Vec::new(), Vec::new());
let dss = dss_parts(doc, &old);
@@ -1250,10 +1252,23 @@ fn classify_changes(doc: &Document, old: Option<Document>, p: Option<u8>) -> Mod
}
continue;
}
let what = match &*new {
// A resource of a signed page is page content whatever keys its new version carries
// (#1787): a /FT or /Subtype /Widget written by whoever rewrote a font, an XObject or a
// graphics state does not make that a form fill. A container (the resources dictionary,
// a category dictionary) that kept every entry it had and only gained names changes
// nothing the signed content draws, and is judged as before.
let kept_entries = |d: &Dict| before.as_deref().and_then(Object::as_dict).is_some_and(|b| b.iter().all(|(k, v)| d.get(k) == Some(v)));
let resource_kind = match (signed_resources.get(&r), &*new) {
(Some(false), _) => Some(kind::PAGE_CONTENT),
(Some(true), Object::Dict(d)) if kept_entries(d) => None,
(Some(true), _) => Some(kind::PAGE_CONTENT),
(None, _) => None,
};
let what = match (resource_kind, &*new) {
(Some(k), _) => k,
// The Document Security Store's own arrays and dictionaries, when they only grew.
_ if dss.contains(&num) => kind::SECURITY_STORE,
Object::Dict(d) => {
(_, _) if dss.contains(&num) => kind::SECURITY_STORE,
(_, Object::Dict(d)) => {
let was = before.as_deref().and_then(Object::as_dict);
if signed_pages.contains(&r) {
// A signed page is always judged by what the page shows, whatever it is now
@@ -1266,7 +1281,7 @@ fn classify_changes(doc: &Document, old: Option<Document>, p: Option<u8>) -> Mod
change_kind(doc, &old, d, was)
}
}
Object::Stream(s) => {
(_, Object::Stream(s)) => {
// File plumbing is what the stream was when signed: the /Type it has now is
// written by whoever changed it, and the renderer ignores /Type (a page's Form
// XObject relabelled /ObjStm still draws its new content).
@@ -1487,6 +1502,88 @@ fn is_signature_dict(d: &Dict) -> bool {
matches!(d.name(b"Type"), Some(b"Sig" | b"DocTimeStamp" | b"SigRef" | b"TransformParams")) || d.name(b"FT") == Some(b"Sig")
}
/// The objects the signed pages draw from, by reference, with the role each held when signed
/// (#1787): `true` for a container (a page's resources dictionary, or one of its category
/// dictionaries), where a name added later can't be reached by unchanged content; `false` for
/// everything reached from a container's entries (fonts and their files, XObjects and their own
/// resources, graphics states and soft masks, colour spaces, patterns, shadings, optional-content
/// 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<ObjRef>) -> HashMap<ObjRef, bool> {
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<ObjRef>, roles: &mut HashMap<ObjRef, bool>) {
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() {
return;
}
resolved = old.get(*r);
&*resolved
}
other => other,
};
match o {
Object::Dict(d) => d.iter().for_each(|(_, v)| reach(old, v, depth + 1, stop, roles)),
Object::Stream(s) => s.dict.iter().for_each(|(_, v)| reach(old, v, depth + 1, stop, roles)),
Object::Array(a) => a.iter().for_each(|v| reach(old, v, depth + 1, stop, roles)),
_ => {}
}
}
/// A page's `/Resources`, its own or the nearest ancestor's.
fn resources(old: &Document, page: ObjRef) -> Option<Object> {
let mut seen = HashSet::new();
let mut cur = page;
while seen.insert(cur) && seen.len() <= MAX_INHERITANCE {
let d = old.get(cur);
let d = d.as_dict()?;
if let Some(r) = d.get(b"Resources") {
return Some(r.clone());
}
cur = d.get(b"Parent").and_then(Object::as_ref)?;
}
None
}
let mut roles = HashMap::new();
let mut stop = pages.clone();
stop.extend(old.root());
for page in pages {
let Some(res) = resources(old, *page) else { continue };
let container = |o: &Object, roles: &mut HashMap<ObjRef, bool>| -> Arc<Object> {
match o {
Object::Ref(r) => {
roles.entry(*r).or_insert(true);
old.get(*r)
}
other => Arc::new(other.clone()),
}
};
let res = container(&res, &mut roles);
let Some(d) = res.as_dict() else { continue };
for category in CATEGORIES {
let Some(v) = d.get(category) else { continue };
let category = container(v, &mut roles);
if let Some(entries) = category.as_dict() {
entries.iter().for_each(|(_, entry)| reach(old, entry, 0, &stop, &mut roles));
}
}
}
roles
}
/// 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 added annotations are permitted; any other
/// difference (content, resources, a relabelled or dropped `/Type`) is a page content change.
+141 -9
View File
@@ -942,7 +942,7 @@ fn a_dss_entry_does_not_make_the_object_it_names_part_of_the_store() {
#[test]
fn an_existing_dictionary_is_not_a_store_part_because_the_store_names_it() {
// The font dictionary (object 5), rewritten and then listed as the store, its /VRI, a VRI
// entry, or simply labelled /Type /DSS.
// entry, or simply labelled /Type /DSS. The page draws it, so each is page content (#1787).
let rewrite_font = |doc: &mut Document, extra: Option<(&str, Object)>| {
let mut d = dict_of(vec![("Type", Object::name("Font")), ("Subtype", Object::name("Type1")), ("BaseFont", Object::name("Courier"))]);
if let Some((k, v)) = extra {
@@ -950,19 +950,19 @@ fn an_existing_dictionary_is_not_a_store_part_because_the_store_names_it() {
}
doc.set(pdfcraft_cos::ObjRef { num: 5, generation: 0 }, Object::Dict(d));
};
assert_refused_at_every_level(&fixture(), "other changes", |doc| {
assert_refused_at_every_level(&fixture(), "page content", |doc| {
rewrite_font(doc, Some(("Type", Object::name("DSS"))));
});
assert_refused_at_every_level(&fixture(), "other changes", |doc| {
assert_refused_at_every_level(&fixture(), "page content", |doc| {
set_dss(doc, oref(5));
rewrite_font(doc, None);
});
assert_refused_at_every_level(&fixture(), "other changes", |doc| {
assert_refused_at_every_level(&fixture(), "page content", |doc| {
set_dss(doc, Object::Dict(dict_of(vec![("VRI", oref(5))])));
rewrite_font(doc, None);
});
// An existing dictionary with only DSS-looking keys that the signed file never had as a store.
assert_refused_at_every_level(&fixture(), "other changes", |doc| {
assert_refused_at_every_level(&fixture(), "page content", |doc| {
doc.set(
pdfcraft_cos::ObjRef { num: 5, generation: 0 },
Object::Dict(dict_of(vec![("Type", Object::name("Font")), ("Cert", Object::Array(vec![]))])),
@@ -985,7 +985,8 @@ fn an_existing_array_stays_what_it_is_when_the_store_points_at_it() {
#[test]
fn a_dictionary_labelled_metadata_is_not_a_metadata_change() {
// Only the catalog's XMP stream is metadata (a stream, not a dictionary a label was put on).
assert_refused_at_every_level(&fixture(), "other changes", |doc| {
// The dictionary is the page's font, so the rewrite is page content (#1787).
assert_refused_at_every_level(&fixture(), "page content", |doc| {
doc.set(
pdfcraft_cos::ObjRef { num: 5, generation: 0 },
Object::Dict(dict_of(vec![
@@ -1042,9 +1043,10 @@ fn fixture_with_stream_7(catalog_extra: &str, drawn: bool, dict: &str, body: &[u
#[test]
fn a_stream_already_labelled_metadata_when_signed_is_not_metadata_unless_the_catalog_names_it() {
// A Form XObject the page draws, labelled /Type /Metadata before signing (the renderer ignores
// /Type). Rewriting it later changes what the page shows, so it is not a metadata change.
// /Type). Rewriting it later changes what the page shows, so it is not a metadata change
// but page content (#1787).
let base = fixture_with_stream_7("", true, "/Type /Metadata /Subtype /Form /BBox [0 0 300 300]", b"0 0 1 rg 10 10 100 100 re f");
assert_refused_at_every_level(&base, "other changes", |doc| {
assert_refused_at_every_level(&base, "page content", |doc| {
let d = dict_of(vec![
("Type", Object::name("Metadata")),
("Subtype", Object::name("Form")),
@@ -1193,7 +1195,8 @@ fn a_form_xobject_relabelled_as_file_plumbing_is_still_judged() {
});
let s = signatures(&open(&edited), &edited, &trust).into_iter().find(|s| s.signed).unwrap();
assert_eq!(s.status, Status::Invalid, "{certify:?} /{label}: {:?}", s.details);
assert!(matches!(&s.modification, Modification::Disallowed(k) if k.contains(&"other changes")), "{:?}", s.modification);
// The page draws the XObject: its rewrite is page content (#1787).
assert!(matches!(&s.modification, Modification::Disallowed(k) if k.contains(&"page content")), "{:?}", s.modification);
}
}
}
@@ -2067,3 +2070,132 @@ fn a_cross_reference_entry_that_misses_its_object_is_not_an_unchanged_object() {
let s = signatures(&open(&honest), &honest, &trust).into_iter().find(|s| s.signed).unwrap();
assert_eq!(s.status, Status::Valid, "{:?}", s.details);
}
/// `fixture()` with the page's resources indirect: object 7 is the resources dictionary, 8 its
/// `/Font` dictionary, 9 an `/ExtGState` the content uses, and 5 the font as before.
fn fixture_with_indirect_resources() -> Vec<u8> {
let objs: Vec<&[u8]> = vec![
b"<< /Type /Catalog /Pages 2 0 R /AcroForm << /Fields [6 0 R] >> >>",
b"<< /Type /Pages /Kids [3 0 R] /Count 1 >>",
b"<< /Type /Page /Parent 2 0 R /MediaBox [0 0 300 300] /Contents 4 0 R /Resources 7 0 R /Annots [6 0 R] >>",
b"<< /Length 52 >>\nstream\n/GS1 gs BT /F1 14 Tf 20 250 Td (Contract text) Tj ET\nendstream",
b"<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica >>",
b"<< /Type /Annot /Subtype /Widget /FT /Sig /T (Approval) /Rect [150 20 280 70] /P 3 0 R /F 4 >>",
b"<< /Font 8 0 R /ExtGState << /GS1 9 0 R >> >>",
b"<< /F1 5 0 R >>",
b"<< /Type /ExtGState /ca 1 >>",
];
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", i + 1).as_bytes());
out.extend_from_slice(o);
out.extend_from_slice(b"\nendobj\n");
}
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
}
/// A signed page's resources are judged by the role they held when signed, not by the keys a
/// later revision writes into them (#1787): rewriting the page's resources dictionary, a
/// category dictionary, a font or a graphics state changes what the page shows, however the
/// new version is labelled (/FT, /DR, /T + /Kids, /Subtype /Widget, /Type /Annot + /Rect, which
/// otherwise read as form filling or a comment, permitted under P=2 and P=3). A category
/// dictionary that only gained a name changes nothing drawn and is judged as before.
#[test]
fn rewritten_page_resources_are_page_content_whatever_keys_they_carry() {
/// Replace object `num` with a dictionary; each attack runs once per certification level,
/// so the entries are cloned into each.
fn set(num: u32, entries: Vec<(&'static str, Object)>) -> impl Fn(&mut Document) {
move |doc| doc.set(pdfcraft_cos::ObjRef { num, generation: 0 }, Object::Dict(dict_of(entries.clone())))
}
/// A second font, so a name can be made to resolve elsewhere.
fn courier(doc: &mut Document) -> pdfcraft_cos::ObjRef {
doc.add(Object::Dict(dict_of(vec![
("Type", Object::name("Font")),
("Subtype", Object::name("Type1")),
("BaseFont", Object::name("Courier")),
])))
}
// The resources dictionary: another font under the same name, plus /FT.
assert_refused_at_every_level(&fixture_with_indirect_resources(), "page content", |doc| {
let f = courier(doc);
set(
7,
vec![
("Font", Object::Dict(dict_of(vec![("F1", Object::Ref(f))]))),
("ExtGState", Object::Dict(dict_of(vec![("GS1", oref(9))]))),
("FT", Object::name("Tx")),
],
)(doc);
});
// The /Font dictionary: the name now resolves elsewhere, plus /DR.
assert_refused_at_every_level(&fixture_with_indirect_resources(), "page content", |doc| {
let f = courier(doc);
set(8, vec![("F1", Object::Ref(f)), ("DR", Object::Dict(dict_of(vec![])))])(doc);
});
// The graphics state: everything drawn through it becomes invisible, plus /T and /Kids.
assert_refused_at_every_level(
&fixture_with_indirect_resources(),
"page content",
set(
9,
vec![
("Type", Object::name("ExtGState")),
("ca", Object::Real(0.0)),
("T", Object::String(PdfString::literal("x"))),
("Kids", Object::Array(vec![])),
],
),
);
// The font itself, relabelled a widget.
assert_refused_at_every_level(
&fixture_with_indirect_resources(),
"page content",
set(5, vec![("Type", Object::name("Font")), ("BaseFont", Object::name("Courier")), ("Subtype", Object::name("Widget"))]),
);
// The resources dictionary relabelled a comment (permitted under P=3 and by an approval signature).
assert_refused_at_every_level(&fixture_with_indirect_resources(), "page content", |doc| {
let f = courier(doc);
set(
7,
vec![
("Type", Object::name("Annot")),
("Subtype", Object::name("Square")),
("Rect", Object::Array(vec![Object::Int(0), Object::Int(0), Object::Int(10), Object::Int(10)])),
("Font", Object::Dict(dict_of(vec![("F1", Object::Ref(f))]))),
],
)(doc);
});
// The same page resources inline in the page keep being page content through the page itself.
assert_refused_at_every_level(&fixture(), "page content", |doc| {
let f = courier(doc);
doc.update_dict(pdfcraft_cos::ObjRef { num: 3, generation: 0 }, |page| {
page.set(b"Resources".to_vec(), Object::Dict(dict_of(vec![("Font", Object::Dict(dict_of(vec![("F1", Object::Ref(f))])))])));
})
.unwrap();
});
// Positive control: a /Font dictionary that only gained a name changes nothing the signed
// content draws, so it is not page content (it is still an unclassified change, refused as
// "other", as before).
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_with_indirect_resources()), &id, &SignOptions { certify: Some(3), ..opts() }).unwrap();
let grown = edit_after(&signed, |doc| {
let f = courier(doc);
set(8, vec![("F1", oref(5)), ("F2", Object::Ref(f))])(doc);
});
let s = signatures(&open(&grown), &grown, &trust).into_iter().find(|s| s.signed).unwrap();
assert!(
matches!(&s.modification, Modification::Disallowed(k) if !k.contains(&"page content") && k.contains(&"other changes")),
"{:?}",
s.modification
);
}
+2 -2
View File
@@ -6325,8 +6325,8 @@ milestone = "M9"
status = "shipped"
commands = ["sign.panel"]
tools = ["sign_list"]
evidence = ["crates/sign/tests/pdf.rs::later_changes_are_classified_under_the_signature_permissions", "crates/sign/tests/pdf.rs::signing_an_existing_field_and_counter_signing", "crates/engine/src/tests.rs::signing_saving_trusting_and_commenting_afterwards", "crates/sign/tests/pdf.rs::validation_data_and_xmp_added_after_signing_are_not_tampering", "crates/sign/tests/pdf.rs::a_dss_entry_does_not_make_the_object_it_names_part_of_the_store", "crates/sign/tests/pdf.rs::an_existing_dictionary_is_not_a_store_part_because_the_store_names_it", "crates/sign/tests/pdf.rs::an_existing_array_stays_what_it_is_when_the_store_points_at_it", "crates/sign/tests/pdf.rs::a_signed_store_may_grow_but_not_lose_or_swap_entries", "crates/sign/tests/pdf.rs::a_dictionary_labelled_metadata_is_not_a_metadata_change", "crates/sign/tests/pdf.rs::a_stream_already_labelled_metadata_when_signed_is_not_metadata_unless_the_catalog_names_it", "crates/sign/tests/pdf.rs::the_catalogs_own_xmp_stream_may_still_be_rewritten_after_signing"]
notes = "revision diff classified as signing, form fill, comments, metadata, page content or structure. Later revisions that add DSS data (new /Certs, /CRLs, /OCSPs, /VRI objects, or ones of the signed store that only grew) or rewrite the catalog's XMP are permitted changes; a /DSS entry that names other objects, or a /Type /DSS or /Type /Metadata label on an existing dictionary or stream, does not excuse changing them (negative tests at every DocMDP level)"
evidence = ["crates/sign/tests/pdf.rs::later_changes_are_classified_under_the_signature_permissions", "crates/sign/tests/pdf.rs::signing_an_existing_field_and_counter_signing", "crates/engine/src/tests.rs::signing_saving_trusting_and_commenting_afterwards", "crates/sign/tests/pdf.rs::validation_data_and_xmp_added_after_signing_are_not_tampering", "crates/sign/tests/pdf.rs::a_dss_entry_does_not_make_the_object_it_names_part_of_the_store", "crates/sign/tests/pdf.rs::an_existing_dictionary_is_not_a_store_part_because_the_store_names_it", "crates/sign/tests/pdf.rs::an_existing_array_stays_what_it_is_when_the_store_points_at_it", "crates/sign/tests/pdf.rs::a_signed_store_may_grow_but_not_lose_or_swap_entries", "crates/sign/tests/pdf.rs::a_dictionary_labelled_metadata_is_not_a_metadata_change", "crates/sign/tests/pdf.rs::a_stream_already_labelled_metadata_when_signed_is_not_metadata_unless_the_catalog_names_it", "crates/sign/tests/pdf.rs::the_catalogs_own_xmp_stream_may_still_be_rewritten_after_signing", "crates/sign/tests/pdf.rs::rewritten_page_resources_are_page_content_whatever_keys_they_carry"]
notes = "revision diff classified as signing, form fill, comments, metadata, page content or structure. Later revisions that add DSS data (new /Certs, /CRLs, /OCSPs, /VRI objects, or ones of the signed store that only grew) or rewrite the catalog's XMP are permitted changes; a /DSS entry that names other objects, or a /Type /DSS or /Type /Metadata label on an existing dictionary or stream, does not excuse changing them (negative tests at every DocMDP level). A signed page's resources (fonts, XObjects, graphics states and what they reach) are page content by the role they held when signed, whatever keys a later revision writes into them (#1787)"
[[feature]]
id = "sign.view-signed-version"