From 2d5024bf82c7dec0017d8e443fc3e6d578745cb2 Mon Sep 17 00:00:00 2001 From: Taha-Mahmoodi <85902429+Taha-Mahmoodi@users.noreply.github.com> Date: Sat, 24 Oct 2026 06:23:58 +0430 Subject: [PATCH] PDF import: a run of "<" in skipped text no longer spends the pre-open scan's budget MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review of #3924: every `<<` in a string, comment or stream's data was read as a dictionary of its own, and in a run of `<` (an uncompressed image's flat area of grey 60) every position is one, each read to past the run, so about NĀ²/2 bytes for N `<` and a 1 MiB run refused the file. The reader starts a dictionary there only where its repair scan tries one (after white space or a dictionary's `>>`) or after an object's `N G obj`: so only those `<<` are read, in a run of `<` only the first. A reading goes on as far as 64 KiB past the last `<<` opened on the way, looks at each nested dictionary's `>>` for a stream and marks each `<<`, so it is not read again; a comment's end of line is kept, as many readings cross the same comment; the `<<<` second-`<` reading in the main pass goes (the reader never starts there). The dictionary text a stream check parses is charged to the budget. --- crates/pdf/src/streams.rs | 229 +++++++++++++++++++++++++++----------- docs/gaps.md | 2 +- 2 files changed, 164 insertions(+), 67 deletions(-) diff --git a/crates/pdf/src/streams.rs b/crates/pdf/src/streams.rs index 4b2086de9..c19b99a74 100644 --- a/crates/pdf/src/streams.rs +++ b/crates/pdf/src/streams.rs @@ -275,18 +275,19 @@ fn unsafe_data(stream: &Stream<'_>, inside: bool) -> bool { /// 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). +/// its repair scan can point there), so each such `<<` it could start at 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), a run of `<` included (#3924 review). pub(crate) fn unsafe_file_streams(pdf: &[u8]) -> bool { - let mut scan = FileScan { pdf, budget: pdf.len().saturating_mul(4).saturating_add(1 << 20), endstream: None }; + let mut scan = FileScan { pdf, budget: pdf.len().saturating_mul(4).saturating_add(1 << 20), endstream: None, line: None, read_from: Vec::new() }; 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. +/// The most bytes a dictionary read from inside a string, comment or stream data may span past +/// the last `<<` in it, 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. @@ -338,40 +339,13 @@ 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, - } - } - 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)) +/// Can the reader start a dictionary at the `<<` at `at`? Its repair scan tries one at the start +/// of the file, after white space and after a dictionary it read (`>>`); an object that a +/// cross-reference offset points at begins `N G obj`, then white space or comments. A `<<` after +/// anything else is read only as part of what is before it: in a run of `<`, only the first. +fn startable(pdf: &[u8], at: usize) -> bool { + let before = pdf.get(..at).unwrap_or_default(); + before.last().is_none_or(|b| white(*b)) || before.ends_with(b">>") || before.ends_with(b"obj") } /// The pre-open scan of a file's streams ([`unsafe_file_streams`]). @@ -382,6 +356,11 @@ struct FileScan<'a> { budget: usize, /// Where an `endstream` was last looked for, and the first one found there (`None`: none). endstream: Option<(usize, Option)>, + /// Where a comment's end of line was last looked for, and where it is. + line: Option<(usize, usize)>, + /// The `<<` a dictionary in skipped text was read from, a bit a byte of the file (empty + /// until one is). + read_from: Vec, } impl FileScan<'_> { @@ -415,10 +394,6 @@ impl FileScan<'_> { 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">>") => { @@ -444,39 +419,123 @@ impl FileScan<'_> { Ok(()) } - /// Each `<<` in `from..to`, text the reading skipped, read as a dictionary of its own. + /// Each `<<` in `from..to`, text the reading skipped, that the reader could start at, 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)?; + if startable(self.pdf, at) { + self.dicts(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> { + /// The dictionary at `at` in skipped text, read to the `>>` that closes it, as far as + /// [`MAX_SKIPPED_DICT`] past the last `<<` opened on the way; and each dictionary it holds. + /// Each `>>` closing one is looked at for a stream after it, and each `<<` opening one is + /// marked: a reading from there reads the same text as far, so a later call for it returns + /// at once and text dense with `<<` is read about once. + fn dicts(&mut self, at: usize) -> Result<(), Unsafe> { + if self.was_read(at) { + return Ok(()); + } 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); + let mut opens = std::collections::VecDeque::new(); + let (mut depth, mut limit, mut i, mut work) = (0usize, at.saturating_add(MAX_SKIPPED_DICT), at, 0usize); + while let Some(&b) = pdf.get(i).filter(|_| i < limit) { + let next = match pdf.get(i..i + 2) { + Some(b"<<") => { + self.mark(i); + limit = limit.max(i.saturating_add(MAX_SKIPPED_DICT)); + if opens.len() == MAX_OPEN { + opens.pop_front(); + } + opens.push_back(i); + depth += 1; + Ok(i + 2) + } + Some(b">>") => { + depth = depth.saturating_sub(1); + if let Some(open) = opens.pop_back() { + self.closed(open, i + 2)?; + } + if depth == 0 { Err(i + 2) } else { Ok(i + 2) } + } + _ => match b { + b'(' => string_end(pdf, i, limit), + b'<' => hex_end(pdf, i, limit), + b'%' => { + i = self.line_end(i)?; + continue; + } + _ => Ok(i + 1), + }, + }; + let (Ok(n) | Err(n)) = next; + work += n.saturating_sub(i); + if next.is_err() { + break; + } + i = n; + } + self.charge(work) + } + + /// The dictionary `open..close` in skipped text: the stream it begins if `stream` follows it. + fn closed(&mut self, open: usize, close: usize) -> Result<(), Unsafe> { + let mut keyword = close; + while let Some(&b) = self.pdf.get(keyword) { + match b { + b'%' => keyword = self.line_end(keyword)?, + b if white(b) => keyword += 1, + _ => break, + } + } self.charge(keyword - close)?; - if pdf.get(keyword..).is_some_and(|rest| rest.starts_with(b"stream")) { - self.stream(at, close, keyword + 6)?; + if self.pdf.get(keyword..).is_some_and(|rest| rest.starts_with(b"stream")) { + self.stream(open, close, keyword + 6)?; } Ok(()) } + /// The end of the comment at `at` ([`comment_end`]), the last one found kept: comments in + /// skipped text are read again from many places. + fn line_end(&mut self, at: usize) -> Result { + if let Some((from, end)) = self.line + && from <= at + && at <= end + { + return Ok(end); + } + let end = comment_end(self.pdf, at); + self.charge(end - at)?; + self.line = Some((at, end)); + Ok(end) + } + + fn mark(&mut self, at: usize) { + if self.read_from.is_empty() { + self.read_from = vec![0; self.pdf.len() / 64 + 1]; + } + if let Some(word) = self.read_from.get_mut(at / 64) { + *word |= 1 << (at % 64); + } + } + + fn was_read(&self, at: usize) -> bool { + self.read_from.get(at / 64).is_some_and(|word| word >> (at % 64) & 1 == 1) + } + /// 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')); + self.charge(close.saturating_sub(open))?; 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 { @@ -695,29 +754,67 @@ mod tests { #[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. + // a dictionary in them (a cross-reference offset can point at `1 0 obj` anywhere, its + // repair scan tries a `<<` after white space), 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") >>"], + // `data` as the data of a stream whose length holds all of it. + let image = |data: &[u8]| [format!("1 0 obj << /Length {} >> stream\n", data.len()).as_bytes(), data, b"\nendstream endobj"].concat(); + let run = vec![b'<'; 1 << 16]; + let cases = [ + image(bad), + [&b"(1 0 obj "[..], bad, b")"].concat(), + [&b"(1 0 obj"[..], bad, b")"].concat(), + [&b"% "[..], bad, b"\n"].concat(), + [&b"<< /Note (1 0 obj "[..], bad, b") >>"].concat(), + // #3924 review: right after a run of `<` in an image, and as the end of one (a + // reader starting at the run's first `<<` reads it inside the others). + image(&[&run[..], b"\n", bad].concat()), + image(&[&run[..], b" /Type /XRef /DecodeParms 9 0 R >> stream\nx\nendstream"].concat()), + ]; + let unread: Vec<_> = + cases.iter().filter(|bad| !unsafe_file_streams(bad)).map(|bad| String::from_utf8_lossy(&bad[..bad.len().min(80)])).collect(); + assert!(unread.is_empty(), "{unread:#?}"); + } + + #[test] + fn a_run_of_lt_in_an_image_a_string_or_a_comment_opens() { + // #3924 review: every `<<` in a run of `<` (an image's flat area of grey 60) was read as a + // dictionary to past the run, so 1 MiB of it spent the budget and the file was refused. + // The reader starts a dictionary only at the run's first `<<`. + let run = vec![b'<'; 1 << 20]; + let image = b"1 0 obj << /Type /XObject /Subtype /Image /Width 1024 /Height 1024 /ColorSpace /DeviceGray /BitsPerComponent 8 /Length 1048576 >> stream\n"; + for (what, pdf) in [ + ("image", [&b"%PDF-1.7\n"[..], image, &run[..], b"\nendstream endobj\n"].concat()), + ("string", [&b"%PDF-1.7\n2 0 obj ("[..], &run[..], b") endobj\n"].concat()), + ("comment", [&b"%PDF-1.7\n% "[..], &run[..], b"\n"].concat()), ] { - let pdf = wrap.concat(); - assert!(unsafe_file_streams(&pdf), "{}", String::from_utf8_lossy(&pdf)); + let t = std::time::Instant::now(); + assert!(!unsafe_file_streams(&pdf), "{what}"); + let took = t.elapsed(); + assert!(took < std::time::Duration::from_secs(5), "{what} took {took:?}"); } } #[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"] { + // Each `<<` in a string or comment the reader could start at is read as a dictionary of + // its own, within a budget for the whole file. + for unit in [&b"<<("[..], b"(<<", b"%<<\n", b"<>stream) >>stream\n", b"<"] { 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)); } + // An image's flat area of a colour whose bytes are `<<` and white space, `(` or `%` (a + // reader may start at each `<<` after white space): read about once, and it opens. + for pixel in [&b" <<"[..], b"<<(", b"<<%", b" <<%", b"\0<<\0"] { + let data = pixel.repeat((1 << 20) / pixel.len()); + let pdf = [format!("1 0 obj << /Length {} >> stream\n", data.len()).as_bytes(), &data[..], b"\nendstream endobj\n"].concat(); + let t = std::time::Instant::now(); + assert!(!unsafe_file_streams(&pdf), "{}", String::from_utf8_lossy(pixel)); + let took = t.elapsed(); + assert!(took < std::time::Duration::from_secs(5), "{} took {took:?}", String::from_utf8_lossy(pixel)); + } } } diff --git a/docs/gaps.md b/docs/gaps.md index 3bb786ce3..e2e64bad0 100644 --- a/docs/gaps.md +++ b/docs/gaps.md @@ -613,7 +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 | 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 that the reader could start a dictionary at (after white space, `>>` or `obj`) is read as one too, each about once, so a run of `<` in an uncompressed image (grey 60) no longer spends the budget and refuses the file (#3924 review). 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. |