diff --git a/src/codecs/gif.rs b/src/codecs/gif.rs index 70b748054c..593c490ba9 100644 --- a/src/codecs/gif.rs +++ b/src/codecs/gif.rs @@ -43,22 +43,36 @@ use crate::error::{ UnsupportedError, UnsupportedErrorKind, }; use crate::image::{self, AnimationDecoder, ImageDecoder, ImageFormat}; +use crate::io::Limits; use crate::traits::Pixel; use crate::ImageBuffer; /// GIF decoder pub struct GifDecoder { reader: gif::Decoder, + limits: Limits, } impl GifDecoder { - /// 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> { 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> { + 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, }) } } @@ -154,7 +168,14 @@ impl<'a, R: 'a + Read> ImageDecoder<'a> for GifDecoder { } 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_usize(buffer_size); + 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..2cac99a47a 100644 --- a/src/io/mod.rs +++ b/src/io/mod.rs @@ -1,5 +1,7 @@ //! Input and output of images. +use std::convert::TryFrom; + use crate::{error, ImageError, ImageResult}; pub(crate) mod free_functions; @@ -122,8 +124,24 @@ impl Limits { Ok(()) } + /// This function acts identically to [`reserve`], but takes a `usize` for convenience. + pub fn reserve_usize(&mut self, amount: usize) -> ImageResult<()> { + 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) { @@ -131,4 +149,17 @@ impl Limits { *max_alloc = max_alloc.saturating_add(amount); } } + + /// This function acts identically to [`free`], but takes a `usize` for convenience. + pub fn free_usize(&mut self, amount: usize) { + match u64::try_from(amount) { + Ok(n) => self.free(n), + Err(_) if self.max_alloc.is_some() => { + panic!("max_alloc is set, we should have exited earlier when the reserve failed"); + } + Err(_) => { + // Out of bounds, but we weren't asked to consider any limit. + } + } + } } diff --git a/tests/regression.rs b/tests/regression.rs index 2710185c27..927447d7a2 100644 --- a/tests/regression.rs +++ b/tests/regression.rs @@ -59,3 +59,23 @@ 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 (or an unsupported error, for the case that we're + // running these tests without gif being actually supported) + let error = image::load_from_memory(&data).unwrap_err(); + + assert!( + matches!(error, image::ImageError::Limits(_)) + | matches!(error, image::ImageError::Unsupported(_)) + ); +}