Check for integer overflows in RawBmp::parse (#32)

* Check for integer overflows in RawBmp::parse

* Don't allow images with zero width or height

* Check for negative width and add tests

* Reformat code

Author: rfuest

CHANGELOG.md

@@ -23,6 +23,10 @@
 - [#28](https://github.com/embedded-graphics/tinybmp/pull/28) `Bpp::bits`, `RawBmp::image_data`, `RawBmp::header`, and `RawPixel::new` are now `const`.
 - [#28](https://github.com/embedded-graphics/tinybmp/pull/28) BMP files with incomplete image data are now detected by `Bmp::from_slice`.
 
+### Fixed
+
+- [#32](https://github.com/embedded-graphics/tinybmp/pull/32) Report error for images with `width <= 0` or `height == 0` instead of causing a panic.
+
 ## [0.3.3] - 2022-04-18
 
 ### Fixed

src/header/dib_header.rs

@@ -48,7 +48,7 @@ impl DibHeader {
         };
 
         // Fields common to all DIB variants
-        let (dib_header_data, image_width) = le_u32(dib_header_data)?;
+        let (dib_header_data, image_width) = le_i32(dib_header_data)?;
         let (dib_header_data, image_height) = le_i32(dib_header_data)?;
         let (dib_header_data, _color_planes) = le_u16(dib_header_data)?;
         let (dib_header_data, bpp) = Bpp::parse(dib_header_data)?;
@@ -91,6 +91,10 @@ impl DibHeader {
             colors_used
         };
 
+        if image_width <= 0 || image_height == 0 {
+            return Err(ParseError::InvalidImageDimensions);
+        }
+
         let row_order = if image_height < 0 {
             RowOrder::TopDown
         } else {
@@ -101,7 +105,7 @@ impl DibHeader {
             input,
             Self {
                 header_type,
-                image_size: Size::new(image_width, image_height.unsigned_abs()),
+                image_size: Size::new(image_width.unsigned_abs(), image_height.unsigned_abs()),
                 image_data_len,
                 bpp,
                 channel_masks,

src/lib.rs

@@ -365,6 +365,9 @@ pub enum ParseError {
 
     /// Unsupported channel masks.
     UnsupportedChannelMasks,
+
+    /// Invalid image dimensions.
+    InvalidImageDimensions,
 }
 
 #[cfg(test)]

src/raw_bmp.rs

@@ -44,10 +44,21 @@ impl<'a> RawBmp<'a> {
 
         let color_type = ColorType::from_header(&header)?;
 
-        let data_length = header.bytes_per_row() * header.image_size.height as usize;
-
+        let height = header
+            .image_size
+            .height
+            .try_into()
+            .map_err(|_| ParseError::InvalidImageDimensions)?;
+
+        let data_length = header
+            .bytes_per_row()
+            .checked_mul(height)
+            .ok_or(ParseError::InvalidImageDimensions)?;
+
+        // The `get` is split into two calls to prevent an possible integer overflow.
         let image_data = &bytes
-            .get(header.image_data_start..header.image_data_start + data_length)
+            .get(header.image_data_start..)
+            .and_then(|bytes| bytes.get(..data_length))
             .ok_or(ParseError::UnexpectedEndOfFile)?;
 
         Ok(Self {

tests/error-height-0.bmp

@@ -0,0 +1,26 @@
+use embedded_graphics::pixelcolor::Rgb888;
+use tinybmp::{Bmp, ParseError};
+
+#[test]
+fn zero_width() {
+    assert_eq!(
+        Bmp::<Rgb888>::from_slice(include_bytes!("error-width-0.bmp")),
+        Err(ParseError::InvalidImageDimensions)
+    );
+}
+
+#[test]
+fn negative_width() {
+    assert_eq!(
+        Bmp::<Rgb888>::from_slice(include_bytes!("error-width-negative.bmp")),
+        Err(ParseError::InvalidImageDimensions)
+    );
+}
+
+#[test]
+fn zero_height() {
+    assert_eq!(
+        Bmp::<Rgb888>::from_slice(include_bytes!("error-height-0.bmp")),
+        Err(ParseError::InvalidImageDimensions)
+    );
+}