mirror of
https://github.com/storytold/pdfcraft.git
synced 2026-10-15 03:37:03 -04:00
M2: bound lopdf predictor scratch to available samples (#470)
* M2: bound lopdf predictor scratch to available samples Return empty PNG predictor frames before allocating row buffers. Reject overflowing packed TIFF row dimensions and bound component scratch to the samples present in the input, aligning with the existing lopdf#589 fix. Cover hostile dimensions and valid packed rows with synthetic regressions and record the vendored patch and its upstream proposal. Co-Authored-By: OpenAI Codex <[email protected]> * Keep main's M1 roadmap row after merging main --------- Co-authored-by: OpenAI Codex <[email protected]> Co-authored-by: Brandon Thomas <[email protected]>
This commit is contained in:
5 files changed
+77
-6
No files matched your search
+2
-1
@@ -90,8 +90,8 @@ Hours are for a single agent (low–high). "Done" is the estimated fraction of t
|
||||
| M | Milestone | Est. hours | Done | Remaining (h) | Notes |
|
||||
|---|---|---|---|---|---|
|
||||
| M0 | Skeleton: workspace, xtask gates, CI | 15–30 | 92% | 2–4 | GitHub workflow, `deny.toml` (licence audit of every dependency), parity checklist (826 features, `xtask parity`) done. Missing: remaining crate stubs, testkit/oracle crates |
|
||||
| M2 | Model, render, text | 200–350 | 15% | 170–300 | hayro bootstrap renderer (vendored patches); inspector predictor scratch is bounded by available input. Text extraction reaches word-F1 0.98 against pdftotext. Missing: model crate, fonts, DisplayList, renderer independent of hayro |
|
||||
| M1 | COS: filters, crypt, parser, xref, writer | 120–200 | 78% | 25–45 | Done:<br>- filters and crypt: every standard-security revision R2–R6 (RC4, AES-128/256), SASLprep, permissions, creating encryption;<br>- cos: parse and repair (including invalid xref object-number ranges), decoded-stream limits including unfiltered data, decrypt on load, re-encrypt on save, incremental and full writing.<br>Corpus: open/edit/save passes on 958 files, and all 7 password-protected files open.<br>Full saves now pack objects into compressed object streams. Fuzzing runs nightly (`xtask fuzz`). Missing: ≥ 250 tests, own image codecs |
|
||||
| M2 | Model, render, text | 200–350 | 15% | 170–300 | hayro bootstrap renderer (vendored patches). Text extraction reaches word-F1 0.98 against pdftotext. Missing: model crate, fonts, DisplayList, renderer independent of hayro |
|
||||
| M3 | Viewer app (native + web) | 80–150 | 88% | 10–19 | Acrobat-style shell, find, select, panels, tiles, web build, UI control channel for agents (opt-in). Missing: 60 fps test on a 500-page document, snapshot tests of every panel Done since: fit visible, document title in the window; Linux middle-button scrolling; shared three-state theme preferences in the toolbar, View menu and control channel. |
|
||||
| M4 | Engine, history, save, organize | 100–180 | 95% | 9–17 | Done:<br>- command registry (menus, shortcuts and palette all use it);<br>- undo/redo; incremental, atomic and encrypted saves;<br>- autosave and crash recovery;<br>- organize, combine, extract, split and insert-from-file, with identical fonts and images stored once;<br>- CLI `edit/combine/extract/split`.<br>Done since: Combine files in its own tab (sortable, resizable columns; folders; early warnings; unlocking protected files), bookmark editing, page labels (Number pages), CLI `run`, Set Page Boxes, Crop tool, Duplicate pages, Ctrl+A / Cmd+A to select all pages in Organize. Missing: Replace pages, page transitions, recovery on the web |
|
||||
| M5 | Comments (all annotation types, XFDF) | 120–200 | 88% | 15–25 | Done: notes, highlight/underline/strikeout/squiggly, text boxes, ink, lines, arrows, rectangles, ovals, stamps (dynamic, Sign Here, Standard Business), with appearance streams; replies, status, checkmarks, locking, move/resize/restyle/delete; properties dialog; panel filter and sort; hide all; summaries (comments only); XFDF/FDF import and export; flatten; Fill & Sign; agent tools. Done since: polygons, connected lines, clouds, callouts, inserted text, colour and checkmark filters; image signature pointer previews, selection handles and live image movement/resizing with proportional corners and independent edge handles. Missing: custom stamps, replace-text proposals, summary layouts with the page |
|
||||
@@ -138,6 +138,7 @@ M0 → M1 → M2 → M3 → M4 must happen in order. After M4, M5–M12 can run
|
||||
|
||||
Newest first. One line per session: the date, what moved, and the new overall percentage.
|
||||
|
||||
- **2026-10-09 (M2, predictor hardening):** Empty PNG predictor frames return without row buffers. Packed TIFF Predictor 2 rows reject overflowing dimensions and size per-component scratch to the samples present in the input, aligned with upstream lopdf#589. Synthetic regressions cover malformed dimensions, partial rows, 1/2/4-bit decoding and padding. Overall estimate unchanged (about 30–35%).
|
||||
- **2026-10-09 (community, document inspection):** Annotation dates preserve non-date Unicode text instead of panicking while opening; failed auxiliary structure inspection keeps renderer page geometry and reports a warning. Synthetic UTF16BE annotation, UTF8-boundary and independent panic regressions cover the fix. Overall estimate unchanged (about 30–35%).
|
||||
|
||||
- **2026-10-09 (community, Home pinned folders):** Home gets a Folders section: File ▸ Pin folder to Home… (or "Pin a folder…" on Home) pins up to 8 folders, and each lists its newest PDFs (name, how long ago, size; 8 at first, up to 100 with Show more), one click to open, Unpin to remove. Folders are read on a worker thread and listed again every few seconds while Home shows, so new scans appear without reopening anything; a folder that can't be read (moved, offline synced drive) stays pinned and says so. Each folder is described by where it is ("iCloud Drive", "OneDrive · Contoso", "Documents › Work") rather than its full path, so two folders called "Scans" can be told apart. Cloud drives work through their local synced folder; nothing is read through a provider's API. Settings keep only absolute paths, without duplicates. Command label translated in every catalog; Home strings in French, Spanish, Japanese and both Chinese catalogs. Not yet: subfolders, search inside a pinned folder, the web build. Overall estimate unchanged (about 30–35%).
|
||||
|
||||
@@ -1013,6 +1013,65 @@ mod tests {
|
||||
assert_eq!(lopdf::filters::png::decode_frame(&[0, 1, 2, 3, 2, 1, 1, 1], 1, 3).unwrap(), [1, 2, 3, 2, 3, 4]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn empty_png_predictor_frames_do_not_allocate_rows() {
|
||||
// usize::MAX cannot be reserved, so the old code safely errors before allocating.
|
||||
// An empty frame needs no rows regardless of the declared width.
|
||||
assert!(lopdf::filters::png::decode_frame(&[], 1, usize::MAX).unwrap().is_empty());
|
||||
}
|
||||
|
||||
fn tiff_predictor_stream(data: Vec<u8>, columns: i64, colors: i64, bits: i64) -> lopdf::Stream {
|
||||
let mut params = lopdf::Dictionary::new();
|
||||
params.set("Predictor", 2i64);
|
||||
params.set("Columns", columns);
|
||||
params.set("Colors", colors);
|
||||
params.set("BitsPerComponent", bits);
|
||||
let mut dict = lopdf::Dictionary::new();
|
||||
dict.set("DecodeParms", params);
|
||||
let mut stream = lopdf::Stream::new(dict, data);
|
||||
stream.compress().unwrap();
|
||||
assert_eq!(stream.dict.get(b"Filter").unwrap().as_name().unwrap(), b"FlateDecode");
|
||||
stream
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn tiff_subbyte_predictor_row_width_overflow_is_refused() {
|
||||
// Each multiplication overflows before any scratch allocation in the old debug build.
|
||||
for bits in [1, 2, 4] {
|
||||
let stream = tiff_predictor_stream(vec![0; 64], i64::MAX, 3, bits);
|
||||
assert!(
|
||||
matches!(stream.decompressed_content_with_limit(64), Err(lopdf::Error::Decompress(lopdf::DecompressError::Predictor(_)))),
|
||||
"{bits}-bit row"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn tiff_subbyte_predictor_scratch_is_bounded_by_available_samples() {
|
||||
// 1/2-bit row widths fit usize on 32- and 64-bit hosts; Colors previously made Vec<u16>
|
||||
// reject the capacity before allocating. A partial row has no preceding pixel.
|
||||
for bits in [1, 2] {
|
||||
let data = vec![0b1010_0110; 64];
|
||||
let stream = tiff_predictor_stream(data.clone(), 1, i64::try_from(isize::MAX).unwrap(), bits);
|
||||
assert_eq!(stream.decompressed_content_with_limit(64).unwrap(), data, "{bits}-bit row");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn tiff_subbyte_predictors_keep_components_rows_and_padding() {
|
||||
// Repeated tiny rows compress through the public Stream API. Rows remain independent,
|
||||
// differences wrap at each component depth, and trailing padding bits survive.
|
||||
for (columns, colors, bits, encoded, decoded) in [
|
||||
(16, 1, 1, vec![255, 170, 8, 255], vec![170, 204, 15, 85]),
|
||||
(6, 1, 2, vec![85, 179], vec![108, 147]),
|
||||
(3, 1, 4, vec![25, 16], vec![26, 176]),
|
||||
(2, 2, 4, vec![18, 34], vec![18, 52]),
|
||||
] {
|
||||
let stream = tiff_predictor_stream(encoded.repeat(32), columns, colors, bits);
|
||||
assert_eq!(stream.decompressed_content_with_limit(128).unwrap(), decoded.repeat(32), "{bits}-bit, {colors} colours");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn view_and_user_space_round_trip_for_every_rotation() {
|
||||
for rotation in [0u16, 90, 180, 270] {
|
||||
|
||||
Vendored
+1
-1
@@ -11,7 +11,7 @@ the fix. Vendoring copyleft code is never allowed (plan/adr/0001).
|
||||
|
||||
| hayro | 0.7.1 | Apache-2.0 OR MIT | (1) `RenderSettings::{x_offset, y_offset}` to render one tile of a large page; (2) images over `MAX_IMAGE_PIXELS` (2^28) are skipped (a fuzzed `/W 4294967295` hung in resampling); (3) `tiling_cell_scale` keeps a tiling pattern's cell pixmap within 3000 px a side whatever its `/XStep`/`/YStep` (a fuzzed step far beyond `/BBox` allocated 2 GB); (4) stroke widths clamped to 4 × (canvas width + height) device pixels (a fuzzed `/LW 9223372036854775807` allocated 10 GB in stroke expansion); (5) `usable_dash_pattern`: a stroke whose dash pattern could cut it into more than `MAX_DASHES_PER_STROKE` (250,000) pieces, counting kurbo's restart of the pattern at every subpath and `ClosePath`, is drawn solid (a fuzzed `/D [[11] 0]` on a line from x = 92234775807 allocated over 5 GB, `[0.000001] 0 d` on a 20 pt line over 10 GB); (6) a dash pattern with a negative or non-finite entry, or a period that isn't positive, is drawn solid (`[-1 -1] 0 d` looped for ever in kurbo) | (1) `raster::tests::tiles_match_full_render`; (2) `raster::tests::absurd_image_dimensions_are_skipped`; (3) `raster::tests::tiling_cells_stay_small_whatever_the_step`; (4) `raster::tests::absurd_predictor_columns_and_line_widths_render`; (5) `raster::tests::dash_patterns_that_cut_a_stroke_into_billions_of_pieces_render`; (6) `raster::tests::invalid_dash_patterns_terminate_and_are_drawn_solid` |
|
||||
| hayro-jbig2 | 0.3.0 | Apache-2.0 OR MIT | (1) at most 2^28 pixels per bitmap, page bitmap, halftone grid and grayscale image (each side could be 65535: 4.3 billion pixels decoded one by one); (2) at most 2^26 symbol pixels per symbol dictionary (a fuzzed dictionary decoded for minutes) | (1) `raster::tests::jbig2_regions_of_billions_of_pixels_are_refused`; (2) the nightly `cargo xtask fuzz` finding, which now finishes in about a second |
|
||||
| lopdf | 0.45.0 | MIT | (1) PNG predictor rows longer than the data are refused before their buffers are allocated (a fuzzed `/Columns 4294967295` allocated two 4 GiB rows); (2) predictor row widths use saturating arithmetic | (1) `inspect::tests::png_predictor_rows_longer_than_the_data_are_refused_before_allocating` |
|
||||
| lopdf | 0.45.0 | MIT | (1) PNG predictor rows longer than the data are refused before their buffers are allocated (a fuzzed `/Columns 4294967295` allocated two 4 GiB rows), and empty frames return without row buffers; (2) predictor row widths use saturating arithmetic or reject overflow; (3) sub-byte TIFF Predictor 2 scratch is bounded by the samples present in the input instead of the declared `/Colors`, aligned with [lopdf#589](https://github.com/J-F-Liu/lopdf/pull/589) | (1) `inspect::tests::png_predictor_rows_longer_than_the_data_are_refused_before_allocating`, `inspect::tests::empty_png_predictor_frames_do_not_allocate_rows`; (2) `inspect::tests::tiff_subbyte_predictor_row_width_overflow_is_refused`; (3) `inspect::tests::tiff_subbyte_predictor_scratch_is_bounded_by_available_samples`, `inspect::tests::tiff_subbyte_predictors_keep_components_rows_and_padding` |
|
||||
| hayro-syntax | 0.7.2 | Apache-2.0 OR MIT | (1) Page-tree cycle guard in `resolve_pages` (visited set + depth cap): a `/Kids` loop among compressed objects overflowed the stack and aborted the process; (2) `ccitt_size_ok`: CCITT images wider than 2^20 columns or over 2^28 pixels are not decoded (a fuzzed `/Columns 4294967295` allocated 4 GiB); (3) Flate/LZW predictor row widths are saturating `usize`, a zero width is refused and a row longer than the data is not allocated (a fuzzed `/Columns 9223372036854775807` wrapped to a 2^61-byte allocation and aborted); (4) the inline-image end-of-data check (`EI`) doesn't nest inside its own look-ahead and examines at most 256 candidates and steps (50 images with "EI" in their data ran for over ten minutes); (5) a stream whose `/Length` is wrong is recovered only when `stream` is the next token — scanning ahead to a later object's `stream` made an indirect appearance-state dictionary (Quartz checkboxes: `/AP /N` → `<< /Yes … /Off … >>`) render as a non-form, so the widget was skipped | (1) `cos/tests/writer.rs::page_tree_loops_in_object_streams_do_not_crash_readers`; (2) `raster::tests::absurd_mask_and_fax_sizes_are_refused_before_decoding`; (3) `raster::tests::absurd_predictor_columns_and_line_widths_render`; (4) `raster::tests::inline_images_with_ei_in_their_data_parse_in_linear_time`; (5) `raster::tests::indirect_appearance_state_dict_is_not_another_stream` |
|
||||
|
||||
Temporary: hayro is the bootstrap renderer and lopdf the bootstrap inspector (ADR-0004); M2.6 replaces them with our own devices and `cos`/`model`.
|
||||
|
||||
Vendored
+6
-1
@@ -86,10 +86,15 @@ pub fn decode_row(filter: FilterType, bpp: usize, previous: &[u8], current: &mut
|
||||
}
|
||||
|
||||
pub fn decode_frame(content: &[u8], bpp: usize, bytes_per_row: usize) -> Result<Vec<u8>> {
|
||||
// PdfCraft patch: an empty frame needs no row buffers, even for an absurd declared width.
|
||||
if content.is_empty() {
|
||||
return Ok(Vec::new());
|
||||
}
|
||||
|
||||
// PdfCraft patch: each row is a filter byte plus `bytes_per_row` bytes, so a row longer
|
||||
// than the data can never be read (`read_exact` below fails). Refuse it before allocating:
|
||||
// a fuzzed `/Columns 4294967295` allocated two 4 GiB rows here.
|
||||
if !content.is_empty() && bytes_per_row >= content.len() {
|
||||
if bytes_per_row >= content.len() {
|
||||
return Err(Error::new(ErrorKind::InvalidData, "PNG predictor row is longer than the data"));
|
||||
}
|
||||
let mut previous = Vec::new();
|
||||
|
||||
Vendored
+9
-3
@@ -1277,7 +1277,12 @@ impl Stream {
|
||||
}
|
||||
}
|
||||
1 | 2 | 4 => {
|
||||
let stride = (columns * colors * bits).div_ceil(8);
|
||||
// PdfCraft patch: reject overflowing packed row widths (lopdf#589).
|
||||
let stride = columns
|
||||
.checked_mul(colors)
|
||||
.and_then(|samples| samples.checked_mul(bits))
|
||||
.ok_or(DecompressError::Predictor("predictor row size overflows"))?
|
||||
.div_ceil(8);
|
||||
if stride > 0 {
|
||||
for row in data.chunks_mut(stride) {
|
||||
Self::reverse_tiff_predictor2_subbyte(row, columns, colors, bits);
|
||||
@@ -1299,8 +1304,9 @@ impl Stream {
|
||||
/// `2^bits`, then repacked MSB-first; the row's trailing padding bits are kept.
|
||||
fn reverse_tiff_predictor2_subbyte(row: &mut [u8], columns: usize, colors: usize, bits: usize) {
|
||||
let mask = (1u16 << bits) - 1;
|
||||
let samples = columns.saturating_mul(colors).min(row.len() * 8 / bits); // PdfCraft patch: saturating
|
||||
let mut acc = vec![0u16; colors];
|
||||
// PdfCraft patch: bound scratch to the samples present in the input (lopdf#589).
|
||||
let samples = columns.saturating_mul(colors).min(row.len().saturating_mul(8 / bits));
|
||||
let mut acc = vec![0u16; colors.min(samples)];
|
||||
let mut out = vec![0u8; row.len()];
|
||||
let mut in_bit = 0usize;
|
||||
let mut out_buf = 0u32;
|
||||
|
||||
Reference in new issue
Block a user