fix(redact): charge repeated name-tree kids to the node budget

The name-tree count cloned each node's /Kids array and skipped repeated
references without charging the budget, so a node listing the same kid
many times cost work proportional to every entry while the budget saw one
step. The walk now borrows the array, charges every entry it does not
descend into, and stops once the budget is spent.
This commit is contained in:
aiongg
2026-10-22 13:28:40 -04:00
committed by Dylan M. Taylor
parent 4d64ca8e2d
commit 241c5512d6
2 changed files with 40 additions and 10 deletions
+20 -10
View File
@@ -92,28 +92,38 @@ const NAME_TREE_NODE_BUDGET: usize = 100_000;
/// Entries of a name tree (`/Names` leaves, `/Kids` recursion). Each indirect node is read once,
/// however many `/Kids` arrays list it, so shared or cyclic kids cost one step per node rather
/// than one per path; depth and a total node budget bound the rest.
/// than one per path. Every `/Kids` entry the walk does not descend into (a repeated reference or
/// a non-dictionary) costs a step as well, so a long `/Kids` array of repeats cannot outrun the
/// budget. Depth and a total step budget bound the rest.
fn name_tree_len(doc: &Document, node: &Dict) -> usize {
let (mut seen, mut visited) = (std::collections::HashSet::new(), 0);
name_tree_len_seen(doc, node, 0, &mut seen, &mut visited)
}
fn name_tree_len_seen(doc: &Document, node: &Dict, depth: usize, seen: &mut std::collections::HashSet<ObjRef>, visited: &mut usize) -> usize {
*visited += 1;
*visited = visited.saturating_add(1);
if depth > 32 || *visited > NAME_TREE_NODE_BUDGET {
return 0;
}
let leaves = node.get(b"Names").and_then(|n| doc.resolve(n).as_array().map(|a| a.len() / 2)).unwrap_or(0);
let kids = node.get(b"Kids").and_then(|k| doc.resolve(k).as_array().cloned()).unwrap_or_default();
let mut n = leaves;
for k in &kids {
if let Object::Ref(r) = k
&& !seen.insert(*r)
{
continue;
let kids = node.get(b"Kids").map(|k| doc.resolve(k));
for k in kids.as_deref().and_then(Object::as_array).map_or(&[][..], Vec::as_slice) {
if *visited > NAME_TREE_NODE_BUDGET {
break;
}
if let Some(kid) = doc.resolve(k).as_dict() {
n = n.saturating_add(name_tree_len_seen(doc, kid, depth + 1, seen, visited));
let held;
let kid = match k {
Object::Ref(r) if !seen.insert(*r) => None,
Object::Ref(r) => {
held = doc.get(*r);
held.as_dict()
}
other => other.as_dict(),
};
match kid {
Some(kid) => n = n.saturating_add(name_tree_len_seen(doc, kid, depth + 1, seen, visited)),
None => *visited = visited.saturating_add(1),
}
}
n
+20
View File
@@ -2671,3 +2671,23 @@ fn owners_nested_too_deeply_are_refused() {
let out = remove_hidden(&mut doc, &[Hidden::LinksActionsScripts]);
assert!(matches!(out, Err(RedactError::TooDeep)), "{out:?}");
}
/// A name tree node that lists the same kid more times than the node budget: each repeat costs a
/// step, so the walk stops at the budget instead of scanning every repeat, and the distinct kid
/// listed after them is never reached.
#[test]
fn repeated_name_tree_kids_count_towards_the_node_budget() {
use crate::sanitize::{Hidden, scan};
let repeats = "6 0 R ".repeat(100_010);
let doc = pdf(vec![
b"<< /Type /Catalog /Pages 2 0 R /Names << /EmbeddedFiles 5 0 R >> >>".to_vec(),
b"<< /Type /Pages /Kids [3 0 R] /Count 1 >>".to_vec(),
b"<< /Type /Page /Parent 2 0 R /MediaBox [0 0 300 300] /Contents 4 0 R >>".to_vec(),
stream("", b"BT ET"),
format!("<< /Kids [{repeats}7 0 R] >>").into_bytes(),
b"<< /Names [(a.txt) << /Type /Filespec /F (a.txt) >>] >>".to_vec(),
b"<< /Names [(b.txt) << /Type /Filespec /F (b.txt) >>] >>".to_vec(),
]);
let counts: std::collections::HashMap<Hidden, usize> = scan(&doc).into_iter().collect();
assert_eq!(counts[&Hidden::Attachments], 1, "the walk went past its node budget to the last kid");
}