Repository: image-rs/image-tiff
Pull request #139 (2021-05-28 19:37:59)

Check for various arithmetic overflows
This causes a panic with multiplication overflow in debug mode (and, on this particular case, a `unreachable!` if overflow checking is disabled).

`LimitsExceeded` seems like the most reasonable error to emit, even though we don't care about the limits the user has set here. I could swap it out for `IntSizeError`, but it's not an error during *conversion*, it's an error during an *operation*.


Patch:
diff --git a/src/decoder/ifd.rs b/src/decoder/ifd.rs
index 862616a..23416a4 100644
--- a/src/decoder/ifd.rs
+++ b/src/decoder/ifd.rs
@@ -349,19 +349,26 @@ impl Entry {
         }
 
         let bo = decoder.byte_order();
-        let value_bytes = self.count
-            * match self.type_ {
-                Type::BYTE | Type::SBYTE | Type::ASCII | Type::UNDEFINED => 1,
-                Type::SHORT | Type::SSHORT => 2,
-                Type::LONG | Type::SLONG | Type::FLOAT | Type::IFD => 4,
-                Type::LONG8
-                | Type::SLONG8
-                | Type::DOUBLE
-                | Type::RATIONAL
-                | Type::SRATIONAL
-                | Type::IFD8 => 8,
-                Type::__NonExhaustive => unreachable!(),
-            };
+
+        let tag_size = match self.type_ {
+            Type::BYTE | Type::SBYTE | Type::ASCII | Type::UNDEFINED => 1,
+            Type::SHORT | Type::SSHORT => 2,
+            Type::LONG | Type::SLONG | Type::FLOAT | Type::IFD => 4,
+            Type::LONG8
+            | Type::SLONG8
+            | Type::DOUBLE
+            | Type::RATIONAL
+            | Type::SRATIONAL
+            | Type::IFD8 => 8,
+            Type::__NonExhaustive => unreachable!(),
+        };
+
+        let value_bytes = match self.count.checked_mul(tag_size) {
+            Some(n) => n,
+            None => {
+                return Err(TiffError::LimitsExceeded);
+            }
+        };
 
         // Case 2: there is one value.
         if self.count == 1 {
diff --git a/src/decoder/mod.rs b/src/decoder/mod.rs
index 368d8fc..f101c7c 100644
--- a/src/decoder/mod.rs
+++ b/src/decoder/mod.rs
@@ -1277,7 +1277,13 @@ impl<R: Read + Seek> Decoder<R> {
             return Ok(0);
         }
 
-        Ok((self.height + rows_per_strip - 1) / rows_per_strip)
+        // rows_per_strip - 1 can never fail since we know it's at least 1
+        let height = match self.height.checked_add(rows_per_strip - 1) {
+            Some(h) => h,
+            None => return Err(TiffError::IntSizeError),
+        };
+
+        Ok(height / rows_per_strip)
     }
 
     /// Number of tiles in image
@@ -1544,7 +1550,13 @@ impl<R: Read + Seek> Decoder<R> {
     }
 
     fn result_buffer(&self, width: usize, height: usize) -> TiffResult<DecodingResult> {
-        let buffer_size = width * height * self.bits_per_sample.len();
+        let buffer_size = match width
+            .checked_mul(height)
+            .and_then(|x| x.checked_mul(self.bits_per_sample.len()))
+        {
+            Some(s) => s,
+            None => return Err(TiffError::LimitsExceeded),
+        };
 
         let max_sample_bits = self.bits_per_sample.iter().cloned().max().unwrap_or(8);
         match self.sample_format.first().unwrap_or(&SampleFormat::Uint) {
@@ -1646,8 +1658,13 @@ impl<R: Read + Seek> Decoder<R> {
         let rows_per_strip =
             usize::try_from(self.get_tag_u32(Tag::RowsPerStrip).unwrap_or(self.height))?;
 
-        let samples_per_strip =
-            usize::try_from(self.width)? * rows_per_strip * self.bits_per_sample.len();
+        let samples_per_strip = match usize::try_from(self.width)?
+            .checked_mul(rows_per_strip)
+            .and_then(|x| x.checked_mul(self.bits_per_sample.len()))
+        {
+            Some(s) => s,
+            None => return Err(TiffError::LimitsExceeded),
+        };
 
         let mut result =
             self.result_buffer(usize::try_from(self.width)?, usize::try_from(self.height)?)?;


Test patch:
diff --git a/tests/decode_images.rs b/tests/decode_images.rs
index 58ffc4a..59bcca0 100644
--- a/tests/decode_images.rs
+++ b/tests/decode_images.rs
@@ -307,3 +307,81 @@ fn test_div_zero() {
         unexpected => panic!("Unexpected error {}", unexpected),
     }
 }
+
+#[test]
+fn test_too_many_value_bytes() {
+    let image = [
+        73, 73, 43, 0, 8, 0, 0, 0, 8, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 255, 0, 8, 0, 0, 0,
+        23, 0, 12, 0, 0, 65, 4, 0, 1, 6, 0, 0, 1, 16, 0, 1, 0, 0, 0, 0, 0, 0, 128, 0, 0, 0, 0, 0,
+        0, 0, 3, 0, 1, 0, 0, 0, 1, 0, 0, 0, 59, 73, 84, 186, 202, 83, 240, 66, 1, 53, 22, 56, 47,
+        0, 0, 0, 0, 0, 0, 1, 222, 4, 0, 58, 0, 0, 0, 0, 0, 0, 0, 1, 1, 0, 0, 1, 1, 4, 0, 0, 100, 0,
+        0, 89, 89, 89, 89, 89, 89, 89, 89, 96, 1, 20, 89, 89, 89, 89, 18,
+    ];
+
+    let error = tiff::decoder::Decoder::new(std::io::Cursor::new(&image)).unwrap_err();
+
+    match error {
+        tiff::TiffError::LimitsExceeded => {}
+        unexpected => panic!("Unexpected error {}", unexpected),
+    }
+}
+
+#[test]
+fn test_bad_strip_count() {
+    let image = [
+        73, 73, 42, 0, 8, 0, 0, 0, 8, 0, 0, 1, 4, 0, 1, 0, 0, 0, 100, 0, 0, 0, 1, 1, 4, 0, 1, 0, 0,
+        0, 158, 0, 0, 251, 3, 1, 3, 0, 1, 0, 0, 0, 1, 0, 0, 0, 6, 1, 3, 0, 1, 0, 0, 0, 0, 0, 0, 0,
+        17, 1, 4, 0, 1, 0, 0, 0, 0, 0, 0, 0, 2, 1, 3, 0, 0, 0, 0, 0, 246, 16, 0, 0, 22, 1, 4, 0, 1,
+        0, 0, 0, 40, 0, 251, 255, 23, 1, 4, 0, 1, 0, 0, 0, 48, 178, 178, 178, 178, 178, 178, 178,
+        178, 178, 178,
+    ];
+
+    let mut decoder = tiff::decoder::Decoder::new(std::io::Cursor::new(&image)).unwrap();
+
+    let err = decoder.read_image().unwrap_err();
+
+    match err {
+        tiff::TiffError::IntSizeError => {}
+        unexpected => panic!("Unexpected error {}", unexpected),
+    }
+}
+
+#[test]
+fn test_too_big_buffer_size() {
+    let image = [
+        73, 73, 42, 0, 8, 0, 0, 0, 8, 0, 0, 1, 4, 0, 1, 0, 0, 0, 99, 255, 255, 254, 1, 1, 4, 0, 1,
+        0, 0, 0, 158, 0, 0, 251, 3, 1, 3, 255, 254, 255, 255, 0, 1, 0, 0, 0, 6, 1, 3, 0, 1, 0, 0,
+        0, 0, 0, 0, 0, 17, 1, 4, 0, 9, 0, 0, 0, 0, 0, 0, 0, 2, 1, 3, 0, 2, 0, 0, 0, 63, 0, 0, 0,
+        22, 1, 4, 0, 1, 0, 0, 0, 44, 0, 0, 0, 23, 1, 4, 0, 0, 0, 0, 0, 0, 0, 2, 1, 3, 1, 0, 178,
+        178,
+    ];
+
+    let mut decoder = tiff::decoder::Decoder::new(std::io::Cursor::new(&image)).unwrap();
+
+    let err = decoder.read_image().unwrap_err();
+
+    match err {
+        tiff::TiffError::LimitsExceeded => {}
+        unexpected => panic!("Unexpected error {}", unexpected),
+    }
+}
+
+#[test]
+fn test_stripped_image_overflow() {
+    let image = [
+        73, 73, 42, 0, 8, 0, 0, 0, 8, 0, 0, 1, 4, 0, 1, 0, 0, 0, 100, 0, 0, 148, 1, 1, 4, 0, 1, 0,
+        0, 0, 158, 0, 0, 251, 3, 1, 3, 255, 254, 255, 255, 0, 1, 0, 0, 0, 6, 1, 3, 0, 1, 0, 0, 0,
+        0, 0, 0, 0, 17, 1, 4, 0, 1, 0, 0, 0, 0, 0, 0, 0, 2, 1, 3, 0, 2, 0, 0, 0, 63, 0, 0, 0, 22,
+        1, 4, 0, 1, 0, 0, 0, 44, 0, 248, 255, 23, 1, 4, 0, 1, 0, 0, 0, 178, 178, 178, 0, 1, 178,
+        178, 178,
+    ];
+
+    let mut decoder = tiff::decoder::Decoder::new(std::io::Cursor::new(&image)).unwrap();
+
+    let err = decoder.read_image().unwrap_err();
+
+    match err {
+        tiff::TiffError::LimitsExceeded => {}
+        unexpected => panic!("Unexpected error {}", unexpected),
+    }
+}