Merge pull request #2407 from dotCooCoo/fix/crop-image-trim-size-check

Crop Image: trim checks the image's RGBA size before decoding it

crates/engine/src/cmd/menucmds.rs

@@ -664,9 +664,18 @@ fn trim_image(s: &mut Session, id: NodeId, im: ImageObject) -> Result<Value> {
         return Ok(json!({ "id": id.0, "width": im.width, "height": im.height, "trimmed": false }));
     }
     // Print rasters run to gigabytes decoded: the same allowance opening them has.
-    let mut reader = image::ImageReader::new(std::io::Cursor::new(blob.bytes.as_slice()))
-        .with_guessed_format()
-        .map_err(|_| bad(C, "the image's pixels can't be read"))?;
+    let reader = || {
+        image::ImageReader::new(std::io::Cursor::new(blob.bytes.as_slice()))
+            .with_guessed_format()
+            .map_err(|_| bad(C, "the image's pixels can't be read"))
+    };
+    // The image's size as RGBA, read from its header, is held to that allowance before anything
+    // is decoded (#1717).
+    let (w, h) = reader()?.into_dimensions().map_err(|e| bad(C, format!("the image's pixels can't be read: {e}")))?;
+    if u64::from(w) * u64::from(h) > crate::cmd::fileio::MAX_RASTER_ALLOC / 4 {
+        return Err(bad(C, format!("an image of {w} × {h} pixels is too large to draw")));
+    }
+    let mut reader = reader()?;
     let mut limits = image::Limits::default();
     limits.max_alloc = Some(crate::cmd::fileio::MAX_RASTER_ALLOC);
     reader.limits(limits);

crates/engine/src/tests_linkspanel.rs

@@ -19,7 +19,7 @@ fn place_embedded(s: &mut Session, path: &str) -> NodeId {
 
 /// A PNG that says it is `w` × `h` 1-bit grey pixels (its header and no rows, which is all that
 /// placing and listing read).
-fn png_header(w: u32, h: u32) -> Vec<u8> {
+pub(crate) fn png_header(w: u32, h: u32) -> Vec<u8> {
     let mut out = b"\x89PNG\r\n\x1a\n".to_vec();
     let ihdr: Vec<u8> = [&b"IHDR"[..], &w.to_be_bytes(), &h.to_be_bytes(), &[1, 0, 0, 0, 0]].concat();
     for chunk in [&ihdr[..], b"IDAT\x78\x9c", b"IEND"] {

crates/engine/src/tests_menucmds.rs

@@ -2423,6 +2423,24 @@ fn crop_image_trim_cuts_to_the_opaque_pixels_as_they_are() {
     assert_eq!(bounds(&s, img), b);
 }
 
+#[test]
+fn crop_image_trim_checks_the_rgba_size_before_decoding() {
+    // 1-bit gray PNGs, header only: their samples are within the decoder's limit, and as RGBA
+    // (4 bytes a pixel) they are just over or just within the 2 GB allowance.
+    let trim = |w: u32, h: u32| {
+        let png = super::tests_linkspanel::png_header(w, h);
+        let mut s = Session::new();
+        s.execute("document.open", &json!({"name": "wall.png", "dataBase64": vectorcraft_format::base64_encode(&png)})).unwrap();
+        s.execute("select.all", &json!({})).unwrap();
+        s.execute("object.cropImage", &json!({"trim": true})).unwrap_err().to_string()
+    };
+    let over = trim(23_171, 23_170);
+    assert!(over.contains("an image of 23171 × 23170 pixels is too large to draw"), "{over}");
+    // One column fewer is within the allowance: it is decoded, and its missing rows can't be read.
+    let within = trim(23_170, 23_170);
+    assert!(within.contains("the image's pixels can't be read"), "{within}");
+}
+
 #[test]
 fn crop_image_trim_leaves_a_cmyk_image_alone() {
     // CMYK images keep their ink amounts and have no alpha: nothing to cut, and not an error.