Merge pull request #6695 from storytold/fix/sec-trace-gif-jpeg

Trace: refuse a GIF frame larger than the pixel ceiling before decoding

crates/trace/src/raster.rs

@@ -56,6 +56,37 @@ pub struct Raster {
     pub rgba: Vec<u8>,
 }
 
+/// The pixels the first frame of GIF `bytes` declares in its image descriptor, which the decoder
+/// allocates for before it crops the frame to the logical screen the dimension check reads. `None`
+/// when the bytes aren't a GIF or hold no frame header.
+fn gif_first_frame_pixels(bytes: &[u8]) -> Option<u64> {
+    if !bytes.starts_with(b"GIF8") {
+        return None;
+    }
+    let flags = *bytes.get(10)?;
+    let mut at = 13 + if flags & 0x80 != 0 { 3 << ((flags & 7) + 1) } else { 0 };
+    loop {
+        match *bytes.get(at)? {
+            // An extension: its data sub-blocks end at a zero length.
+            0x21 => {
+                at += 2;
+                loop {
+                    let len = usize::from(*bytes.get(at)?);
+                    at += 1 + len;
+                    if len == 0 {
+                        break;
+                    }
+                }
+            }
+            0x2C => {
+                let d = bytes.get(at + 5..at + 9)?;
+                return Some(u64::from(u16::from_le_bytes([d[0], d[1]])) * u64::from(u16::from_le_bytes([d[2], d[3]])));
+            }
+            _ => return None,
+        }
+    }
+}
+
 impl Raster {
     /// A raster from RGBA bytes, which must be exactly `width * height * 4` of them: any other
     /// length (bytes from a file, a plug-in or a message) is a [`TraceError::Decode`] error, not
@@ -85,7 +116,7 @@ impl Raster {
         if w == 0 || h == 0 {
             return Err(TraceError::Empty);
         }
-        if u64::from(w) * u64::from(h) > MAX_PIXELS {
+        if u64::from(w) * u64::from(h) > MAX_PIXELS || gif_first_frame_pixels(bytes).is_some_and(|p| p > MAX_PIXELS) {
             return Err(TraceError::TooLarge { width: w, height: h, max: MAX_PIXELS >> 20 });
         }
         let img = image::load_from_memory(bytes).map_err(decode_err)?.to_rgba8();
@@ -112,6 +143,9 @@ impl Raster {
         }
         // Decoded within the allocation the source limit needs; the web keeps file import's 512 MB.
         // A deeper image (16 bits a channel) that needs more fails with a decode error.
+        if gif_first_frame_pixels(bytes).is_some_and(|p| p > MAX_SOURCE_PIXELS) {
+            return Err(TraceError::TooLarge { width: w, height: h, max: MAX_SOURCE_PIXELS >> 20 });
+        }
         let mut reader = image::ImageReader::new(std::io::Cursor::new(bytes)).with_guessed_format().map_err(|e| TraceError::Decode(e.to_string()))?;
         let mut limits = image::Limits::default();
         limits.max_alloc = Some(if cfg!(target_arch = "wasm32") { 512 << 20 } else { MAX_SOURCE_PIXELS * 4 });

crates/trace/src/tests.rs

@@ -2908,3 +2908,15 @@ fn noise_keeps_a_kept_rare_colours_regions_but_not_its_specks() {
     assert_eq!(a, b);
     assert!(a.iter().all(|&v| v == 0));
 }
+
+/// A GIF whose logical screen is one pixel but whose frame declares 65535 × 65535 is refused before
+/// the decoder allocates for the frame.
+#[test]
+fn a_gif_frame_larger_than_its_screen_is_refused_before_decoding() {
+    let mut gif = b"GIF89a".to_vec();
+    gif.extend([1, 0, 1, 0, 0, 0, 0]); // screen 1 × 1, no color table
+    gif.extend([0x2C, 0, 0, 0, 0, 0xFF, 0xFF, 0xFF, 0xFF, 0]); // frame at 0,0 of 65535 × 65535
+    gif.extend([2, 1, 0, 0, 0x3B]);
+    assert!(matches!(Raster::decode(&gif), Err(TraceError::TooLarge { .. })), "{:?}", Raster::decode(&gif).err());
+    assert!(matches!(Raster::decode_to_fit(&gif), Err(TraceError::TooLarge { .. })));
+}