From fbada004d32d4fbb24638b2fe6eb27c0b9637f49 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 14:38:04 +0000 Subject: [PATCH] fix(sign): treat a changed trailer /Root after signing as not permitted Revision checks compared objects only. A later revision that pointed the trailer at another catalog already present in the signed file changed no object, so the signature stayed valid while the reader showed a different document. A /Root that differs from the signed revision's is now a disallowed change to the document structure. --- crates/sign/src/pdf.rs | 5 +++++ crates/sign/tests/it/pdf.rs | 24 ++++++++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/crates/sign/src/pdf.rs b/crates/sign/src/pdf.rs index 57c9f95fd..50f5d4a19 100644 --- a/crates/sign/src/pdf.rs +++ b/crates/sign/src/pdf.rs @@ -1286,6 +1286,11 @@ fn classify_changes(doc: &Document, old: Option, p: Option) -> Mod disallowed.push(what); } } + // The trailer chooses the catalog. A later revision that points it at another catalog (one + // already in the signed file, so no object changes) shows another document. + if doc.root() != old.root() { + disallowed.push(kind::STRUCTURE); + } for num in doc.object_numbers() { if same_place(num) { continue; diff --git a/crates/sign/tests/it/pdf.rs b/crates/sign/tests/it/pdf.rs index e9db0b570..29f61305f 100644 --- a/crates/sign/tests/it/pdf.rs +++ b/crates/sign/tests/it/pdf.rs @@ -2471,3 +2471,27 @@ fn rewritten_page_resources_are_page_content_whatever_keys_they_carry() { s.modification ); } + +/// A later revision that only points the trailer at another catalog already in the signed file +/// shows a different document while every signed object is unchanged. That is not permitted. +#[test] +fn switching_the_trailer_to_another_signed_catalog_is_not_permitted() { + let base = fixture_with( + b"<< /Type /Catalog /Pages 2 0 R /AcroForm << /Fields [6 0 R] >> >>", + &[ + b"<< /Type /Catalog /Pages 8 0 R /AcroForm << /Fields [6 0 R] >> >>", + b"<< /Type /Pages /Kids [9 0 R] /Count 1 >>", + b"<< /Type /Page /Parent 8 0 R /MediaBox [0 0 300 300] /Contents 10 0 R /Resources << /Font << /F1 5 0 R >> >> /Annots [6 0 R] >>", + b"<< /Length 44 >>\nstream\nBT /F1 14 Tf 20 250 Td (Contract void) Tj ET\nendstream", + ], + ); + let id = pkcs12::open(&data("ec-p256.p12"), "test").unwrap(); + let trust = TrustStore { certs: vec![id.certificate.clone()] }; + let signed = pdfcraft_sign::sign(&open(&base), &id, &SignOptions { field: Some("Approval".into()), rect: None, ..opts() }).unwrap(); + let s = signatures(&open(&signed), &signed, &trust).into_iter().find(|s| s.signed).unwrap(); + assert_eq!(s.status, Status::Valid, "{:?}", s.details); + let switched = edit_after(&signed, |doc| doc.trailer_mut().set(b"Root".to_vec(), oref(7))); + let s = signatures(&open(&switched), &switched, &trust).into_iter().find(|s| s.signed).expect("the signature is still listed"); + assert!(matches!(s.modification, Modification::Disallowed(_)), "{:?}", s.modification); + assert_eq!(s.status, Status::Invalid, "{:?}", s.details); +}