From d55be717d1db2412c55259b5cb01d229a31958de Mon Sep 17 00:00:00 2001 From: "Sergey \"Shnatsel\" Davidoff" Date: Fri, 12 Jan 2024 05:36:04 +0000 Subject: [PATCH] Fix setting GIF limits, implement limits for GIF animations (#2090) --- src/codecs/gif.rs | 66 ++++++++++-- .../gif/anim/large-gif-anim-combine.gif | Bin 0 -> 2705 bytes .../large-gif-anim-full-frame-replace.gif | Bin 0 -> 3532 bytes tests/limits_anim.rs | 99 ++++++++++++++++++ 4 files changed, 157 insertions(+), 8 deletions(-) create mode 100644 tests/images/gif/anim/large-gif-anim-combine.gif create mode 100644 tests/images/gif/anim/large-gif-anim-full-frame-replace.gif create mode 100644 tests/limits_anim.rs diff --git a/src/codecs/gif.rs b/src/codecs/gif.rs index a5e52fa095..6f3f87d09c 100644 --- a/src/codecs/gif.rs +++ b/src/codecs/gif.rs @@ -60,7 +60,7 @@ impl GifDecoder { 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 { )) } + 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 { width: u32, height: u32, - non_disposed_frame: ImageBuffer, Vec>, + non_disposed_frame: Option, Vec>>, + limits: Limits, } impl GifFrameIterator { fn new(decoder: GifDecoder) -> GifFrameIterator { 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 Iterator for GifFrameIterator { type Item = ImageResult; fn next(&mut self) -> Option> { + 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 Iterator for GifFrameIterator { 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 Iterator for GifFrameIterator { && (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); diff --git a/tests/images/gif/anim/large-gif-anim-combine.gif b/tests/images/gif/anim/large-gif-anim-combine.gif new file mode 100644 index 0000000000000000000000000000000000000000..2d623937dc36d10eb1d45d226787ec0dff5a3e96 GIT binary patch literal 2705 zcmeIy{ZG;f00wZ%QqNfPs;P5U)^&1+Y^@htrTCgFr_QyTrHLt} zrKy!E3JL-ODgwU0%NIehEU|n;QQlv^fC7rb*>3ko+^%QOkI#?KK4IaZ!N1*+*~#qs zcS5#OSqT{%gV-Ptn+-BCGxtvq*>**qzjFEPnW*!}{R8Z_ZS4+Oe?I&0{++b@_dGM= z(@>sa;TNMIht_uNbb>fQ;19Pwg`{tfwS(AMgWG4U0VCn$u3+V8hb;#7@?LxuibPFP zcBjGxY02}MRPAG}trwe$fV&p~_=nu2km_+_T14Y@+7u<7r_SmT(KC_$?*%eO)@Vw{ zJji|=Nt@Q1JO2`BEw=0@Pjhmg<)QPR7ZhS%yeuky^}3|=O<8%x+js9PKU7s?Yie=$ zx_Sb!fkdV>Qfc(2<`zaPlf`ap@96C6?&l98TYPqtm{a`Ag(_rQ^D?jEMB*I3 ztwk!po-I6Zv7I5GAYLZl8f|Y?3h8%wRTn#$>S^{P<=kioOFQ42v+H1FCtD{5O8jn* zb+#F#tT;i8rA-o!vULynLOC?dE zKFg(7uEsCFxfa{7Ty`T}zFZ!Y=Od}OR~9dMi>hysyh~!rCGS&*eWaCXlkw6InUV%+ z)nlVvTAgk0E5qixB*<#=y-2cJOrS!BD-QLQ<4dn5$m=R%N%H#2bcLLN&GS_d@ns2$ z24X!)L834f3Nn4zS4m+^CMX-(5|WbIX;dg_z4m@8`k+gqstNG2l2y(8K&7f>Jk(Fk z_qK&ric%DofO~$?D0Pb`?{p=>UiQw4E!HiP|o+gskn}Fe~r*{fc?-Q6*%A=2G^?O))RzdsEGem>`W=TpZ@Vwp7Z9PFY*7h@-AaDl;@|QY_kkU4}1DS(g(D)O7`g zrCC?fIsO(ELzrSwv!zsvrqiUcXnP%wZGeNW4>ojwH*G`D57KTJ#>0+nu6&Muu(>)F zN82>cWN0@{BJ?q>^p~=?X}+#L;OJO z2Vy@E`#&OP{m&tm;|Z;C3`6wqc?aggkTp)xK)*{n_$&ZZ1C2urxPJrlg84POGk}2u z>md3xwALAo7(DE*%SRw<_ml&JULm^YQJC6&1jLZtJEw5plmlAiw_&YmRpV literal 0 HcmV?d00001 diff --git a/tests/images/gif/anim/large-gif-anim-full-frame-replace.gif b/tests/images/gif/anim/large-gif-anim-full-frame-replace.gif new file mode 100644 index 0000000000000000000000000000000000000000..c1bb8d6b79d1f67a0b79cadd764eb977f06fe41f GIT binary patch literal 3532 zcmeIz>rWGg8V2xgcz~mI;cNy-;~Z`VDj*VPfHAXhrh~Q?My3Hmm}8rRa+Np&gqQ_I zDMC>crC1RPExps0YfHJxt#^uYDI!Xt(EI&Pf%dqZujdaqA9(Y8eDi#Il3x-uG47jR zRX!@8$$tPoTx%;}ZVuoD0K8tn!rJ@)=A`#X$`4nsT>N+H59gwyeLni|chd8(i#PxH zwe*kcA?10ch>#>`aw_2D&Y^!E2KWIY|8Dxb6X4^C8@s`m$fh~bVf*$hlH~EF}?enBkSa&94g;pliSB}e=74WF%5>Hmj`KY`Vb40 zpM`_nZ&@XB!Hdf*`n%x@tLB}M#aJJ&@;Pz>&Sq{*)qWOnO~meza~r?9+y)=&-4?cd zUGQEs)TfkSFV>$(=Jcx#}OsCFA3s4=eHo%c>3qB4oGyLrJnbz!;URIv^3earamnV&h(L4r!z2c$sPg8Bzn@ ztUc9?*t`!WkT&Zg2UVN((X(JV>ijZ7-VmoC$sb&BsN@e5d{1vRUJA_HdISk2Z#{;` zsJEI@6HjkHxtf-@{WK$oyxp8trrt(p*PK?gTyM@(JVOx33QXakTJgMO_O!CKbU9D? zqC!Ddw%u~5mF-o&;VSIizBMyz0G2t7bV@?Ma=z4ge9;&Wa6rqnRdwu{zUKKvJZ6`dE6Qc_uiQW}ZD>t}}B(kdYSNDRiNQ z4<^zqf=HImB8=unT1Drjg;sH#l4hN|;M7?q34Uj6^Ou5(Y_A|;-L}{8SiNl_HR+6f z@oIXJeJLZi+rFGtuD46Gk!Kt$*U?3eHwa?4W3`Z_cf2j(o^h^~N{gKDDwN&M_qUvS z=X#Z2luLFusMxhp8%B3+qGAm$d1F%4&eju8dhyP7OD=s!(OPcUQDTu%ZWSI~>{gSA zbhn1aGPt!&Zq%->Us}AYXDjKuh7qS>*Er#KcF!~&bYsuV3uEkAM6t#_>wMDLecNLC zjeYw{E@R)ZR&LyP%8+M0t}XNp&yI@7@VIp>qi5H|J?q`GNpE=fT}p=6v*$E^SRfD! z@PkZz3^G;u!?7O+@g{uZO}7J3*h68EmxnV=cY;M&-&o$uPby8-A;1p*B*^5a7}H%a zyyI{>Z!(Z$x)+V=0OmrbK&z&jI8nzZ<-DmAZWA&A*cpI?On(V7*FxZ(pQ3rwq4DPX zsi@9lM955drrA@MA?gfd@n)hb&Gp$pTrd|hdk$kpA>cTWls6m0F*lT;aL1JpZv3kG zL4^o+!pY-)=QcmA0^&pbpuF!vmd08*{>vafFEQTos1b!f6$a(QGA)l=MEKBHK0l?> z(u4(efs>$upD>muBzRYNI$w~+u{>p>x*~I-!pv1mGh5UZRn8Y)b6e08KtePUD*6p% zZQ;QQ=g@pnPQ3NmJc@9h2o>jNS}`jkLJW&9F0QmbmjQ`!T^O9_*5R_*i(G4+~LARTkt8C6GV*?yN-CP5BnZ8Gh*AmPfa e;_OHNlP|~tUk7|0@O8k~0bd7v{eScI?SBBC{!E+z literal 0 HcmV?d00001 diff --git a/tests/limits_anim.rs b/tests/limits_anim.rs new file mode 100644 index 0000000000..ed191800d8 --- /dev/null +++ b/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"); +}