CVE-2019-16138

@@ -329,11 +329,21 @@ impl<R: BufRead> HDRDecoder<R> {
         // expression self.width > 0 && self.height > 0 is true from now to the end of this method
         // scanline buffer
         let uszwidth = self.width as usize;
+        let uszheight = self.height as usize;
+        let pixel_count = uszwidth.checked_mul(uszheight);
+
+        if uszwidth as u32 != self.width || uszheight as u32 != self.height || pixel_count.is_none() {
+            return Err(ImageError::DimensionError)
+        }
+
+        let pixel_count = pixel_count.unwrap();
 
-        let pixel_count = self.width as usize * self.height as usize;
         // Did not overflow, all chunks are full sized.
+        assert!(uszheight > 0);
+        assert!(uszwidth > 0);
+        assert!(uszwidth <= pixel_count);
         assert!(pixel_count % uszwidth == 0);
-        assert!(pixel_count / uszwidth == self.height as usize);
+        assert!(pixel_count / uszwidth == uszheight);
         assert!(pixel_count <= isize::max_value() as usize);
 
         let mut ret = Vec::with_capacity(pixel_count);
@@ -350,15 +360,17 @@ impl<R: BufRead> HDRDecoder<R> {
             let mut pool = Pool::new(8); //
 
             try!(pool.scoped(|scope| {
-                for chunk_index in 0..self.height as usize {
+                for chunk_index in 0..uszheight {
                     let mut buf = vec![RGBE8Pixel{ c: [0, 0, 0], e: 0, }; uszwidth];
                     try!(read_scanline(&mut self.r, &mut buf[..]));
 
                     let f = &f;
                     // Each chunk is a unique slice of exactly `uszwidth` uninitialized elements.
                     let chunk = unsafe {
-                        // SAFETY: within the allocation as per `pixel_count` assertions.This is
+                        // SAFETY: within the allocation as per `pixel_count` assertions. This is
                         // in-bounds since the `Vec` has enough capacity and is never re-allocated.
+                        // Also, chunk_index * uszwidth <= pixel_count and thus does not overflow
+                        // and fits into isize.
                         let start: *mut T = chunks_base.offset((chunk_index * uszwidth) as isize);
                         // INVARIANT: unique since all slices are spaced `uszwidth` elements and
                         // also exactly as large.
@@ -369,7 +381,8 @@ impl<R: BufRead> HDRDecoder<R> {
                         let UninitSlice(base, len) = chunk;
                         for (dst, &pix) in (0..len).zip(buf.iter()) {
                             unsafe {
-                                // SAFETY:
+                                // SAFETY: Same as for `start`. But additionally, all chunks are
+                                // disjunct and this write is not a data race.
                                 ptr::write(base.offset(dst), f(pix))
                             }
                         }

@@ -330,57 +330,48 @@ impl<R: BufRead> HDRDecoder<R> {
     pub fn read_image_transform<T: Send, F: Send + Sync + Fn(RGBE8Pixel) -> T>(
         mut self,
         f: F,
-    ) -> ImageResult<Vec<T>> {
+        output_slice: &mut [T],
+    ) -> ImageResult<()> {
+        assert_eq!(output_slice.len(), self.width as usize * self.height as usize);
+
         // Don't read anything if image is empty
         if self.width == 0 || self.height == 0 {
-            return Ok(vec![]);
+            return Ok(());
         }
-        // expression self.width > 0 && self.height > 0 is true from now to the end of this method
-        // scanline buffer
-        let uszwidth = self.width as usize;
 
-        let pixel_count = self.width as usize * self.height as usize;
-        let mut ret = Vec::with_capacity(pixel_count);
-        unsafe {
-            // RGBE8Pixel doesn't implement Drop, so it's Ok to drop half-initialized ret
-            ret.set_len(pixel_count);
-        } // ret contains uninitialized data, so now it's my responsibility to return fully initialized ret
-
-        {
-            let chunks_iter = ret.chunks_mut(uszwidth);
-            let mut pool = Pool::new(8); //
-
-            try!(pool.scoped(|scope| {
-                for chunk in chunks_iter {
-                    let mut buf = Vec::<RGBE8Pixel>::with_capacity(uszwidth);
-                    unsafe {
-                        buf.set_len(uszwidth);
+        let chunks_iter = output_slice.chunks_mut(self.width as usize);
+        let mut pool = Pool::new(8); //
+
+        try!(pool.scoped(|scope| {
+            for chunk in chunks_iter {
+                let mut buf = vec![Default::default(); self.width as usize];
+                try!(read_scanline(&mut self.r, &mut buf[..]));
+                let f = &f;
+                scope.execute(move || {
+                    for (dst, &pix) in chunk.iter_mut().zip(buf.iter()) {
+                        *dst = f(pix);
                     }
-                    try!(read_scanline(&mut self.r, &mut buf[..]));
-                    let f = &f;
-                    scope.execute(move || {
-                        for (dst, &pix) in chunk.iter_mut().zip(buf.iter()) {
-                            *dst = f(pix);
-                        }
-                    });
-                }
-                Ok(())
-            }) as Result<(), ImageError>);
-        }
-
-        Ok(ret)
+                });
+            }
+            Ok(())
+        }) as Result<(), ImageError>);
+        Ok(())
     }
 
     /// Consumes decoder and returns a vector of Rgb<u8> pixels.
     /// scale = 1, gamma = 2.2
     pub fn read_image_ldr(self) -> ImageResult<Vec<Rgb<u8>>> {
-        self.read_image_transform(|pix| pix.to_ldr())
+        let mut ret = vec![Rgb([0,0,0]); self.width as usize * self.height as usize];
+        self.read_image_transform(|pix| pix.to_ldr(), &mut ret[..])?;
+        Ok(ret)
     }
 
     /// Consumes decoder and returns a vector of Rgb<f32> pixels.
     ///
     pub fn read_image_hdr(self) -> ImageResult<Vec<Rgb<f32>>> {
-        self.read_image_transform(|pix| pix.to_hdr())
+        let mut ret = vec![Rgb([0.0, 0.0, 0.0]); self.width as usize * self.height as usize];
+        self.read_image_transform(|pix| pix.to_hdr(), &mut ret[..])?;
+        Ok(ret)
     }
 }