From cac78c2b943bddaf4c10453c352c4a0259692e40 Mon Sep 17 00:00:00 2001
From: 5225225 <5225225@mailbox.org>
Date: Sun, 17 Apr 2022 01:48:11 +0100
Subject: [PATCH] Fix OOM when decoding gif with very large buffer

---
 src/codecs/gif.rs   | 26 ++++++++++++++++++++++++--
 src/io/mod.rs       | 20 +++++++++++++++++++-
 tests/regression.rs | 11 +++++++++++
 3 files changed, 54 insertions(+), 3 deletions(-)

diff --git a/src/codecs/gif.rs b/src/codecs/gif.rs
index 70b748054c..aef3c136d2 100644
--- a/src/codecs/gif.rs
+++ b/src/codecs/gif.rs
@@ -36,6 +36,7 @@ use gif::ColorOutput;
 use gif::{DisposalMethod, Frame};
 use num_rational::Ratio;
 
+use crate::io::Limits;
 use crate::animation;
 use crate::color::{ColorType, Rgba};
 use crate::error::{
@@ -49,16 +50,29 @@ use crate::ImageBuffer;
 /// GIF decoder
 pub struct GifDecoder<R: Read> {
     reader: gif::Decoder<R>,
+    limits: Limits,
 }
 
 impl<R: Read> GifDecoder<R> {
-    /// Creates a new decoder that decodes the input steam ```r```
+    /// Creates a new decoder that decodes the input steam `r`
     pub fn new(r: R) -> ImageResult<GifDecoder<R>> {
         let mut decoder = gif::DecodeOptions::new();
         decoder.set_color_output(ColorOutput::RGBA);
 
         Ok(GifDecoder {
             reader: decoder.read_info(r).map_err(ImageError::from_decoding)?,
+            limits: Limits::default(),
+        })
+    }
+
+    /// Creates a new decoder that decodes the input steam `r`, using limits `limits`
+    pub fn with_limits(r: R, limits: Limits) -> ImageResult<GifDecoder<R>> {
+        let mut decoder = gif::DecodeOptions::new();
+        decoder.set_color_output(ColorOutput::RGBA);
+
+        Ok(GifDecoder {
+            reader: decoder.read_info(r).map_err(ImageError::from_decoding)?,
+            limits,
         })
     }
 }
@@ -116,6 +130,7 @@ impl<'a, R: 'a + Read> ImageDecoder<'a> for GifDecoder<R> {
             }
         };
 
+
         let (width, height) = self.dimensions();
 
         if frame.left == 0
@@ -154,7 +169,14 @@ impl<'a, R: 'a + Read> ImageDecoder<'a> for GifDecoder<R> {
         } else {
             // If the frame does not match the logical screen, read into an extra buffer
             // and 'insert' the frame from left/top to logical screen width/height.
-            let mut frame_buffer = vec![0; self.reader.buffer_size()];
+            let buffer_size = self.reader.buffer_size();
+
+            self.limits.reserve_usize(buffer_size)?;
+
+            let mut frame_buffer = vec![0; buffer_size];
+
+            self.limits.free(u64::try_from(buffer_size).expect("if buffer_size overflows a usize, we should have returned already"));
+
             self.reader
                 .read_into_buffer(&mut frame_buffer[..])
                 .map_err(ImageError::from_decoding)?;
diff --git a/src/io/mod.rs b/src/io/mod.rs
index 71f20a0258..ff72f8a72d 100644
--- a/src/io/mod.rs
+++ b/src/io/mod.rs
@@ -122,8 +122,26 @@ impl Limits {
         Ok(())
     }
 
+    /// This function acts identically to [`reserve`], but takes a `usize` for convenience.
+    pub fn reserve_usize(&mut self, amount: usize) -> ImageResult<()> {
+        use std::convert::TryFrom;
+
+        match u64::try_from(amount) {
+            Ok(n) => self.reserve(n),
+            Err(_) if self.max_alloc.is_some() => {
+                return Err(ImageError::Limits(error::LimitError::from_kind(
+                    error::LimitErrorKind::InsufficientMemory,
+                )));
+            }
+            Err(_) => {
+                // Out of bounds, but we weren't asked to consider any limit.
+                Ok(())
+            }
+        }
+    }
+
     /// This function increases the `max_alloc` limit with amount. Should only be used
-    /// togheter with [`reserve`].
+    /// together with [`reserve`].
     ///
     /// [`reserve`]: #method.reserve
     pub fn free(&mut self, amount: u64) {
diff --git a/tests/regression.rs b/tests/regression.rs
index 2710185c27..4b6435c6db 100644
--- a/tests/regression.rs
+++ b/tests/regression.rs
@@ -59,3 +59,14 @@ fn bad_bmps() {
         assert!(im.is_err());
     }
 }
+
+#[test]
+fn bad_gif_oom() {
+    let data = [71, 73, 70, 56, 55, 97, 0, 0, 0, 0, 0, 0, 0, 44, 255, 255, 219, 255, 172, 199, 199, 255, 216, 255, 255, 0, 0, 48, 230, 0, 195, 195, 195, 195, 255, 239, 0];
+
+    // The original code made a vec![0; very_large_number] which due to overcommit *does not* OOM.
+    // It then exits normally with an EOF when reading.
+    //
+    // So instead we look for a limits error.
+    assert!(matches!(image::load_from_memory(&data), Err(image::ImageError::Limits(_))));
+}