Never crash: JBIG2 symbol dictionaries can't loop without end (#408)
`cargo xtask fuzz` found a 1.1 KB page whose JBIG2 image never finished decoding (over two minutes, flat memory). It differs from a valid file by one byte inside a symbol dictionary's arithmetic-coded data. Past the end of its data the arithmetic decoder keeps returning values, and two loops in symbol.rs only move on when those values make progress: - Export runs (6.5.10): a run of length 0 doesn't advance, and the decoder returned 0 for ever. This was the fuzz finding. - Height classes (6.5.5): a class whose first width is already the end-of-class marker decodes no symbol. A read-only sweep of the crate found this one. In a synthetic dictionary every class after the first symbol adds 13 to the height and decodes nothing, so it only fails when the height overflows u32, after 330 million classes (about five seconds per image, logged); a delta of 0 would never end. The sweep found every other loop in hayro-jbig2 bounded by a symbol, instance or pixel cap or by the end of the data. Bounded is not cheap everywhere: refinement bitmaps in text regions and aggregate symbols (up to 10,000 instances of up to 2^28 pixels each) are not changed here. Vendored hayro-jbig2 patch (3): a valid dictionary needs at most one height class per new symbol and `2 x symbols + 1` export runs (they alternate, so at most one zero-length run before each symbol's). Both loops get that budget plus `EXTRA_EMPTY_STEPS` (1024), then fail the dictionary as invalid. Encoders don't write empty steps back to back, so the slack only tolerates oddities. The fuzz finding now renders in 0.2 s. Synthetic regression test in render: two dictionaries of a few arbitrary bytes each, found by searching for streams that decode the same way (no corpus bytes). The height-class one is drawn as twenty images so that it stalls well past the 20 s timeout without the patch on any machine. Each case is red with its own budget removed and green with it. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Brandon Thomas <echelon@gmail.com>
This commit is contained in:
jcl-bit
GitHub
parent
49fb1b1b35
commit
ee06583523
|
|
@ -1613,6 +1613,70 @@ trailer << /Root 1 0 R >>
|
|||
assert_eq!(&page.rgba[((38 * 40 + 1) * 4)..][..4], &[255, 0, 0, 255], "the rest of the page draws");
|
||||
}
|
||||
|
||||
/// From `cargo xtask fuzz`: one changed byte in a symbol dictionary's arithmetic-coded data
|
||||
/// hung the renderer. Two of its loops can stop advancing: an export run of length 0, and a
|
||||
/// height class whose first width is already the end-of-class marker. Past the end of its
|
||||
/// data the decoder can return those for ever. Vendored hayro-jbig2 patch: at most one height
|
||||
/// class per new symbol and `2 × symbols + 1` export runs, each plus `EXTRA_EMPTY_STEPS`.
|
||||
/// These synthetic dictionaries (two new symbols each, a few bytes of arbitrary data) decode
|
||||
/// the same way. In the second every class after the first symbol adds 13 to the height, so it
|
||||
/// only fails when the height overflows `u32`, after 330 million empty classes (about five
|
||||
/// seconds; a delta of 0 would never end). The page draws it as twenty images, so that the
|
||||
/// stall without the patch is well past the timeout on any machine.
|
||||
#[test]
|
||||
fn jbig2_symbol_dictionaries_that_never_advance_terminate() {
|
||||
for (what, data, copies) in
|
||||
[("zero-length export runs", &[0x05, 0x70, 0x05, 0x14, 0x02, 0x1d, 0x56, 0x3f][..], 1), ("empty height classes", &[0x06, 0x99][..], 20)]
|
||||
{
|
||||
// Embedded JBIG2 segments (ISO 14492 §7.2): page information, then a symbol dictionary.
|
||||
let mut jbig2 = Vec::new();
|
||||
let mut segment = |number: u32, kind: u8, data: &[u8]| {
|
||||
jbig2.extend_from_slice(&number.to_be_bytes());
|
||||
jbig2.extend_from_slice(&[kind, 0, 1]);
|
||||
jbig2.extend_from_slice(&u32::try_from(data.len()).unwrap().to_be_bytes());
|
||||
jbig2.extend_from_slice(data);
|
||||
};
|
||||
let mut page = Vec::new();
|
||||
page.extend_from_slice(&8u32.to_be_bytes());
|
||||
page.extend_from_slice(&8u32.to_be_bytes());
|
||||
page.extend_from_slice(&[0; 8]); // resolution
|
||||
page.extend_from_slice(&[0, 0, 0]); // flags, striping
|
||||
segment(0, 48, &page);
|
||||
let mut dictionary = vec![0, 0]; // arithmetic coding, template 0, no refinement
|
||||
dictionary.extend_from_slice(&[3, 0xff, 0xfd, 0xff, 2, 0xfe, 0xfe, 0xfe]); // AT pixels
|
||||
dictionary.extend_from_slice(&1u32.to_be_bytes()); // exported symbols
|
||||
dictionary.extend_from_slice(&2u32.to_be_bytes()); // new symbols
|
||||
dictionary.extend_from_slice(data);
|
||||
segment(1, 0, &dictionary);
|
||||
// Each copy is its own image XObject (5, 6, …), all drawn by the page.
|
||||
let images: Vec<usize> = (5..5 + copies).collect();
|
||||
let mut content: String = images.iter().map(|n| format!("q 20 0 0 20 5 5 cm /Im{n} Do Q ")).collect();
|
||||
content.push_str("1 0 0 rg 0 0 4 4 re f");
|
||||
let names: String = images.iter().map(|n| format!("/Im{n} {n} 0 R ")).collect();
|
||||
let mut pdf = format!("%PDF-1.7\n1 0 obj << /Type /Catalog /Pages 2 0 R >> endobj\n2 0 obj << /Type /Pages /Kids [3 0 R] /Count 1 >> endobj\n3 0 obj << /Type /Page /Parent 2 0 R /MediaBox [0 0 40 40] /Contents 4 0 R /Resources << /XObject << {names}>> >> >> endobj\n").into_bytes();
|
||||
pdf.extend_from_slice(format!("4 0 obj << /Length {} >> stream\n{content}\nendstream endobj\n", content.len()).as_bytes());
|
||||
for n in &images {
|
||||
pdf.extend_from_slice(
|
||||
format!("{n} 0 obj << /Type /XObject /Subtype /Image /Width 8 /Height 8 /BitsPerComponent 1 /ColorSpace /DeviceGray /Filter /JBIG2Decode /Length {} >> stream\n", jbig2.len())
|
||||
.as_bytes(),
|
||||
);
|
||||
pdf.extend_from_slice(&jbig2);
|
||||
pdf.extend_from_slice(b"\nendstream endobj\n");
|
||||
}
|
||||
pdf.extend_from_slice(b"trailer << /Root 1 0 R >>\n%%EOF\n");
|
||||
let (tx, rx) = std::sync::mpsc::channel();
|
||||
std::thread::spawn(move || {
|
||||
let mut r = PageRenderer::new(Arc::new(pdf), RenderConfig::default());
|
||||
let _ = tx.send(r.render(RenderRequest { page: 0, kind: RequestKind::Pixels, tile: None, scale: 1.0, tag: 0 }));
|
||||
});
|
||||
let page = rx
|
||||
.recv_timeout(std::time::Duration::from_secs(20))
|
||||
.unwrap_or_else(|_| panic!("{what}: a JBIG2 symbol dictionary must not stall the renderer"));
|
||||
assert!(page.error.is_none(), "{what}: {:?}", page.error);
|
||||
assert_eq!(&page.rgba[((38 * 40 + 1) * 4)..][..4], &[255, 0, 0, 255], "{what}: the rest of the page draws");
|
||||
}
|
||||
}
|
||||
|
||||
/// From the nightly `cargo xtask fuzz`: an embedded Type 1 font program holding a long run of
|
||||
/// integers. read-fonts 0.39 (through skrifa 0.42) looked ahead after every integer by
|
||||
/// parsing the next token, which looked ahead again, recursing through the whole run: a long
|
||||
|
|
|
|||
|
|
@ -10,7 +10,7 @@ the fix. Vendoring copyleft code is never allowed (plan/adr/0001).
|
|||
| hayro-interpret | 0.7.0 | Apache-2.0 OR MIT | (1) select `/AP /N` appearance states via `/AS` (checkboxes, radios, stateful icons) and honour the NoView flag; (2) Type3 Unicode fallback to `/Encoding` glyph names, then printable ASCII; (3) `Type3Glyph::advance_width`; (4) `InterpreterSettings::ocg_overrides` for viewer layer toggles; (5) soft masks whose group lacks `/CS` were dropped (Chrome gradient text); (6) paint nesting cap `MAX_PAINT_NESTING` for tiling patterns and Type 3 glyphs (self-referencing content overflowed the stack); (7) CID width ranges in `/W` and `/W2` clamped to `MAX_CID` (a `0 4294967295 w` range hung); (8) `InterpreterSettings::hide_comments` skips markup annotations but keeps widgets and links (View ▸ Hide all comments); (9) `image_size_ok`: images and masks over 2^28 pixels are not decoded (a fuzzed `/W 4294967295` stencil mask allocated 4 GiB); (10) `MAX_NESTED_PAINTS`: nested paints (forms, Type 3 glyphs and tiling patterns inside one another) share a per-page budget of 50,000 (a glyph showing eight glyphs of its own font, sixteen deep, hung); (11) depends on skrifa 0.47 instead of 0.42 (upstream read-fonts fixed a Type 1 tokenizer that recursed through runs of numbers: a fuzzed font hung or overflowed the stack), with the one API change (`Charset::glyph_id` returns `Option`); (12) annotation appearances whose `/BBox` or `/Matrix` collapses to a line or a point are skipped (mapping them onto `/Rect` divided by zero and handed the device infinite or NaN matrices); (13) a Highlight annotation without a usable normal appearance is drawn from its `/QuadPoints`, `/C` and `/CA` with Multiply blending, as pdfcraft-annot draws highlights (it was not drawn at all); (14) simple-font `/Widths` (`read_widths`): entries only for codes 0–255 (one entry per code below `/FirstChar` was allocated: `4294967295` asked for 34 GB), an entry that isn't a number leaves its code missing instead of ending the array, and a `/LastChar` below `/FirstChar` gives no widths instead of discarding the font for a standard one; (15) the appearance box is mapped onto `/Rect` corner to corner: the translation now accounts for the scale (an appearance whose box didn't start at the origin and differed in size from `/Rect` was drawn shifted); (16) a clip set by `W`/`W*` takes effect after whichever path-painting operator ends the path (`S`, `f`, `B`, … as well as `n`; ISO 32000-2 §8.5.4): it was applied only on `n`, so `re W* S` clipped nothing and the clip stayed pending for a later, unrelated `n`; (17) `InterpreterSettings::cancelled`: once set, interpretation stops before the next content operator (a render pool sets it when dropped, after an edit or on closing, so its renders in progress stop instead of running to the end for answers nobody can receive) | (1) `raster::tests::widget_appearance_states`; (2, 3) `xtask text-oracle` on corpus/pdfjs issue918; (4) `raster::tests::layer_override_hides_content`; (5) `raster::tests::soft_mask_without_group_colour_space_is_applied`; (6) `raster::tests::self_referencing_tiling_pattern_terminates`, `raster::tests::self_referencing_type3_glyph_terminates`; (7) `raster::tests::huge_cid_width_ranges_terminate`; (8) `raster::tests::hiding_comments_keeps_fields`; (9) `raster::tests::absurd_mask_and_fax_sizes_are_refused_before_decoding`; (10) `raster::tests::type3_glyphs_that_fan_out_into_themselves_finish`; (11) `raster::tests::type1_font_programs_with_long_runs_of_numbers_load_quickly`; (12) `raster::tests::degenerate_annotation_appearances_are_skipped`; (13) `raster::tests::highlights_without_an_appearance_are_drawn`; (14) `raster::tests::malformed_simple_font_widths_keep_later_widths_and_the_font`, `raster::tests::huge_simple_font_first_char_terminates`; (15) `raster::tests::annotation_appearance_box_maps_onto_rect`; (16) `raster::tests::clipping_path_applies_after_any_painting_operator`; (17) `raster::tests::dropping_the_pool_cancels_its_renders` |
|
||||
|
||||
| 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 |
|
||||
| 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); (3) a symbol dictionary's height classes are capped at one per new symbol and its export runs at `2 × symbols + 1`, each plus `EXTRA_EMPTY_STEPS` (1024): an empty height class or a zero-length export run doesn't advance, and past the end of its data the arithmetic decoder can decode them for ever (one changed byte hung the renderer) | (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; (3) `raster::tests::jbig2_symbol_dictionaries_that_never_advance_terminate` |
|
||||
| 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` |
|
||||
|
||||
|
|
|
|||
|
|
@ -29,6 +29,15 @@ const MAX_SYMBOLS: u32 = u16::MAX as u32;
|
|||
/// file decoding for minutes; real dictionaries hold about a million pixels.
|
||||
const MAX_DICTIONARY_PIXELS: u64 = 1 << 26;
|
||||
|
||||
/// PdfCraft patch: decoding steps that decode nothing, allowed beyond what a valid dictionary can
|
||||
/// need. A height class without a symbol and an export run of length 0 don't move their loops
|
||||
/// on, and past the end of its data the arithmetic decoder can decode them for ever: one changed
|
||||
/// byte in a symbol dictionary hung the renderer. A valid dictionary needs at most one height
|
||||
/// class per new symbol, and `2 × symbols + 1` export runs (they alternate between exported and
|
||||
/// not, so at most one zero-length run before each symbol's). Encoders don't write empty steps
|
||||
/// back to back; this tolerates a thousand.
|
||||
const EXTRA_EMPTY_STEPS: u64 = 1024;
|
||||
|
||||
/// Decode a symbol dictionary segment (7.4.2, 6.5).
|
||||
pub(crate) fn decode(
|
||||
header: &SymbolDictionaryHeader<'_>,
|
||||
|
|
@ -79,7 +88,14 @@ pub(crate) fn decode(
|
|||
}
|
||||
};
|
||||
|
||||
// PdfCraft patch: see `EXTRA_EMPTY_STEPS`.
|
||||
let mut height_classes_left = u64::from(num_new_symbols) + EXTRA_EMPTY_STEPS;
|
||||
|
||||
while ctx.symbols_decoded_count < num_new_symbols {
|
||||
if height_classes_left == 0 {
|
||||
bail!(SymbolError::Invalid);
|
||||
}
|
||||
height_classes_left -= 1;
|
||||
let height_class_delta =
|
||||
read_height_class_delta(&mut ctx)?.ok_or(SymbolError::UnexpectedOob)?;
|
||||
|
||||
|
|
@ -694,8 +710,14 @@ fn export_symbols(ctx: &mut SymbolDecodeContext<'_>) -> Result<Vec<Bitmap>> {
|
|||
let mut exported = Vec::with_capacity(ctx.header.num_exported_symbols as usize);
|
||||
let mut index: u32 = 0;
|
||||
let mut should_export = false;
|
||||
// PdfCraft patch: see `EXTRA_EMPTY_STEPS`.
|
||||
let mut runs_left = u64::from(total_symbols) * 2 + 1 + EXTRA_EMPTY_STEPS;
|
||||
|
||||
while index < total_symbols {
|
||||
if runs_left == 0 {
|
||||
bail!(SymbolError::Invalid);
|
||||
}
|
||||
runs_left -= 1;
|
||||
let run_length = read_run_length()?;
|
||||
|
||||
let end_index = index.checked_add(run_length).ok_or(OverflowError::Index)?;
|
||||
|
|
|
|||
Loading…
Reference in New Issue