Merge pull request #6668 from storytold/fix/sec-pdf-scan-budget

PDF: note fonts once per walk, place repeated rasters without converting again, check image headers before embedding

crates/pdf/src/export/images.rs

@@ -254,10 +254,20 @@ pub(super) fn png_8_bit(bytes: &[u8]) -> Option<Vec<u8>> {
     Some(png)
 }
 
+/// Most pixels an image may have to be embedded: twice what the app reads (64 million), so every image
+/// it holds is still written.
+const MAX_EMBED_PIXELS: u64 = 1 << 27;
+
 /// Image `blob` as it is: PNG, JPEG, GIF and WebP through the PDF writer (JPEG data unchanged),
 /// other formats decoded. Without `sixteen_bit` (a file before PDF 1.5), a PNG of 16 bits a
 /// sample is written at 8 bits ([`png_8_bit`]).
 pub(super) fn embed(blob: &vectorcraft_doc::ImageBlob, interpolate: bool, sixteen_bit: bool) -> Option<Image> {
+    // The header's size first: the writer's decoders allocate what it declares, and the encoded
+    // bytes of an image that is far larger than any the app reads may be a few kilobytes.
+    let size = image::ImageReader::new(std::io::Cursor::new(blob.bytes.as_slice())).with_guessed_format().ok()?.into_dimensions();
+    if size.is_ok_and(|(w, h)| u64::from(w) * u64::from(h) > MAX_EMBED_PIXELS) {
+        return None;
+    }
     let bytes = blob.bytes.as_ref().clone();
     let direct = match blob.mime.as_str() {
         "image/png" if !sixteen_bit && png_16_bit(&bytes) => png_8_bit(&bytes).and_then(|png| Image::from_png(png.into(), interpolate).ok()),
@@ -281,6 +291,18 @@ mod tests {
     use vectorcraft_doc::{Document, ImageBlob, ImageObject};
     use vectorcraft_geom::Affine;
 
+    #[test]
+    fn an_image_whose_header_declares_far_more_pixels_than_the_app_reads_is_left_out() {
+        // A real 8 × 8 PNG whose IHDR (width, height at 16..24) claims 65535 × 65535, with its CRC fixed.
+        let mut png = vec![];
+        image::DynamicImage::ImageRgb8(image::RgbImage::new(8, 8)).write_to(&mut std::io::Cursor::new(&mut png), image::ImageFormat::Png).unwrap();
+        png[16..24].copy_from_slice(&[0, 0, 0xFF, 0xFF, 0, 0, 0xFF, 0xFF]);
+        let crc =
+            !png[12..29].iter().fold(!0u32, |c, b| (0..8).fold(c ^ u32::from(*b), |c, _| if c & 1 == 1 { 0xEDB8_8320 ^ (c >> 1) } else { c >> 1 }));
+        png[29..33].copy_from_slice(&crc.to_be_bytes());
+        assert!(super::embed(&ImageBlob::png(png), false, true).is_none());
+    }
+
     #[test]
     fn colorized_image_caches_rejected_images() {
         let mut doc = Document::new(10.0, 10.0);

crates/pdf/src/import/builder.rs

@@ -26,6 +26,7 @@ impl<'p> Builder<'p> {
             actual: Default::default(),
             images: HashMap::new(),
             image_keys: HashMap::new(),
+            raster_scales: HashMap::new(),
             cmyk_keys: HashSet::new(),
             separation_paints: HashMap::new(),
             opaque: HashSet::new(),

crates/pdf/src/import/device.rs

@@ -285,6 +285,12 @@ impl<'a> Device<'a> for Builder<'_> {
                     self.add_image(key, || Some((ImageBlob::new("image/jpeg", bytes), w, h)), transform);
                     return;
                 }
+                // Drawn before: the image is there already, so its pixels aren't converted again.
+                if let Some(sf) = self.raster_scales.get(&key).copied().filter(|_| self.image_keys.contains_key(&key)) {
+                    let xf = transform * Affine::scale_non_uniform(sf.0 as f64, sf.1 as f64);
+                    self.add_image(key, || None, xf);
+                    return;
+                }
                 let mut decoded: Option<Decoded> = None;
                 r.with_rgba(
                     |img, alpha| {
@@ -303,6 +309,7 @@ impl<'a> Device<'a> for Builder<'_> {
                     self.warn("an image could not be decoded and was skipped");
                     return;
                 };
+                self.raster_scales.insert(key, sf);
                 let xf = transform * Affine::scale_non_uniform(sf.0 as f64, sf.1 as f64);
                 self.add_image(key, || rgba_png(rgba, w, h).map(|png| (ImageBlob::new("image/png", png), w, h)), xf);
             }

crates/pdf/src/import/state.rs

@@ -85,6 +85,9 @@ pub(super) struct Builder<'p> {
     pub(super) actual: super::actual_text::ActualText,
     pub(super) images: HashMap<String, ImageBlob>,
     pub(super) image_keys: HashMap<u128, (String, u32, u32)>,
+    /// The scale factors of each decoded raster by its key: drawn again, it is placed from them
+    /// without its pixels being decoded and converted again.
+    pub(super) raster_scales: HashMap<u128, (f32, f32)>,
     /// The images kept with their CMYK samples ([`crate::import_image`]).
     pub(super) cmyk_keys: HashSet<u128>,
     /// Images in a Separation space, by key: the paint of their ink, or `None` when they open in

crates/pdf/src/import_scan.rs

@@ -455,12 +455,15 @@ fn blank_tail(mut bytes: &[u8]) -> bool {
     }
 }
 
-struct Walker<'w, 'p> {
+pub(crate) struct Walker<'w, 'p> {
     ocgs: &'w mut Ocgs,
     /// The groups that are off for the interpreter (none once [`all_on`] turned them on).
     off: HashSet<ObjectIdentifier>,
     xref: &'p craft_pdf::syntax::xref::XRef,
-    fonts: &'w mut FontNames,
+    pub(crate) fonts: &'w mut FontNames,
+    /// The font dictionaries ([`Dict::cache_key`]) whose fonts were noted: a form drawn again with
+    /// the same resources isn't read for its fonts again.
+    pub(crate) font_dicts: HashSet<u128>,
     visible: Vec<bool>,
     ops: usize,
     max_ops: usize,
@@ -632,6 +635,9 @@ impl Walker<'_, '_> {
     }
 
     fn note_fonts(&mut self, res: &Resources<'_>) {
+        if !self.font_dicts.insert(res.fonts.cache_key()) {
+            return;
+        }
         for key in res.fonts.keys() {
             if let Some(f) = res.fonts.get::<Dict<'_>>(key.as_ref())
                 && let Some(name) = font_name(&f)
@@ -1049,13 +1055,20 @@ pub(crate) fn scan_page(page: &Page<'_>, ocgs: &mut Ocgs, all_on: bool, fonts: &
     if scan.cut { Err(incomplete_content()) } else { Ok(scan) }
 }
 
-fn walk_page<'w, 'p>(page: &Page<'p>, ocgs: &'w mut Ocgs, all_on: bool, fonts: &'w mut FontNames, max_form_bytes: usize) -> Walker<'w, 'p> {
+pub(crate) fn walk_page<'w, 'p>(
+    page: &Page<'p>,
+    ocgs: &'w mut Ocgs,
+    all_on: bool,
+    fonts: &'w mut FontNames,
+    max_form_bytes: usize,
+) -> Walker<'w, 'p> {
     let off = if all_on { HashSet::new() } else { ocgs.off.clone() };
     let mut w = Walker {
         ocgs,
         off,
         xref: page.xref(),
         fonts,
+        font_dicts: HashSet::new(),
         visible: vec![],
         ops: 0,
         max_ops: max_ops(),

crates/pdf/src/tests/colorize.rs

@@ -493,3 +493,21 @@ fn a_duotone_devicen_image_still_opens_as_rgb() {
     assert_eq!(blobs.len(), 1);
     assert!(!blobs[0].0.is_cmyk() && !blobs[0].1, "RGB, not colorized");
 }
+
+/// A raster drawn again is placed from what the first drawing found, as the same image at its own
+/// size, without its pixels being converted again.
+#[test]
+fn a_raster_drawn_twice_is_one_image_placed_twice() {
+    let content = "q 80 0 0 10 10 50 cm /Im1 Do Q q 40 0 0 10 10 20 cm /Im1 Do Q";
+    let r = separation_report("/DeviceRGB", 2, 8, "", "FF0000 00FF00", &[], content);
+    let mut placed = vec![];
+    r.document.walk(|n| {
+        if let NodeKind::Image(im) = &n.kind {
+            placed.push((im.key.clone(), im.xf.as_coeffs()[0]));
+        }
+    });
+    assert_eq!(placed.len(), 2, "{placed:?}");
+    assert_eq!(placed[0].0, placed[1].0);
+    assert!(placed[0].1 > placed[1].1 * 1.9 && placed[0].1 < placed[1].1 * 2.1, "{placed:?}");
+    assert_eq!(r.document.images.len(), 1);
+}

crates/pdf/src/tests/import_fidelity.rs

@@ -474,6 +474,22 @@ fn a_composite_font_is_noted_by_its_cid_font_name() {
     assert_eq!(noted("<< /Type /Font /Subtype /Type1 /BaseFont /Vctest-Identity-H >>"), ["Vctest-Identity-H"]);
 }
 
+/// A form drawn again and again without resources of its own reads the page's font dictionary once,
+/// not at every drawing; the fonts are noted all the same.
+#[test]
+fn a_form_drawn_many_times_reads_its_font_resources_once() {
+    let form_no = first_extra(1);
+    let form = stream("/Type /XObject /Subtype /Form /BBox [0 0 100 100]", "0 0 1 rg 50 50 30 30 re f");
+    let resources = format!("/Font << /F1 << /Type /Font /Subtype /Type1 /BaseFont /Helvetica >> >> /XObject << /X0 {form_no} 0 R >>");
+    let page = PdfPage { resources, ..PdfPage::new(100.0, 100.0, &format!("BT /F1 24 Tf 10 30 Td (Hi) Tj ET {}", "/X0 Do ".repeat(500))) };
+    let pdf = craft_pdf::syntax::Pdf::new(pdf_with(&[page], &[&form], None)).unwrap();
+    let mut fonts = crate::import_scan::FontNames::new();
+    let mut ocgs = crate::import_scan::Ocgs::read(&pdf);
+    let walk = crate::import_scan::walk_page(&pdf.pages()[0], &mut ocgs, false, &mut fonts, pdf.decode_limits().max_decoded_stream());
+    assert_eq!(walk.font_dicts.len(), 1);
+    assert_eq!(walk.fonts.len(), 1);
+}
+
 /// A value of the page's font dictionary that the reader can't parse (a name with a broken `#`
 /// escape, a number with a stray `-`) is left out when the page's fonts are noted, and the fonts
 /// after it are noted.