Fix setting GIF limits, implement limits for GIF animations (#2090)

src/codecs/gif.rs

@@ -60,7 +60,7 @@ impl<R: Read> GifDecoder<R> {
 
         Ok(GifDecoder {
             reader: decoder.read_info(r).map_err(ImageError::from_decoding)?,
-            limits: Limits::default(),
+            limits: Limits::no_limits(),
         })
     }
 
@@ -112,6 +112,17 @@ impl<'a, R: 'a + Read> ImageDecoder<'a> for GifDecoder<R> {
         ))
     }
 
+    fn set_limits(&mut self, limits: Limits) -> ImageResult<()> {
+        limits.check_support(&crate::io::LimitSupport::default())?;
+
+        let (width, height) = self.dimensions();
+        limits.check_dimensions(width, height)?;
+
+        self.limits = limits;
+
+        Ok(())
+    }
+
     fn read_image(mut self, buf: &mut [u8]) -> ImageResult<()> {
         assert_eq!(u64::try_from(buf.len()), Ok(self.total_bytes()));
 
@@ -222,23 +233,23 @@ struct GifFrameIterator<R: Read> {
     width: u32,
     height: u32,
 
-    non_disposed_frame: ImageBuffer<Rgba<u8>, Vec<u8>>,
+    non_disposed_frame: Option<ImageBuffer<Rgba<u8>, Vec<u8>>>,
+    limits: Limits,
 }
 
 impl<R: Read> GifFrameIterator<R> {
     fn new(decoder: GifDecoder<R>) -> GifFrameIterator<R> {
         let (width, height) = decoder.dimensions();
+        let limits = decoder.limits.clone();
 
         // intentionally ignore the background color for web compatibility
 
-        // create the first non disposed frame
-        let non_disposed_frame = ImageBuffer::from_pixel(width, height, Rgba([0, 0, 0, 0]));
-
         GifFrameIterator {
             reader: decoder.reader,
             width,
             height,
-            non_disposed_frame,
+            non_disposed_frame: None,
+            limits,
         }
     }
 }
@@ -247,6 +258,30 @@ impl<R: Read> Iterator for GifFrameIterator<R> {
     type Item = ImageResult<animation::Frame>;
 
     fn next(&mut self) -> Option<ImageResult<animation::Frame>> {
+        fn limits_reserve_buffer(limits: &mut Limits, width: u32, height: u32) -> ImageResult<()> {
+            limits.check_dimensions(width, height)?;
+            // cannot overflow because width/height are u16 in the actual GIF format,
+            // so this cannot exceed 64GB which easily fits into a u64
+            let in_memory_size = width as u64 * height as u64 * 4;
+            limits.reserve(in_memory_size)
+        }
+
+        // Allocate the buffer for the previous frame.
+        // This is done here and not in the constructor because
+        // the constructor cannot return an error when the allocation limit is exceeded.
+        if self.non_disposed_frame.is_none() {
+            if let Err(e) = limits_reserve_buffer(&mut self.limits, self.width, self.height) {
+                return Some(Err(e));
+            }
+            self.non_disposed_frame = Some(ImageBuffer::from_pixel(
+                self.width,
+                self.height,
+                Rgba([0, 0, 0, 0]),
+            ));
+        }
+        // Bind to a variable to avoid repeated `.unwrap()` calls
+        let non_disposed_frame = self.non_disposed_frame.as_mut().unwrap();
+
         // begin looping over each frame
 
         let frame = match self.reader.next_frame_info() {
@@ -261,6 +296,17 @@ impl<R: Read> Iterator for GifFrameIterator<R> {
             Err(err) => return Some(Err(ImageError::from_decoding(err))),
         };
 
+        // All allocations we do from now on will be freed at the end of this function.
+        // Therefore, do not count them towards the persistent limits.
+        // Instead, create a local instance of `Limits` for this function alone
+        // which will be dropped along with all the buffers when they go out of scope.
+        let mut local_limits = self.limits.clone();
+
+        // Check the allocation we're about to perform against the limits
+        if let Err(e) = limits_reserve_buffer(&mut local_limits, frame.width, frame.height) {
+            return Some(Err(e));
+        }
+        // Allocate the buffer now that the limits allowed it
         let mut vec = vec![0; self.reader.buffer_size()];
         if let Err(err) = self.reader.read_into_buffer(&mut vec) {
             return Some(Err(ImageError::from_decoding(err)));
@@ -325,15 +371,19 @@ impl<R: Read> Iterator for GifFrameIterator<R> {
             && (self.width, self.height) == frame_buffer.dimensions()
         {
             for (x, y, pixel) in frame_buffer.enumerate_pixels_mut() {
-                let previous_pixel = self.non_disposed_frame.get_pixel_mut(x, y);
+                let previous_pixel = non_disposed_frame.get_pixel_mut(x, y);
                 blend_and_dispose_pixel(frame.disposal_method, previous_pixel, pixel);
             }
             frame_buffer
         } else {
+            // Check limits before allocating the buffer
+            if let Err(e) = limits_reserve_buffer(&mut local_limits, self.width, self.height) {
+                return Some(Err(e));
+            }
             ImageBuffer::from_fn(self.width, self.height, |x, y| {
                 let frame_x = x.wrapping_sub(frame.left);
                 let frame_y = y.wrapping_sub(frame.top);
-                let previous_pixel = self.non_disposed_frame.get_pixel_mut(x, y);
+                let previous_pixel = non_disposed_frame.get_pixel_mut(x, y);
 
                 if frame_x < frame_buffer.width() && frame_y < frame_buffer.height() {
                     let mut pixel = *frame_buffer.get_pixel(frame_x, frame_y);

tests/limits_anim.rs

@@ -0,0 +1,99 @@
+//! Test enforcement of size and memory limits for animation decoding APIs.
+
+use image::{io::Limits, AnimationDecoder, ImageDecoder, ImageResult};
+
+#[cfg(feature = "gif")]
+use image::codecs::gif::GifDecoder;
+
+#[cfg(feature = "gif")]
+fn gif_decode(data: &[u8], limits: Limits) -> ImageResult<()> {
+    let mut decoder = GifDecoder::new(data).unwrap();
+    decoder.set_limits(limits)?;
+    {
+        let frames = decoder.into_frames();
+        for result in frames {
+            result?;
+        }
+    }
+    Ok(())
+}
+
+/// Checks that the function returned `ImageError::Limits`, panics otherwise
+#[track_caller]
+fn assert_limit_error(res: ImageResult<()>) {
+    let err = res.expect_err("The input should have been rejected because it exceeds limits");
+    match err {
+        image::ImageError::Limits(_) => (), // all good
+        _ => panic!("Decoding failed due to an error unrelated to limits"),
+    }
+}
+
+/// Each frame is the size of the image,
+/// so we can just output each raw GIF frame buffer as the final composited frame
+/// with no additional scratch space
+#[test]
+#[cfg(feature = "gif")]
+fn animated_full_frame_discard() {
+    let data =
+        std::fs::read("tests/images/gif/anim/large-gif-anim-full-frame-replace.gif").unwrap();
+
+    let mut limits_dimensions_too_small = Limits::default();
+    limits_dimensions_too_small.max_image_width = Some(500);
+    limits_dimensions_too_small.max_image_height = Some(500);
+    assert_limit_error(gif_decode(&data, limits_dimensions_too_small));
+
+    let mut limits_memory_way_too_small = Limits::default();
+    // Start with a ridiculously low memory allocation cap
+    limits_memory_way_too_small.max_alloc = Some(5);
+    assert_limit_error(gif_decode(&data, limits_memory_way_too_small));
+
+    let mut limits_memory_too_small = Limits::default();
+    // 1000 * 1000 * 4 would be the exact size of the buffer for one RGBA frame.
+    // The decoder always peaks with at least two frames in memory at the same time.
+    // Set the limit a little higher than 1 frame than that it doesn't run into trivial checks
+    // for output frame size, and make it run into actual buffer allocation errors.
+    limits_memory_too_small.max_alloc = Some(1000 * 1000 * 5);
+    assert_limit_error(gif_decode(&data, limits_memory_too_small));
+
+    let mut limits_just_enough = Limits::default();
+    limits_just_enough.max_image_height = Some(1000);
+    limits_just_enough.max_image_width = Some(1000);
+    limits_just_enough.max_alloc = Some(1000 * 1000 * 4 * 2); // 4 for RGBA, 2 for 2 buffers kept in memory simultaneously
+
+    gif_decode(&data, limits_just_enough)
+        .expect("With these limits it should have decoded successfully");
+}
+
+/// The GIF frame does not cover the whole image, requiring additional scratch space
+#[test]
+#[cfg(feature = "gif")]
+fn animated_frame_combine() {
+    let data = std::fs::read("tests/images/gif/anim/large-gif-anim-combine.gif").unwrap();
+
+    let mut limits_dimensions_too_small = Limits::default();
+    limits_dimensions_too_small.max_image_width = Some(500);
+    limits_dimensions_too_small.max_image_height = Some(500);
+    assert_limit_error(gif_decode(&data, limits_dimensions_too_small));
+
+    let mut limits_memory_way_too_small = Limits::default();
+    // Start with a ridiculously low memory allocation cap
+    limits_memory_way_too_small.max_alloc = Some(5);
+    assert_limit_error(gif_decode(&data, limits_memory_way_too_small));
+
+    let mut limits_memory_too_small = Limits::default();
+    // 1000 * 1000 * 4 * would be the exact size of two buffers for an RGBA frame.
+    // In this mode the decoder uses 2 full frames (accumulated result and the output frame)
+    // plus the smaller frame size from the GIF format decoder that it composites onto the output frame.
+    // So two full frames are not actually enough for decoding here.
+    // Verify that this is caught.
+    limits_memory_too_small.max_alloc = Some(1000 * 1000 * 4 * 2); // 4 for RGBA, 2 for 2 buffers kept in memory simultaneously
+    assert_limit_error(gif_decode(&data, limits_memory_too_small));
+
+    let mut limits_enough = Limits::default();
+    limits_enough.max_image_height = Some(1000);
+    limits_enough.max_image_width = Some(1000);
+    limits_enough.max_alloc = Some(1000 * 1000 * 4 * 3); // 4 for RGBA, 2 for 2 buffers kept in memory simultaneously
+
+    gif_decode(&data, limits_enough)
+        .expect("With these limits it should have decoded successfully");
+}