Fail closed when the signature field walk is cut short

The signed revision's DocMDP permission was looked up by walking the old
revision's signature fields, but the walk stops at a depth and a
qualified-name budget. When it stopped before the signature that ends the
revision, infos certify was left as the value read from the current
document, so a later /P rewrite could speak for the signed revision.

The walk now reports that it was cut, and the lookup then claims no
permission instead. Standalone-timestamp exclusion takes its values from a
separate walk that builds no names, so the name budget cannot hide one.
This commit is contained in:
Dylan M. Taylor committed 2026-11-08 12:29:57 -05:00
1 parent 3593d529bd
commit f1055f6938
3 files changed
+94 -15

No files matched your search

+54 -7
View File
@@ -207,17 +207,26 @@ const MAX_FIELD_NAME_BYTES: usize = 32 << 20;
/// Every signature field, depth first in `/Fields` order.
pub(super) fn sig_fields(doc: &Document) -> Vec<Field> {
sig_fields_bounded(doc).0
}
/// [`sig_fields`], with whether the walk was cut short by [`MAX_FIELD_DEPTH`] or
/// [`MAX_FIELD_NAME_BYTES`]: the fields it could not reach are then unknown to the caller, which
/// is what lets a permission lookup fail closed instead of guessing.
pub(super) fn sig_fields_bounded(doc: &Document) -> (Vec<Field>, bool) {
let mut out = Vec::new();
let Some(root) = doc.root() else { return out };
let mut cut = false;
let Some(root) = doc.root() else { return (out, cut) };
let Some(af) = doc.get(root).as_dict().and_then(|c| c.get(b"AcroForm").map(|a| doc.resolve(a))).and_then(|a| a.as_dict().cloned()) else {
return out;
return (out, cut);
};
let Some(fields) = af.get(b"Fields").map(|f| doc.resolve(f)).and_then(|f| f.as_array().cloned()) else { return out };
let Some(fields) = af.get(b"Fields").map(|f| doc.resolve(f)).and_then(|f| f.as_array().cloned()) else { return (out, cut) };
let mut seen = HashSet::new();
let mut stack: Vec<(Object, String, Option<Vec<u8>>, usize)> = fields.into_iter().rev().map(|f| (f, String::new(), None, 0)).collect();
let mut name_bytes = 0usize;
while let Some((o, prefix, inherited_ft, depth)) = stack.pop() {
if depth > MAX_FIELD_DEPTH {
cut = true;
continue;
}
if let Some(r) = o.as_ref()
@@ -239,6 +248,7 @@ pub(super) fn sig_fields(doc: &Document) -> Vec<Field> {
// this name, and the clone of it each queued sub-field is about to hold
name_bytes = name_bytes.saturating_add(name.len().saturating_mul(sub.len() + 1));
if name_bytes > MAX_FIELD_NAME_BYTES {
cut = true;
break;
}
for k in sub.into_iter().rev() {
@@ -253,6 +263,44 @@ pub(super) fn sig_fields(doc: &Document) -> Vec<Field> {
out.push(Field { name, r: o.as_ref(), dict: d, widgets: ws });
}
}
(out, cut)
}
/// Every `/V` an AcroForm field holds, however deep: a standalone document timestamp must not be
/// confused with a signature a field already carries. No qualified names are built here, so the
/// name budget that can stop [`sig_fields`] cannot hide a value from it.
pub(super) fn field_values(doc: &Document) -> HashSet<ObjRef> {
let mut out = HashSet::new();
let Some(root) = doc.root() else { return out };
let Some(fields) = doc
.get(root)
.as_dict()
.and_then(|c| c.get(b"AcroForm").map(|a| doc.resolve(a)))
.and_then(|a| a.as_dict().cloned())
.and_then(|a| a.get(b"Fields").map(|f| doc.resolve(f).as_array().cloned()).unwrap_or_default())
else {
return out;
};
let mut seen = HashSet::new();
let mut stack: Vec<(Object, usize)> = fields.into_iter().rev().map(|f| (f, 0)).collect();
while let Some((o, depth)) = stack.pop() {
if depth > MAX_FIELD_DEPTH {
continue;
}
if let Some(r) = o.as_ref()
&& !seen.insert(r)
{
continue;
}
let Some(d) = doc.resolve(&o).as_dict().cloned() else { continue };
if let Some(v) = d.get(b"V").and_then(Object::as_ref) {
out.insert(v);
}
let kids: Vec<Object> = d.get(b"Kids").map(|k| doc.resolve(k)).and_then(|k| k.as_array().cloned()).unwrap_or_default();
for k in kids.into_iter().rev() {
stack.push((k, depth + 1));
}
}
out
}
@@ -283,11 +331,10 @@ pub fn list(doc: &Document, bytes: &[u8], trust: &TrustStore) -> Vec<SignatureIn
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 field_values: HashSet<ObjRef> = HashSet::new();
// Every value a field holds, listed or not: a signature below the listing's bounds still
// owns its `/V`, which is then not a standalone document timestamp.
let field_values = field_values(doc);
for f in sig_fields(doc) {
if let Some(v_ref) = f.dict.get(b"V").and_then(Object::as_ref) {
field_values.insert(v_ref);
}
let widget = f.widgets.first();
let page = widget.and_then(|(r, w)| r.and_then(|r| pages.get(&r).copied()).or_else(|| w.reference(b"P").and_then(|p| page_index(doc, p))));
let rect = widget
+16 -8
View File
@@ -1,5 +1,5 @@
use super::changes::classify_changes;
use super::listing::{DigestCache, list, nums, sig_fields, text};
use super::listing::{DigestCache, list, nums, sig_fields, sig_fields_bounded, text};
use super::types::{Detail, Fill, Modification, Status, TrustStore};
use super::{
Certificate, Dict, DigestAlg, Document, HashSet, ObjRef, Object, SignError, SignatureInfo, SignedData, Time, build_chain, build_chain_noted,
@@ -246,13 +246,21 @@ fn finish_validation(context: SignatureValidation<'_>, verified: VerifiedSignatu
// The permissions are the signed revision's: a later one can rewrite `/P` (#1788). This
// signature is the one whose byte range ends that revision.
let old = cache.revision(doc, bytes, covered);
if let Some(old) = &old
&& let Some(v) = sig_fields(old)
.iter()
.filter_map(|f| f.signature(old))
.find(|v| nums(old, v, b"ByteRange").is_some_and(|b| b.len() == 4 && b[2] + b[3] == covered as f64))
{
info.certify = docmdp(old, &v);
// The permissions are the signed revision's: a later one can rewrite `/P` (#1788).
if let Some(old) = &old {
// When the field walk cannot reach every field, the signature that ends this revision
// is unknown, so no permission is claimed: keeping `docmdp(doc, v)`, read from the
// current revision, would let a later `/P` rewrite speak for the signed one.
info.certify = None;
let (fields, cut) = sig_fields_bounded(old);
if !cut
&& let Some(v) = fields
.iter()
.filter_map(|f| f.signature(old))
.find(|v| nums(old, v, b"ByteRange").is_some_and(|b| b.len() == 4 && b[2] + b[3] == covered as f64))
{
info.certify = docmdp(old, &v);
}
}
classify_changes(doc, old, info.certify)
};
+24
View File
@@ -397,3 +397,27 @@ fn dss_evidence_decodes_each_stream_once_within_its_caps() {
assert!(ocsps.cut);
assert!(DssEvidence::new(&doc, &dss, b"OCSPs").count() <= MAX_DSS_ENTRIES);
}
/// A field tree deeper than the walk follows is reported as cut: a permission lookup that cannot
/// reach every field must see that, so it fails closed instead of trusting a partial list.
#[test]
fn a_field_tree_past_the_walk_depth_is_reported_as_cut() {
let mut doc = fixture(&[STAMP]);
let mut kid = None;
for i in 0..80 {
let mut f = Dict::new();
f.set(b"FT".to_vec(), Object::name("Sig"));
f.set(b"T".to_vec(), Object::String(PdfString::literal(format!("f{i}"))));
if let Some(k) = kid {
f.set(b"Kids".to_vec(), Object::Array(vec![Object::Ref(k)]));
}
let id = doc.add(f);
kid = Some(id);
}
let mut form = Dict::new();
form.set(b"Fields".to_vec(), Object::Array(vec![Object::Ref(kid.unwrap())]));
doc.update_dict(doc.root().unwrap(), |d| d.set(b"AcroForm".to_vec(), Object::Dict(form))).unwrap();
let (fields, cut) = super::listing::sig_fields_bounded(&doc);
assert!(cut, "the walk stopped at its depth limit without saying so");
assert!(fields.len() < 80, "a cut walk lists what it reached: {}", fields.len());
}