From 1ec46732b86c338300c158ac4331f902776184a7 Mon Sep 17 00:00:00 2001 From: Taha-Mahmoodi <85902429+Taha-Mahmoodi@users.noreply.github.com> Date: Sat, 24 Oct 2026 05:17:25 +0430 Subject: [PATCH] PDF import: the pre-open stream scan doesn't let a "stream" inside a literal string hide the real stream after it Review of #3919: the scan raised its walk-back floor at every `>>stream`, so one inside a literal string moved the floor past the `<<` of the cross-reference stream around it, and that stream was never looked at. The backward walk also counted a `>>` inside a string, and missed a comment between `>>` and `stream`. The scan now reads the file forward once, as the reader does: literal strings (nested, escaped), hex strings and comments are skipped, `<<` and `>>` pair on a stack, and a dictionary followed by `stream` is a stream's, whose data is skipped. Because the reader can still start at a `<<` inside skipped text (a cross-reference offset or its repair scan), each such `<<` is read as a dictionary of its own, up to 64 KiB, within a budget of four times the file plus 1 MiB; a crafted file that spends it is refused. The scan stays linear. --- crates/pdf/src/streams.rs | 340 +++++++++++++++++++++++++++++++------- docs/gaps.md | 1 + 2 files changed, 285 insertions(+), 56 deletions(-) diff --git a/crates/pdf/src/streams.rs b/crates/pdf/src/streams.rs index 25f0c1973..4b2086de9 100644 --- a/crates/pdf/src/streams.rs +++ b/crates/pdf/src/streams.rs @@ -269,67 +269,250 @@ fn unsafe_data(stream: &Stream<'_>, inside: bool) -> bool { /// Their decode parameters must be in their dictionary, as the reader can't be asked for others /// yet. Encrypted object streams are looked at once the file is open (their data is ciphered /// here). +/// +/// The file is read once from the start, as the reader reads PDF: literal strings (nested +/// parentheses, `\` escaping), hex strings and comments are skipped, `<<` and `>>` pair up, and a +/// dictionary that `stream` follows (white space and comments between) is a stream's, whose data +/// is skipped. So a `stream` or `>>` in a string or a comment is no keyword (#3919 review). The +/// reader may still start at a `<<` inside what this reading skipped (a cross-reference offset or +/// its repair scan can point there), so each such `<<` is read as a dictionary of its own, within +/// a budget for the whole file: a file that spends it is refused. Linear in the file (#3586). pub(crate) fn unsafe_file_streams(pdf: &[u8]) -> bool { - let find = |from: usize, what: &[u8]| pdf.get(from..).and_then(|rest| rest.windows(what.len()).position(|w| w == what)).map(|at| from + at); - // A walk back through a dictionary stops at `dicts_from`, the end of the last keyword walked - // back from, so the scan reads each byte a bounded number of times (#3586): before, every - // keyword walked back up to 64 KiB through all the dictionaries before it. - let (mut from, mut dicts_from) = (0, 0); - // The first `endstream` at or after the last place it was looked for (`None`: there is none). - let mut endstream: Option> = None; - while let Some(at) = find(from, b"stream") { - from = at + 6; - if at >= 3 && pdf.get(at - 3..at) == Some(b"end") { - continue; - } - // The dictionary before it: back from its `>>` to the `<<` that opens it. - let Some(close) = pdf.get(..at).and_then(|head| head.iter().rposition(|c| !c.is_ascii_whitespace())).map(|i| i + 1) else { continue }; - if close < 2 || pdf.get(close - 2..close) != Some(b">>") { - continue; - } - let floor = close.saturating_sub(1 << 16).max(dicts_from); - dicts_from = from; - let (mut depth, mut open, mut i) = (0usize, None, close); - while i >= floor + 2 { - match pdf.get(i - 2..i) { - Some(b">>") => (depth, i) = (depth + 1, i - 2), - Some(b"<<") => { - depth -= 1; - i -= 2; - if depth == 0 { - open = Some(i); - break; - } + let mut scan = FileScan { pdf, budget: pdf.len().saturating_mul(4).saturating_add(1 << 20), endstream: None }; + scan.read().is_err() +} + +/// A stream the reader would fail on, or a file that spent the scan's budget. +struct Unsafe; + +/// The most bytes a dictionary read from inside a string, comment or stream data may span, as far +/// as the scan looked back from a `stream` keyword before. +const MAX_SKIPPED_DICT: usize = 1 << 16; + +/// The most `<<` the scan keeps open: deeper than the reader nests dictionaries. +const MAX_OPEN: usize = 1024; + +/// PDF white space (ISO 32000-1, 7.2.2). +fn white(b: u8) -> bool { + matches!(b, 0 | 9 | 10 | 12 | 13 | 32) +} + +/// Where `what` is first found in `pdf` at or after `from`. +fn find(pdf: &[u8], from: usize, what: &[u8]) -> Option { + pdf.get(from..).and_then(|rest| rest.windows(what.len()).position(|w| w == what)).map(|at| from + at) +} + +/// Past the literal string opening at `at` (a `(`), as the reader reads it: parentheses nest and +/// `\` escapes the byte after it. `Err` with how far it got when it doesn't close before `limit`. +fn string_end(pdf: &[u8], at: usize, limit: usize) -> Result { + let (mut depth, mut i) = (0usize, at); + while let Some(&b) = pdf.get(i).filter(|_| i < limit) { + match b { + b'\\' => i += 1, + b'(' => depth += 1, + b')' => { + depth = depth.saturating_sub(1); + if depth == 0 { + return Ok(i + 1); } - _ => i -= 1, } + _ => {} } - let Some(dict) = open.and_then(|o| pdf.get(o..close)).and_then(Dict::from_bytes) else { continue }; - if !dict.get::>(b"Type").is_some_and(|t| matches!(t.as_ref(), b"ObjStm" | b"XRef")) { - continue; - } - let indirect = [&b"DecodeParms"[..], b"DP"].iter().any(|k| matches!(dict.get_raw::>(k), Some(MaybeRef::Ref(_)))); - if indirect || params(&dict).iter().any(bad_predictor) { - return true; - } - let start = from - + usize::from(pdf.get(from) == Some(&b'\r')) - + usize::from(pdf.get(from + usize::from(pdf.get(from) == Some(&b'\r'))) == Some(&b'\n')); - let length = dict.get::(b"Length").and_then(|n| start.checked_add(n)).filter(|e| *e <= pdf.len()); - let end = match length { - Some(end) => end, - None => { - if endstream.is_none_or(|e| e.is_some_and(|e| e < start)) { - endstream = Some(find(start, b"endstream")); - } - endstream.flatten().unwrap_or(pdf.len()) - } - }; - if matches!(decode(pdf.get(start..end).unwrap_or_default(), &dict, false), Err(TooBig)) { - return true; + i += 1; + } + Err(i.min(limit)) +} + +/// Past the hex string opening at `at` (a `<`): hex digits and white space, then `>`. `Err` with +/// how far it got for one the reader fails on. +fn hex_end(pdf: &[u8], at: usize, limit: usize) -> Result { + let mut i = at + 1; + while pdf.get(i).is_some_and(|b| i < limit && (b.is_ascii_hexdigit() || white(*b))) { + i += 1; + } + if i < limit && pdf.get(i) == Some(&b'>') { Ok(i + 1) } else { Err(i.min(limit)) } +} + +/// The end of the comment starting at `at` (a `%`): its end of line, not included. +fn comment_end(pdf: &[u8], at: usize) -> usize { + pdf.get(at..).and_then(|rest| rest.iter().position(|b| matches!(b, b'\n' | b'\r'))).map_or(pdf.len(), |n| at + n) +} + +/// Past the white space and comments from `at`. +fn skip_space(pdf: &[u8], mut at: usize) -> usize { + while let Some(&b) = pdf.get(at) { + match b { + b'%' => at = comment_end(pdf, at), + b if white(b) => at += 1, + _ => break, } } - false + at +} + +/// Past the dictionary opening at `at` (a `<<`), read to the `>>` that closes it before `limit`. +/// `Err` with how far it got for one that doesn't close there. +fn dict_end(pdf: &[u8], at: usize, limit: usize) -> Result { + let (mut depth, mut i) = (0usize, at); + while let Some(&b) = pdf.get(i).filter(|_| i < limit) { + match pdf.get(i..i + 2) { + Some(b"<<") => (depth, i) = (depth + 1, i + 2), + Some(b">>") => { + (depth, i) = (depth.saturating_sub(1), i + 2); + if depth == 0 { + return Ok(i); + } + } + _ => match b { + b'(' => i = string_end(pdf, i, limit)?, + b'<' => i = hex_end(pdf, i, limit)?, + b'%' => i = comment_end(pdf, i), + _ => i += 1, + }, + } + } + Err(i.min(limit)) +} + +/// The pre-open scan of a file's streams ([`unsafe_file_streams`]). +struct FileScan<'a> { + pdf: &'a [u8], + /// The bytes left to read again: dictionaries inside skipped text, `endstream` searches and + /// the data decoded. Every byte of a well-formed file is read about once. + budget: usize, + /// Where an `endstream` was last looked for, and the first one found there (`None`: none). + endstream: Option<(usize, Option)>, +} + +impl FileScan<'_> { + fn charge(&mut self, bytes: usize) -> Result<(), Unsafe> { + self.budget = self.budget.checked_sub(bytes).ok_or(Unsafe)?; + Ok(()) + } + + /// The file from the start, as the reader reads it. + fn read(&mut self) -> Result<(), Unsafe> { + let pdf = self.pdf; + let mut opens = std::collections::VecDeque::new(); + // The dictionary just read, while only white space and comments follow it. + let mut closed: Option<(usize, usize)> = None; + let mut i = 0; + while let Some(&b) = pdf.get(i) { + if white(b) { + i += 1; + continue; + } + if b == b'%' { + let end = comment_end(pdf, i); + self.skipped(i, end)?; + i = end; + continue; + } + let last = closed.take(); + match pdf.get(i..i + 2) { + Some(b"<<") => { + if opens.len() == MAX_OPEN { + opens.pop_front(); + } + opens.push_back(i); + // In `<<<` the reader may start a dictionary at the second `<` too. + if pdf.get(i + 2) == Some(&b'<') { + self.dict(i + 1)?; + } + i += 2; + } + Some(b">>") => { + closed = opens.pop_back().map(|open| (open, i + 2)); + i += 2; + } + _ if b == b'(' => { + let end = string_end(pdf, i, pdf.len()).unwrap_or(pdf.len()); + self.skipped(i, end)?; + i = end; + } + _ if b == b'<' => i = hex_end(pdf, i, pdf.len()).unwrap_or(i + 1), + _ => match last { + Some((open, close)) if pdf.get(i..).is_some_and(|rest| rest.starts_with(b"stream")) => { + let end = self.stream(open, close, i + 6)?; + self.skipped(i + 6, end)?; + i = end.max(i + 6); + } + _ => i += 1, + }, + } + } + Ok(()) + } + + /// Each `<<` in `from..to`, text the reading skipped, read as a dictionary of its own. + fn skipped(&mut self, from: usize, to: usize) -> Result<(), Unsafe> { + let text = self.pdf.get(..to).unwrap_or_default(); + let mut i = from; + while let Some(at) = find(text, i, b"<<") { + self.dict(at)?; + i = at + 1; + } + Ok(()) + } + + /// The dictionary opening at `at`, and the stream it begins if `stream` follows it. + fn dict(&mut self, at: usize) -> Result<(), Unsafe> { + let pdf = self.pdf; + let limit = at.saturating_add(MAX_SKIPPED_DICT).min(pdf.len()); + let end = dict_end(pdf, at, limit); + let (Ok(reached) | Err(reached)) = end; + self.charge(reached.saturating_sub(at))?; + let Ok(close) = end else { return Ok(()) }; + let keyword = skip_space(pdf, close); + self.charge(keyword - close)?; + if pdf.get(keyword..).is_some_and(|rest| rest.starts_with(b"stream")) { + self.stream(at, close, keyword + 6)?; + } + Ok(()) + } + + /// The stream whose dictionary is `open..close` and whose `stream` keyword ends at `from`: + /// `Err` when the reader would fail on it, else where its data ends. + fn stream(&mut self, open: usize, close: usize, from: usize) -> Result { + let pdf = self.pdf; + let cr = usize::from(pdf.get(from) == Some(&b'\r')); + let start = from + cr + usize::from(pdf.get(from + cr) == Some(&b'\n')); + let dict = pdf.get(open..close).and_then(Dict::from_bytes); + let length = dict.as_ref().and_then(|d| d.get::(b"Length")).and_then(|n| start.checked_add(n)).filter(|e| *e <= pdf.len()); + let end = match length { + Some(end) => end, + None => self.endstream(start)?, + }; + let Some(dict) = dict.filter(|d| d.get::>(b"Type").is_some_and(|t| matches!(t.as_ref(), b"ObjStm" | b"XRef"))) else { + return Ok(end); + }; + let indirect = [&b"DecodeParms"[..], b"DP"].iter().any(|k| matches!(dict.get_raw::>(k), Some(MaybeRef::Ref(_)))); + if indirect || params(&dict).iter().any(bad_predictor) { + return Err(Unsafe); + } + if !filters(&dict).is_empty() { + self.charge(end.saturating_sub(start))?; + } + if matches!(decode(pdf.get(start..end).unwrap_or_default(), &dict, false), Err(TooBig)) { + return Err(Unsafe); + } + Ok(end) + } + + /// Where the first `endstream` at or after `start` is, or the end of the file. + fn endstream(&mut self, start: usize) -> Result { + let len = self.pdf.len(); + if let Some((from, found)) = self.endstream + && from <= start + && found.is_none_or(|f| f >= start) + { + return Ok(found.unwrap_or(len)); + } + let found = find(self.pdf, start, b"endstream"); + self.charge(found.unwrap_or(len).saturating_sub(start))?; + self.endstream = Some((start, found)); + Ok(found.unwrap_or(len)) + } } #[cfg(test)] @@ -474,7 +657,7 @@ mod tests { ] { assert!(!unsafe_file_streams(&fine)); } - // A bad stream after others is still found: a walk back stops at the previous keyword, not before its own `<<`. + // A bad stream after others is still found. let fine = stream("/Type /XRef /Filter /FlateDecode /DecodeParms << /Predictor 12 /Columns 5 >>", b"x"); let bad = stream("/Type /XRef /Filter /FlateDecode /DecodeParms 9 0 R", b"x"); assert!(unsafe_file_streams(&[fine.as_slice(), b"<< >>stream x endstream", &bad].concat())); @@ -492,4 +675,49 @@ mod tests { assert!(took < std::time::Duration::from_secs(5), "{} took {took:?}", String::from_utf8_lossy(unit)); } } + + #[test] + fn a_stream_keyword_in_a_string_or_comment_hides_no_stream_after_it() { + // #3919 review: a `>>stream` in a literal string (one with a `<<` before it, nested + // parentheses, an escaped one) or in a comment, a `>>` in a string and a comment between + // `>>` and `stream` each left the cross-reference stream they were in or before unread. + let cases = [ + &b"<< /Type /XRef /Note (<<) /Pad (>>stream) /DecodeParms 9 0 R >> stream\nx\nendstream"[..], + b"<< /Type /XRef /Pad (a (b) \\) >>stream) /DecodeParms 9 0 R >> stream\nx\nendstream", + b"<< /Type /XRef /Pad (>>) /DecodeParms 9 0 R >> stream\nx\nendstream", + b"<< /Type /XRef % >>stream\n/DecodeParms 9 0 R >> stream\nx\nendstream", + b"<< /Type /XRef /DecodeParms 9 0 R >> % a comment\nstream\nx\nendstream", + ]; + let unread: Vec<_> = cases.iter().filter(|bad| !unsafe_file_streams(bad)).map(|bad| String::from_utf8_lossy(bad)).collect(); + assert!(unread.is_empty(), "{unread:#?}"); + } + + #[test] + fn a_stream_in_text_the_scan_skips_is_still_read() { + // The scan skips a stream's data, a string and a comment; the reader may still start at + // a dictionary in them (a cross-reference offset can point anywhere), so they're read too. + let bad = b"<< /Type /XRef /DecodeParms 9 0 R >> stream\nx\nendstream"; + for wrap in [ + [&b"1 0 obj << /Length 99 >> stream\n"[..], bad, b"\nendstream endobj"], + [b"(", bad, b")"], + [b"% ", bad, b"\n"], + [b"<< /Note (", bad, b") >>"], + ] { + let pdf = wrap.concat(); + assert!(unsafe_file_streams(&pdf), "{}", String::from_utf8_lossy(&pdf)); + } + } + + #[test] + fn strings_and_comments_dense_with_dictionaries_are_scanned_in_bounded_time() { + // Each `<<` in a string or comment is read as a dictionary of its own (the reader may + // start at one), within a budget for the whole file. + for unit in [&b"<<("[..], b"(<<", b"%<<\n", b"<>stream) >>stream\n"] { + let pdf = unit.repeat((1 << 20) / unit.len()); + let t = std::time::Instant::now(); + unsafe_file_streams(&pdf); + let took = t.elapsed(); + assert!(took < std::time::Duration::from_secs(5), "{} took {took:?}", String::from_utf8_lossy(unit)); + } + } } diff --git a/docs/gaps.md b/docs/gaps.md index 7a3020ed3..3bb786ce3 100644 --- a/docs/gaps.md +++ b/docs/gaps.md @@ -613,6 +613,7 @@ trap, and the defaults (0.25 pt, 100%, 40%) are VectorCraft's own, the pages giv |---|---|---| | 2026-10-24 | minor | G14 shrinks: pen pressure and the pen's eraser end on macOS, read from AppKit's tablet events (#852; not yet tried on a Mac); estimate 8–16 h → 4–10 h. | | 2026-10-24 | minor | G11: Shape Builder pieces: the planar map contracts the stubs the sweep left between two cuts of a line's passes a rounding error apart (bounded by each cluster's extent), so dense hatches at exact binary steps no longer add a speck per crossing; estimate unchanged, #937 open. | +| 2026-10-24 | minor | G10: the PDF pre-open stream scan reads the file as the reader does (literal and hex strings, comments, stream data skipped), so a `stream` or `>>` in a string or comment, or a comment before `stream`, no longer hides a cross-reference or object stream from it (#3919 review); a `<<` in skipped text is read as a dictionary too, within a budget a crafted file that spends is refused on. No entry closed, estimates unchanged. | | 2026-10-24 | minor | G4: images in OpenType SVG glyph documents draw, export and outline with the glyph; colour fonts are done but for CPAL palette variables; estimate unchanged. | | 2026-10-24 | minor | G4: bitmap glyphs (emoji) go with type that effects reshape, into Expand Appearance, envelope Expand and EMF/WMF export, as images that hold their own pixels until a document takes them in; estimate unchanged. | | 2026-10-24 | minor | G10: Fix #3586: the PDF pre-open scan reads a file dense with `stream` keywords in linear time; each keyword walked back up to 64 KiB through every dictionary before it, so 1 MiB of `<> >>stream` took minutes. No entry closed, estimates unchanged. |