M8: the run budget counts what it missed

A review of fee25bfe4 found three costs the budget didn't count:

- A /JBIG2Globals reference in parameters given directly was fetched
  again for every entry of every stream (2,000 entries, 53 KB: 40 s, two
  thousand units of four million). What a reference leads to is now
  remembered for the run under it, and a chain met part way takes what
  is known from there.
- A /Filter name was kept whatever it was, so a 32 MiB name referred to
  nine times was held nine times and copied for each image (725 MiB from
  2.8 KB). Only a name a decoder knows is kept; any other is no filter,
  and a name longer than any decoder's isn't looked at.
- A decode that failed spent nothing, so a dictionary one byte past its
  limit escaped the budget. A failure now costs as much as it was allowed
  to decode: a dictionary MAX_JBIG2_GLOBALS, an image its allowance.

The README says what the budget counts, and that past it the redaction
is refused.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AmsdeUCdTaD72jkDw2pJKY

crates/redact/README.md

@@ -32,8 +32,10 @@ clear_marks(&mut doc, None)?;                        // remove marks without app
   refuses and changes nothing (`RedactError::SharedJbig2Globals`). `remove_hidden` takes them out
   the same way. Another dictionary holding the same shapes in other bytes isn't found, nor is a copy
   nothing names, and an image that hasn't left keeps its globals with its pixels: one another page
-  still draws, or one something that doesn't draw it still refers to. Each image is decoded once,
-  with no total across the document yet. Codecs redaction doesn't decode (DCT, JPX) are
+  still draws, or one something that doesn't draw it still refers to. Finding what to store again
+  reads each object at most once for each way it is read, and what it fetches, remembers and
+  decodes (a decode that fails counting as much as it was allowed) counts against one budget for
+  the run; past it the redaction is refused (`RedactError::SharedJbig2Budget`). Codecs redaction doesn't decode (DCT, JPX) are
   removed whole (fail-closed), and so is a JBIG2 image whose page isn't its size (the renderer
   draws the whole page into the image, so the rows cleared wouldn't be the ones shown) or of more
   than one bit or component, and Group 3 on bytes (`/K` 0 or more, `/EncodedByteAlign`): PdfCraft's

crates/redact/src/globals.rs

@@ -78,12 +78,15 @@ enum How {
 /// chain of them or through an entry, costs it once however many there are.
 pub(crate) struct Run {
     budget: usize,
-    spent: usize,
+    /// What was spent of the budget.
+    pub(crate) spent: usize,
     /// What a reference leads to, as it is read, under every reference on the way.
     reduced: HashMap<(u32, How), Rc<Object>>,
     /// What a reference leads to names, under every reference on the way, with the key of the one
     /// that names what it ends at: aliases and chains find it, and are counted as that one.
     names: HashMap<Key, (Rc<[u32]>, Key)>,
+    /// What a `/JBIG2Globals` reference leads to: it and each reference after it, by its number.
+    chains: HashMap<u32, Rc<[u32]>>,
     /// How many values were read for their names rather than remembered.
     pub(crate) read: usize,
     /// How many objects were fetched.
@@ -92,7 +95,7 @@ pub(crate) struct Run {
 
 impl Run {
     pub(crate) fn new(budget: usize) -> Self {
-        Run { budget, spent: 0, reduced: HashMap::new(), names: HashMap::new(), read: 0, fetched: 0 }
+        Run { budget, spent: 0, reduced: HashMap::new(), names: HashMap::new(), chains: HashMap::new(), read: 0, fetched: 0 }
     }
 
     /// Counts `units` against the budget.
@@ -196,15 +199,36 @@ impl Run {
 
     /// What one parameters entry names as `/JBIG2Globals`, and each reference on the way to it.
     fn entry_names(&mut self, doc: &Document, entry: &Object) -> Result<Vec<u32>, crate::RedactError> {
+        match entry.as_dict().and_then(|d| d.get(b"JBIG2Globals")).and_then(Object::as_ref) {
+            Some(start) => Ok(self.chain(doc, start)?.to_vec()),
+            None => Ok(Vec::new()),
+        }
+    }
+
+    /// `start` and each reference after it, up to the limit, remembered under `start` for the run,
+    /// so entries naming it, directly or not, fetch it once however many there are; a chain met
+    /// part way takes what is known from there.
+    fn chain(&mut self, doc: &Document, start: pdfcraft_cos::ObjRef) -> Result<Rc<[u32]>, crate::RedactError> {
+        if let Some(hit) = self.chains.get(&start.num) {
+            return Ok(hit.clone());
+        }
         let mut found = Vec::new();
-        let Some(mut at) = entry.as_dict().and_then(|d| d.get(b"JBIG2Globals")).and_then(Object::as_ref) else { return Ok(found) };
+        let mut at = start;
         for _ in 0..=MAX_HOPS {
+            if let Some(rest) = self.chains.get(&at.num) {
+                found.extend(rest.iter());
+                break;
+            }
             found.push(at.num);
             match &*self.fetch(doc, at)? {
                 Object::Ref(next) => at = *next,
                 _ => break,
             }
         }
+        found.truncate(MAX_HOPS + 1);
+        let found: Rc<[u32]> = found.into();
+        self.spend(found.len())?;
+        self.chains.insert(start.num, found.clone());
         Ok(found)
     }
 
@@ -214,13 +238,18 @@ impl Run {
         Ok((*self.follow(doc, v, How::Number, read)?).clone())
     }
 
-    /// A `/Filter` through any references, as a decoder reads it: a name, or an array of names, each
-    /// entry through references too. Anything else is null, a chain longer than any decoder takes
-    /// (`MAX_CHAIN`, and a `/Crypt`) included, so a stream that names it is read as one with no
-    /// filter.
+    /// A `/Filter` through any references, as a decoder reads it: a name a decoder knows, or an array
+    /// of them, each entry through references too. Anything else is null, a chain longer than any
+    /// decoder takes (`MAX_CHAIN`, and a `/Crypt`) included, so a stream that names it is read as one
+    /// with no filter.
     fn filter(&mut self, doc: &Document, v: &Object) -> Result<Object, crate::RedactError> {
+        // A name a decoder knows; any other is no filter, and isn't copied, however long.
         fn name(_: &mut Run, _: &Document, o: &Object) -> Result<Object, crate::RedactError> {
-            Ok(if matches!(o, Object::Name(_)) { o.clone() } else { Object::Null })
+            let known = |n: &[u8]| n.len() <= 16 && !matches!(pdfcraft_filters::Filter::from_name(n), pdfcraft_filters::Filter::Unknown(_));
+            Ok(match o {
+                Object::Name(n) if known(n) => o.clone(),
+                _ => Object::Null,
+            })
         }
         let read = |run: &mut Self, doc: &Document, o: &Object| match o {
             Object::Array(a) if a.len() > pdfcraft_filters::MAX_CHAIN + 1 => Ok(Object::Null),
@@ -349,7 +378,7 @@ impl Run {
 
 /// What dictionary `num` holds, to know a copy of it by: a file put together from pages split apart
 /// has one per page, each byte for byte the others.
-fn content(run: &mut Run, doc: &Document, num: u32) -> Result<Option<u64>, crate::RedactError> {
+pub(crate) fn content(run: &mut Run, doc: &Document, num: u32) -> Result<Option<u64>, crate::RedactError> {
     let Ok(object) = doc.try_get(num) else { return Ok(None) };
     let Object::Stream(s) = &*object else { return Ok(None) };
     let readable = run.readable(doc, s)?;
@@ -360,7 +389,11 @@ fn content(run: &mut Run, doc: &Document, num: u32) -> Result<Option<u64>, crate
             run.decoded(data.len())?;
             data.hash(&mut hash);
         }
-        Err(_) => s.raw[..].hash(&mut hash),
+        // What it took to find out it can't be read: as much as it was allowed to decode.
+        Err(_) => {
+            run.decoded(MAX_JBIG2_GLOBALS)?;
+            s.raw[..].hash(&mut hash);
+        }
     }
     Ok(Some(hash.finish()))
 }
@@ -371,7 +404,7 @@ fn content(run: &mut Run, doc: &Document, num: u32) -> Result<Option<u64>, crate
 /// or known to have more than one component, which JBIG2 is drawn as one of whatever the dictionary
 /// says; for one whose data is in another file (`/F`), which readers draw instead of what is here;
 /// and for one that can't be decoded.
-fn without_dictionary(run: &mut Run, doc: &Document, s: &Stream) -> Result<Option<Stream>, crate::RedactError> {
+pub(crate) fn without_dictionary(run: &mut Run, doc: &Document, s: &Stream) -> Result<Option<Stream>, crate::RedactError> {
     if run.elsewhere(doc, s)? {
         return Ok(None);
     }
@@ -396,7 +429,11 @@ fn without_dictionary(run: &mut Run, doc: &Document, s: &Stream) -> Result<Optio
     else {
         return Ok(None);
     };
-    let Ok(data) = doc.decoded_image_within(&readable, allowance) else { return Ok(None) };
+    let Ok(data) = doc.decoded_image_within(&readable, allowance) else {
+        // What it took to find out it can't be read: as much as it was allowed to decode.
+        run.decoded(allowance)?;
+        return Ok(None);
+    };
     run.decoded(data.len())?;
     let mut dict = readable.dict.clone();
     // `/FFilter` and `/FDecodeParms` too: with no data in another file they mean nothing, but

crates/redact/src/tests.rs

@@ -1819,6 +1819,10 @@ fn what_is_made_direct_is_small_however_often_a_large_object_is_named() {
         globals.into_bytes(),
         stream(&long, b""),
         stream(&format!("/Filter [{}]", "/FlateDecode ".repeat(9)), b""),
+        // A name no decoder knows, long or short, is no filter, and isn't copied however often.
+        format!("/{}", "A".repeat(2000)).into_bytes(),
+        format!("[{} /Foo]", " 9 0 R".repeat(8)).into_bytes(),
+        stream("/Filter 10 0 R", b""),
     ]);
     let Object::Stream(s) = &*doc.get(ObjRef::new(5, 0)) else { panic!("object 5 is the stream") };
     let made = crate::globals::Run::new(crate::globals::RUN_BUDGET).readable(&doc, s).unwrap();
@@ -1837,6 +1841,65 @@ fn what_is_made_direct_is_small_however_often_a_large_object_is_named() {
     let Object::Stream(s) = &*doc.get(ObjRef::new(8, 0)) else { panic!("object 8 is the stream") };
     let made = crate::globals::Run::new(crate::globals::RUN_BUDGET).readable(&doc, s).unwrap();
     assert_eq!(made.dict.get(b"Filter").and_then(Object::as_array).map(Vec::len), Some(9));
+    let Object::Stream(s) = &*doc.get(ObjRef::new(11, 0)) else { panic!("object 11 is the stream") };
+    let made = crate::globals::Run::new(crate::globals::RUN_BUDGET).readable(&doc, s).unwrap();
+    assert_eq!(made.dict.get(b"Filter"), Some(&Object::Array(vec![Object::Null; 9])));
+}
+
+/// A `/JBIG2Globals` reference in parameters given directly is fetched once a run, however many
+/// entries and streams name it, and what it leads to is held once.
+#[test]
+fn a_dictionary_named_in_direct_entries_is_fetched_once() {
+    let entries = " << /JBIG2Globals 2 0 R >>".repeat(9);
+    let mut objects = vec![b"<< /Type /Catalog >>".to_vec(), stream("", b"symbols")];
+    objects.extend((0..50).map(|_| stream(&format!("/Filter /JBIG2Decode /DecodeParms [{entries}]"), b"")));
+    let doc = pdf(objects);
+    let mut run = crate::globals::Run::new(crate::globals::RUN_BUDGET);
+    for n in 3..53 {
+        let Object::Stream(s) = &*doc.get(ObjRef::new(n, 0)) else { panic!("object {n} is a stream") };
+        let lists = run.names_of(&doc, s).unwrap();
+        assert_eq!(lists.iter().map(|(list, _)| list.to_vec()).collect::<Vec<_>>(), [vec![2]]);
+    }
+    assert_eq!((run.fetched, run.spent), (1, 2), "one fetch, and the one number held");
+}
+
+/// A `/JBIG2Globals` reference leading through a chain names each reference on the way, up to the
+/// limit of 33, whether the chain is met first part way or from its start.
+#[test]
+fn a_chain_of_references_is_named_to_the_limit() {
+    // Objects 2 to 41 each refer to the next, and 42 is the dictionary; 43 names the chain from 20,
+    // and 44 from its start.
+    let mut objects = vec![b"<< /Type /Catalog >>".to_vec()];
+    objects.extend((3..=42).map(|n| format!("{n} 0 R").into_bytes()));
+    objects.push(stream("", b"symbols"));
+    objects.push(stream("/Filter /JBIG2Decode /DecodeParms << /JBIG2Globals 20 0 R >>", b""));
+    objects.push(stream("/Filter /JBIG2Decode /DecodeParms << /JBIG2Globals 2 0 R >>", b""));
+    let doc = pdf(objects);
+    let mut run = crate::globals::Run::new(crate::globals::RUN_BUDGET);
+    let named = |run: &mut crate::globals::Run, n: u32| {
+        let Object::Stream(s) = &*doc.get(ObjRef::new(n, 0)) else { panic!("object {n} is a stream") };
+        run.names_of(&doc, s).unwrap().iter().flat_map(|(list, _)| list.to_vec()).collect::<Vec<_>>()
+    };
+    assert_eq!(named(&mut run, 43), (20..=42).collect::<Vec<u32>>());
+    assert_eq!(named(&mut run, 44), (2..=34).collect::<Vec<u32>>());
+}
+
+/// What can't be decoded is charged as much as it was allowed to decode, so going past a limit
+/// costs as much as reaching it: a dictionary `MAX_JBIG2_GLOBALS` (here one behind a filter that
+/// isn't decoded here), an image its allowance.
+#[test]
+fn a_decode_that_fails_is_charged_what_it_was_allowed() {
+    let image = "/Type /XObject /Subtype /Image /Width 8 /Height 8 /BitsPerComponent 1 /Filter /JBIG2Decode";
+    let doc = pdf(vec![b"<< /Type /Catalog >>".to_vec(), stream("/Filter /DCTDecode", b"not a JPEG"), stream(image, b"not JBIG2")]);
+    let units = pdfcraft_cos::MAX_JBIG2_GLOBALS / 4096 + 1;
+    let run = |budget| crate::globals::Run::new(budget);
+    assert!(crate::globals::content(&mut run(units), &doc, 2).is_ok());
+    assert_eq!(crate::globals::content(&mut run(units - 1), &doc, 2), Err(RedactError::SharedJbig2Budget));
+    // The image's allowance: (one byte a row of 8, and a byte a row) × 3 a byte, and 4096: 4144
+    // bytes, two units.
+    let Object::Stream(s) = &*doc.get(ObjRef::new(3, 0)) else { panic!("object 3 is the image") };
+    assert!(matches!(crate::globals::without_dictionary(&mut run(2), &doc, s), Ok(None)));
+    assert!(matches!(crate::globals::without_dictionary(&mut run(1), &doc, s), Err(RedactError::SharedJbig2Budget)));
 }
 
 /// What streams share by reference is read once a run, however many share it and however they