From 241c5512d646e9be2964ccba96a69be5ab579bb5 Mon Sep 17 00:00:00 2001
From: aiongg <aiongtaigi@gmail.com>
Date: Thu, 22 Oct 2026 17:04:40 +0000
Subject: [PATCH] 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.
---
 crates/redact/src/sanitize.rs | 30 ++++++++++++++++++++----------
 crates/redact/src/tests.rs    | 20 ++++++++++++++++++++
 2 files changed, 40 insertions(+), 10 deletions(-)

diff --git a/crates/redact/src/sanitize.rs b/crates/redact/src/sanitize.rs
index 7349c433a..5b2918f01 100644
--- a/crates/redact/src/sanitize.rs
+++ b/crates/redact/src/sanitize.rs
@@ -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
diff --git a/crates/redact/src/tests.rs b/crates/redact/src/tests.rs
index 603fede2c..a9902495d 100644
--- a/crates/redact/src/tests.rs
+++ b/crates/redact/src/tests.rs
@@ -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");
+}