From 1f801bf4c987ebff22f6aa868ec926e1c664e316 Mon Sep 17 00:00:00 2001 From: Grzegorz Martens Date: Sun, 25 Oct 2026 14:23:23 +0100 Subject: [PATCH] M11: redaction bounds a fax image's decoding; a test for numbers inside /DecodeParms given by reference - Redacting a CCITT image went through decoded_image, which undoes the filters before the fax decoder up to MAX_DECODED, so a Flate bomb in front of a fax image cost a gigabyte while every other image stops at twice its raster. Document::decoded_image_within takes a bound, and redaction gives a fax image's code about a byte per pixel. A fax image cut short, or behind a bomb, is removed whole; a busy one whose code is bigger than twice its raster is still cleared. - Every image is now decoded through decoded_image_within, which reads /DecodeParms through Document::with_direct_parms: given by reference, the numbers inside (/Predictor 7 0 R) included. That reading was first written here and has moved down to #4304. The predictor test now covers the dictionary, an array's entry and the number itself given by reference; read without them, the cleared bytes drift off the covered pixel and the covered sample stays in the copy. From reviews with the pdfcraft-review skill. Co-Authored-By: Claude Opus 5.5 --- crates/cos/src/document/objects.rs | 14 +++-- crates/redact/README.md | 11 ++-- crates/redact/src/image.rs | 24 +++++---- crates/redact/src/tests.rs | 82 +++++++++++++++++++++++------- docs/pdf-spec-parity.md | 5 +- 5 files changed, 97 insertions(+), 39 deletions(-) diff --git a/crates/cos/src/document/objects.rs b/crates/cos/src/document/objects.rs index 06d488d7c2..4c03d04243 100644 --- a/crates/cos/src/document/objects.rs +++ b/crates/cos/src/document/objects.rs @@ -233,9 +233,17 @@ impl Document { /// whatever `/Rows` says, within [`MAX_DECODED`] bytes; damaged data gives fewer, so a caller /// checks it has the rows it needs. pub fn decoded_image(&self, s: &Stream) -> Result, CosError> { + self.decoded_image_within(s, MAX_DECODED) + } + + /// [`Document::decoded_image`] for an image whose decoding has a tighter bound than + /// [`MAX_DECODED`]: no stage, the filters before a fax decoder included, may produce more than + /// `max` bytes. + pub fn decoded_image_within(&self, s: &Stream, max: usize) -> Result, CosError> { + let max = max.min(MAX_DECODED); let s = &*self.with_direct_parms(s); let chain = s.readable_filters().map_err(|e| CosError::Filter(e.to_string()))?; - let Some(((pdfcraft_filters::Filter::CcittFax, _), before)) = chain.split_last() else { return s.decoded() }; + let Some(((pdfcraft_filters::Filter::CcittFax, _), before)) = chain.split_last() else { return s.decoded_within(max) }; // Its place in /Filter, /Crypt included, pairs it with its /DecodeParms entry (§7.4.1): the // last filter but a /Crypt, which `filters` leaves out wherever it stands. let index = match s.dict.get(b"Filter") { @@ -270,11 +278,11 @@ impl Document { } let height = s.dict.int(b"Height").and_then(|h| u32::try_from(h).ok()).unwrap_or(0); let stride = usize::try_from(p.columns).unwrap_or(usize::MAX).div_ceil(8).max(1); - let rows = u32::try_from(MAX_DECODED / stride).unwrap_or(u32::MAX).min(height); + let rows = u32::try_from(max / stride).unwrap_or(u32::MAX).min(height); // The filters before it, such as ASCII85Decode, are undone first. let data = match before { [] => Cow::Borrowed(&s.raw[..]), - _ => Cow::Owned(pdfcraft_filters::decode_tolerant(before, &s.raw, MAX_DECODED).map_err(|e| CosError::Filter(e.to_string()))?.0), + _ => Cow::Owned(pdfcraft_filters::decode_tolerant(before, &s.raw, max).map_err(|e| CosError::Filter(e.to_string()))?.0), }; pdfcraft_filters::decode_ccitt(&data, &p, rows).map_err(|e| CosError::Filter(e.to_string())) } diff --git a/crates/redact/README.md b/crates/redact/README.md index 2b540927d1..75648b3cc4 100644 --- a/crates/redact/README.md +++ b/crates/redact/README.md @@ -19,11 +19,12 @@ clear_marks(&mut doc, None)?; // remove marks without app inside; - **images**: fully covered → removed; partly covered → a copy with the covered pixels cleared (8/16-bit and 1/2/4-bit images, image masks; Flate/LZW/RL/A85/AHx, and CCITT fax as the last - filter, decoded through `Document::decoded_image`; `/DecodeParms` given by reference count). - Codecs PdfCraft can't decode (DCT, JPX, JBIG2) are removed whole (fail-closed); inline images - under an area are removed. Images with separate `/SMask` or `/Mask` data, and images whose - data decodes to more than twice their declared raster (plus a byte per row and 4 KiB), are - removed whole; cleared copies drop `/Alternates` so alternate pixels cannot survive; + filter, decoded through `Document::decoded_image_within`; `/DecodeParms` given by reference, + numbers inside included, count). Codecs redaction doesn't decode (DCT, JPX, JBIG2) are removed + whole (fail-closed); inline images under an area are removed. Images with separate `/SMask` + or `/Mask` data, and images whose data decodes to more than twice their declared raster (plus + a byte per row and 4 KiB; for a fax image's code, about a byte per pixel), are removed whole; + cleared copies drop `/Alternates` so alternate pixels cannot survive; - **vectors**: covered paths are removed; partly covered paths and shadings are clipped so nothing paints inside the areas; - **form XObjects**: rewritten recursively into new objects (pages sharing the original keep it); diff --git a/crates/redact/src/image.rs b/crates/redact/src/image.rs index 2d22cc8174..6568e8015a 100644 --- a/crates/redact/src/image.rs +++ b/crates/redact/src/image.rs @@ -1,7 +1,7 @@ //! Clearing the pixels of an image under redaction regions. The image is decoded, every pixel //! whose footprint intersects a region is set to zero (no ink for image masks), and the result is //! written as a new Flate image; the original object is untouched (other pages may use it). -//! CCITT fax images decode through `Document::decoded_image`. Images whose codec PdfCraft can't +//! CCITT fax images decode through `Document::decoded_image_within`. Images whose codec PdfCraft can't //! decode (DCT, JPX, JBIG2) return `None`, and the caller removes the whole image instead; that //! is fail-closed. @@ -46,16 +46,13 @@ fn decode_and_clear(doc: &Document, s: &Stream, ctm: &Matrix, rects: &[[f64; 4]] if s.dict.contains(b"SMask") || s.dict.contains(b"Mask") { return None; } - // `Stream::decoded*` reads `/DecodeParms` only when it is direct: a predictor given by - // reference would be skipped, and the cleared bytes would then miss the covered pixels. - let s = &*doc.with_direct_parms(s); let supported = |n: &[u8]| { matches!( n, b"FlateDecode" | b"Fl" | b"LZWDecode" | b"LZW" | b"ASCII85Decode" | b"A85" | b"ASCIIHexDecode" | b"AHx" | b"RunLengthDecode" | b"RL" ) }; - // A CCITT fax image (its last filter) decodes through `Document::decoded_image`, after the + // A CCITT fax image (its last filter) decodes through `Document::decoded_image_within`, after the // filters before it, so a scan stored as Group 4 is cleared like any other image. let fax = |f: &Object| f.as_name().is_some_and(|n| matches!(n, b"CCITTFaxDecode" | b"CCF")); let readable = |f: &Object| f.as_name().is_some_and(supported); @@ -89,11 +86,18 @@ fn decode_and_clear(doc: &Document, s: &Stream, ctm: &Matrix, rects: &[[f64; 4]] let row_bits = row.checked_mul(8)?; bytes.checked_mul(8)?; // Decode no further than the raster can need: a filter stage may legitimately produce up to - // twice it (ASCIIHex text under Flate) plus a PNG predictor's byte per row. Data past that - // can only be a decompression bomb; the image is then removed whole. A fax image decodes to - // at most its /Height rows (the filters before it within `MAX_DECODED`). - let allowance = bytes.checked_add(h)?.checked_mul(2)?.checked_add(4096)?; - let mut data = if is_fax { doc.decoded_image(s).ok()? } else { s.decoded_within(allowance).ok()? }; + // twice it (ASCIIHex text under Flate) plus a PNG predictor's byte per row. A fax image's + // code, which the filters before its decoder produce, takes at most about a byte per pixel + // plus each row's end. Data past that can only be a decompression bomb; the image is then + // removed whole. + let allowance = if is_fax { + w.checked_mul(h)?.checked_add(h.checked_mul(4)?)?.checked_add(4096)? + } else { + bytes.checked_add(h)?.checked_mul(2)?.checked_add(4096)? + }; + // Through the document, which reads `/DecodeParms` given by reference (`Stream::decoded*` + // alone would skip such a predictor, and the cleared bytes would then miss the covered pixels). + let mut data = doc.decoded_image_within(s, allowance).ok()?; if data.len() < bytes { return None; } diff --git a/crates/redact/src/tests.rs b/crates/redact/src/tests.rs index 760d0727cf..9400391bb5 100644 --- a/crates/redact/src/tests.rs +++ b/crates/redact/src/tests.rs @@ -550,29 +550,31 @@ fn redaction_clears_pixels_intersected_without_covering_their_centres() { } /// An image whose PNG predictor is given by reference has its covered pixel cleared, and only -/// that one: `Stream::decoded` reads only a direct `/DecodeParms`, so the row tags stayed in the -/// data, the cleared byte drifted off the covered pixel and the covered sample survived. +/// that one, whichever part is the reference: the dictionary, an array's entry, or the number +/// itself. `Stream::decoded` reads only a direct `/DecodeParms` with direct numbers, so the row +/// tags stayed in the data, the cleared byte drifted off the covered pixel and the covered sample +/// survived. #[test] fn an_image_whose_predictor_is_given_by_reference_has_the_covered_pixel_cleared() { // Two rows of four grey samples, each behind a PNG "None" row tag. let predicted = pdfcraft_filters::encode_flate(&[0, 10, 20, 30, 40, 0, 50, 60, 70, 80]); - let mut doc = one_page( - b"q 40 0 0 20 0 0 cm /Im Do Q", - "/XObject << /Im 7 0 R >>", - vec![ - stream( - "/Type /XObject /Subtype /Image /Width 4 /Height 2 /ColorSpace /DeviceGray /BitsPerComponent 8 /Filter /FlateDecode /DecodeParms 8 0 R", - predicted, - ), - b"<< /Predictor 15 /Colors 1 /BitsPerComponent 8 /Columns 4 >>".to_vec(), - ], - ); - // The top row's second sample. - mark(&mut doc, 0, &[[12.0, 12.0, 18.0, 18.0]], ""); - let report = apply(&mut doc, None).unwrap(); - assert_eq!(report.images_cleared, 1); - let saved = Document::open(Arc::new(write_full(&doc, &SaveOptions::default()).unwrap())).unwrap(); - assert_eq!(named_image(&saved, 0, b"PCRedacted1"), Some(vec![10, 0, 30, 40, 50, 60, 70, 80])); + let parms = "/Predictor 15 /Colors 1 /BitsPerComponent 8 /Columns 4"; + for (decode_parms, object8) in [ + ("8 0 R".to_owned(), format!("<< {parms} >>")), + ("[8 0 R]".to_owned(), format!("<< {parms} >>")), + ("<< /Predictor 8 0 R /Colors 1 /BitsPerComponent 8 /Columns 4 >>".to_owned(), "15".to_owned()), + ] { + let image = format!( + "/Type /XObject /Subtype /Image /Width 4 /Height 2 /ColorSpace /DeviceGray /BitsPerComponent 8 /Filter /FlateDecode /DecodeParms {decode_parms}" + ); + let mut doc = one_page(b"q 40 0 0 20 0 0 cm /Im Do Q", "/XObject << /Im 7 0 R >>", vec![stream(&image, &predicted), object8.into_bytes()]); + // The top row's second sample. + mark(&mut doc, 0, &[[12.0, 12.0, 18.0, 18.0]], ""); + let report = apply(&mut doc, None).unwrap(); + assert_eq!(report.images_cleared, 1, "{decode_parms}"); + let saved = Document::open(Arc::new(write_full(&doc, &SaveOptions::default()).unwrap())).unwrap(); + assert_eq!(named_image(&saved, 0, b"PCRedacted1"), Some(vec![10, 0, 30, 40, 50, 60, 70, 80]), "{decode_parms}"); + } } /// A CCITT fax image (a scan the optimizer or Create PDF stored as Group 4) has its covered @@ -609,6 +611,48 @@ fn a_fax_image_has_its_covered_pixels_cleared_instead_of_being_removed() { } } +/// A fax image that can't be decoded in full is removed whole, as any other image is: one whose +/// code is cut short, and one behind a filter that inflates past what a code for its pixels can +/// take (a decompression bomb, which `decoded_image` alone would undo up to a gigabyte). +#[test] +fn a_fax_image_that_cannot_be_decoded_in_full_is_removed_whole() { + let g4 = pdfcraft_filters::encode_g4(&[0xFF; 4], 16, 2).unwrap(); + // A busy picture's code, cut off in its first row. + let busy = pdfcraft_filters::encode_g4(&[0xAA, 0xAA, 0x55, 0x55], 16, 2).unwrap(); + let short = busy.get(..1).unwrap().to_vec(); + // The real code, then four megabytes of padding the fax decoder would never read. + let mut padded = g4.clone(); + padded.resize(4 << 20, 0); + let bomb = pdfcraft_filters::encode_flate(&padded); + let image = "/Type /XObject /Subtype /Image /Width 16 /Height 2 /ColorSpace /DeviceGray /BitsPerComponent 1"; + let parms = "<< /K -1 /Columns 16 /Rows 2 >>"; + for (filters, data) in [("/CCITTFaxDecode /DecodeParms", short), ("[/FlateDecode /CCITTFaxDecode] /DecodeParms [null", bomb)] { + let close = if filters.ends_with("[null") { "]" } else { "" }; + let dict = format!("{image} /Filter {filters} {parms}{close}"); + let mut doc = one_page(b"q 160 0 0 20 0 0 cm /Im Do Q", "/XObject << /Im 7 0 R >>", vec![stream(&dict, data)]); + mark(&mut doc, 0, &[[0.0, 12.0, 75.0, 18.0]], ""); + let report = apply(&mut doc, None).unwrap(); + assert_eq!((report.images_cleared, report.images_removed), (0, 1), "{dict}"); + } +} + +/// A busy fax image is cleared, though its code is bigger than twice its raster (the bound for +/// other images): a code takes up to about a byte per pixel, and the fax image gets that much. +#[test] +fn a_busy_fax_image_behind_another_filter_is_cleared() { + // A 256 × 256 checkerboard, the worst case for a fax code. + let rows: Vec = (0..256).flat_map(|y| [if y % 2 == 0 { 0xAA } else { 0x55 }; 32]).collect(); + let g4 = pdfcraft_filters::encode_g4(&rows, 256, 256).unwrap(); + let raster = 32 * 256; + assert!(g4.len() > (raster + 256) * 2 + 4096, "{} bytes: within the bound for other images", g4.len()); + let dict = "/Type /XObject /Subtype /Image /Width 256 /Height 256 /ColorSpace /DeviceGray /BitsPerComponent 1 \ + /Filter [/FlateDecode /CCITTFaxDecode] /DecodeParms [null << /K -1 /Columns 256 /Rows 256 >>]"; + let mut doc = one_page(b"q 256 0 0 256 0 0 cm /Im Do Q", "/XObject << /Im 7 0 R >>", vec![stream(dict, pdfcraft_filters::encode_flate(&g4))]); + mark(&mut doc, 0, &[[0.0, 0.0, 10.0, 10.0]], ""); + let report = apply(&mut doc, None).unwrap(); + assert_eq!((report.images_cleared, report.images_removed), (1, 0)); +} + /// Apply Redactions gives the page its own pruned /Resources, but the dictionary it drew through /// before (inherited from the /Pages node, or shared with another page) still named the removed /// image and form, so a full-rewrite save kept them. diff --git a/docs/pdf-spec-parity.md b/docs/pdf-spec-parity.md index 6ed9eba7c1..b68c5b641d 100644 --- a/docs/pdf-spec-parity.md +++ b/docs/pdf-spec-parity.md @@ -1,6 +1,6 @@ # PDF specification and standards parity -> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-26 · **Change:** minor (§7.4: CCITT decoding in `pdfcraft-filters`; earlier, §7.4: CCITT Group 4 encoding in the Optimizer) · **Target:** Adobe Acrobat Pro (Acrobat DC, continuous track 26.002.21931, macOS) +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-26 · **Change:** minor (§7.4: redaction decodes CCITT fax images; earlier, §7.4: CCITT decoding in `pdfcraft-filters`) · **Target:** Adobe Acrobat Pro (Acrobat DC, continuous track 26.002.21931, macOS) How much of ISO 32000-2 (PDF 2.0) and the PDF subset standards PdfCraft implements, set against what Acrobat Pro implements. This is PdfCraft's equivalent of a "geometry" or "codec" checklist: @@ -21,7 +21,7 @@ are borrowed; PDF 2.0 additions and most subset standards are missing. **150–2 |---|---|---|---|---| | §7.2–7.5 | Syntax, objects, file structure, xref tables and streams, object streams, hybrid files, incremental updates | full | **own**, tolerant, repairs damaged files | `crates/cos`; corpus 958 files; fuzzed nightly | | §7.4 | Stream filters: Flate, LZW, ASCII85, ASCIIHex, RunLength, predictors | full | **own** | `crates/filters` | -| §7.4 | Image filters: DCT, JPX, JBIG2, CCITTFax | full | **borrowed** (decode); CCITT Group 4 encoding | `core.image-filters` partial; own codecs M1.1/M1.2; CCITT decoding in `pdfcraft-filters` (`decode_ccitt`, through `hayro-ccitt`) for export and the Optimizer, which encodes Group 4 (`optimize.mono-compression`); no JBIG2 encoding | +| §7.4 | Image filters: DCT, JPX, JBIG2, CCITTFax | full | **borrowed** (decode); CCITT Group 4 encoding | `core.image-filters` partial; own codecs M1.1/M1.2; CCITT decoding in `pdfcraft-filters` (`decode_ccitt`, through `hayro-ccitt`) for export, redaction and the Optimizer, which encodes Group 4 (`optimize.mono-compression`); no JBIG2 encoding | | §7.5.8 | Linearization | R W | R only | `core.linearization` planned | | §7.6.4 | Standard security handler R2–R6 (RC4 40/128, AES-128, AES-256), crypt filters, `/EncryptMetadata`, SASLprep | full | **own**, read and write | `crates/crypt`; written files open in Acrobat DC since #836 (#774: Acrobat needs the catalog outside object streams) | | §7.6.5 | Public-key security handler | full | none | `core.open-public-key`, `protect.certificate-encryption` planned | @@ -130,6 +130,7 @@ are borrowed; PDF 2.0 additions and most subset standards are missing. **150–2 | Date | Change | Summary | |---|---|---| +| 2026-10-26 | minor | §7.4: redaction clears pixels in CCITT fax images through the CCITT decoder, instead of removing them whole | | 2026-10-26 | minor | §7.4: CCITT decoding (`hayro-ccitt`) in `pdfcraft-filters`, for export and the Optimizer | | 2026-10-26 | minor | §7.4: CCITT Group 4 encoding (`fax`) in the Optimizer | | 2026-10-14 | minor | §12.4.4: `/Trans` and `/Dur` read into `DocInfo::transitions` and reported by `doc_info` |