Repository: image-rs/image-tiff
Pull request #141 (2021-05-29 20:29:23)

Fix out of memory issues / misc other decoding bugs.
This contains the fuzzer code as well since I'm lazy and didn't split it out, so I'd prefer to get that reviewed separately then review this one.


Patch:
diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml
index 65fcb70..e444b18 100644
--- a/.github/workflows/rust.yml
+++ b/.github/workflows/rust.yml
@@ -9,7 +9,7 @@ jobs:
     runs-on: ubuntu-latest
     strategy:
       matrix:
-        rust: ["1.34.2", stable, beta, nightly]
+        rust: ["1.48", stable, beta, nightly]
         command: [build, test]
     steps:
     - uses: actions/checkout@v2
@@ -20,7 +20,7 @@ jobs:
     - name: test
       run: >
         cargo test --tests --benches --no-default-features --features "$FEATURES"
-      if: ${{ matrix.rust != '1.34.2' }}
+      if: ${{ matrix.rust != '1.48' }}
       env:
         FEATURES: ${{ matrix.features }}
   rustfmt:
diff --git a/README.md b/README.md
index ce32891..a5fc0a5 100644
--- a/README.md
+++ b/README.md
@@ -44,3 +44,11 @@ Formats and interpretations not listed above or with empty entries are unsupport
 - Baseline tags
   - `ExtraSamples`
 - Extension tags
+
+## Fuzzing
+
+This crate uses [cargo-fuzz](https://github.com/rust-fuzz/cargo-fuzz) in order to test the image parser.
+
+After installing it with `cargo install cargo-fuzz` on a nightly rustc, the
+fuzzing harness can be run with recommended settings using 
+`cargo fuzz run decode_image -snone -- -timeout=5`.
diff --git a/fuzz/fuzz_targets/decode_image.rs b/fuzz/fuzz_targets/decode_image.rs
index 47feb95..51a0892 100644
--- a/fuzz/fuzz_targets/decode_image.rs
+++ b/fuzz/fuzz_targets/decode_image.rs
@@ -23,5 +23,9 @@ fuzz_target!(|data: &[u8]| {
         if !decoder.more_images() {
             break;
         }
+
+        if let Err(_) = decoder.next_image() {
+            break;
+        }
     }
 });
diff --git a/src/decoder/mod.rs b/src/decoder/mod.rs
index 72771e1..f39c531 100644
--- a/src/decoder/mod.rs
+++ b/src/decoder/mod.rs
@@ -1,4 +1,4 @@
-use std::collections::HashMap;
+use std::collections::{HashMap, HashSet};
 use std::convert::TryFrom;
 use std::io::{self, Read, Seek};
 use std::{cmp, ops::Range};
@@ -378,6 +378,7 @@ where
     strip_decoder: Option<StripDecodeState>,
     tile_decoder: Option<TileDecodeState>,
     tile_attributes: Option<TileAttributes>,
+    seen_ifds: HashSet<u64>,
 }
 
 trait Wrapping {
@@ -544,6 +545,7 @@ impl<R: Read + Seek> Decoder<R> {
             strip_decoder: None,
             tile_decoder: None,
             tile_attributes: None,
+            seen_ifds: HashSet::new(),
         }
         .init()
     }
@@ -640,7 +642,7 @@ impl<R: Read + Seek> Decoder<R> {
                 ))
             }
         }
-        self.next_ifd = match self.read_ifd_offset()? {
+        self.next_ifd = match self.read_ifd_offset_nonrepeating()? {
             0 => None,
             n => Some(n),
         };
@@ -736,6 +738,15 @@ impl<R: Read + Seek> Decoder<R> {
         self.byte_order
     }
 
+    fn read_ifd_offset_nonrepeating(&mut self) -> TiffResult<u64> {
+        let result = self.read_ifd_offset()?;
+        if self.seen_ifds.insert(result) {
+            Ok(result)
+        } else {
+            Err(TiffError::FormatError(TiffFormatError::CycleInOffsets))
+        }
+    }
+
     #[inline]
     pub fn read_ifd_offset(&mut self) -> Result<u64, io::Error> {
         if self.bigtiff {
@@ -894,7 +905,7 @@ impl<R: Read + Seek> Decoder<R> {
             };
             dir.insert(tag, entry);
         }
-        self.next_ifd = match self.read_ifd_offset()? {
+        self.next_ifd = match self.read_ifd_offset_nonrepeating()? {
             0 => None,
             n => Some(n),
         };
@@ -1124,6 +1135,7 @@ impl<R: Read + Seek> Decoder<R> {
             length,
             buffer.len(),
             buffer.byte_len(),
+            self.limits.intermediate_buffer_size,
         )?;
 
         // Read into output buffer.
@@ -1188,6 +1200,7 @@ impl<R: Read + Seek> Decoder<R> {
             compressed_length,
             tile_samples,
             byte_len,
+            self.limits.intermediate_buffer_size,
         )?;
 
         for row in 0..(tile_length - padding_down) {
@@ -1232,17 +1245,20 @@ impl<R: Read + Seek> Decoder<R> {
         compressed_length: u64,
         samples: usize,  // Expected chunk length in samples
         byte_len: usize, // Byte length of the samples in result buffer
+        intermediate_buffer_size: usize,
     ) -> TiffResult<Box<dyn Read + 'r>> {
         Ok(match compression_method {
             CompressionMethod::None => Box::new(reader),
-            CompressionMethod::LZW => Box::new(
-                LZWReader::new(
-                    reader,
-                    usize::try_from(compressed_length)?,
-                    samples * byte_len,
-                )?
-                .1,
-            ),
+            CompressionMethod::LZW => {
+                let clen = usize::try_from(compressed_length)?;
+
+                if samples * byte_len > intermediate_buffer_size || clen > intermediate_buffer_size
+                {
+                    return Err(TiffError::LimitsExceeded);
+                }
+
+                Box::new(LZWReader::new(reader, clen, samples * byte_len)?.1)
+            }
             CompressionMethod::PackBits => {
                 Box::new(PackBitsReader::new(reader, usize::try_from(compressed_length)?)?.1)
             }
@@ -1375,7 +1391,17 @@ impl<R: Read + Seek> Decoder<R> {
 
         let jpeg_tables: Option<Vec<u8>> = match self.find_tag(Tag::JPEGTables) {
             Ok(None) => None,
-            Ok(_) => Some(self.get_tag_u8_vec(Tag::JPEGTables)?),
+            Ok(_) => {
+                let vec = self.get_tag_u8_vec(Tag::JPEGTables)?;
+
+                if vec.len() < 2 {
+                    return Err(TiffError::FormatError(
+                        TiffFormatError::InvalidTagValueType(Tag::JPEGTables),
+                    ));
+                }
+
+                Some(vec)
+            }
             Err(e) => return Err(e),
         };
 
@@ -1390,11 +1416,26 @@ impl<R: Read + Seek> Decoder<R> {
             ));
         }
 
+        if offsets[0] as usize > self.limits.intermediate_buffer_size
+            || bytes
+                .iter()
+                .any(|&x| x as usize > self.limits.intermediate_buffer_size)
+        {
+            return Err(TiffError::LimitsExceeded);
+        }
+
         let mut res_img = Vec::with_capacity(offsets[0] as usize);
 
         for (idx, offset) in offsets.iter().enumerate() {
             self.goto_offset(*offset)?;
-            let jpeg_reader = JpegReader::new(&mut self.reader, bytes[idx], &jpeg_tables)?;
+            let length = bytes[idx];
+
+            if jpeg_tables.is_some() && length < 2 {
+                return Err(TiffError::FormatError(
+                    TiffFormatError::InvalidTagValueType(Tag::JPEGTables),
+                ));
+            }
+            let jpeg_reader = JpegReader::new(&mut self.reader, length, &jpeg_tables)?;
             let mut decoder = jpeg::Decoder::new(jpeg_reader);
 
             match decoder.decode() {
@@ -1446,10 +1487,18 @@ impl<R: Read + Seek> Decoder<R> {
         let sized_width = usize::try_from(self.width)?;
         let sized_height = usize::try_from(self.height)?;
 
+        let strip_height_without_padding = index
+            .checked_mul(rows_per_strip)
+            .and_then(|x| sized_height.checked_sub(x))
+            .ok_or(TiffError::IntSizeError)?;
+
         // Ignore potential vertical padding on the bottommost strip
-        let strip_height = cmp::min(rows_per_strip, sized_height - index * rows_per_strip);
+        let strip_height = rows_per_strip.min(strip_height_without_padding);
 
-        let buffer_size = sized_width * strip_height * self.bits_per_sample.len();
+        let buffer_size = sized_width
+            .checked_mul(strip_height)
+            .and_then(|x| x.checked_mul(self.bits_per_sample.len()))
+            .ok_or(TiffError::LimitsExceeded)?;
 
         if buffer.len() < buffer_size {
             return Err(TiffError::FormatError(
diff --git a/src/decoder/stream.rs b/src/decoder/stream.rs
index 294a1c8..c1c990d 100644
--- a/src/decoder/stream.rs
+++ b/src/decoder/stream.rs
@@ -225,6 +225,18 @@ impl JpegReader {
 
         match jpeg_tables {
             Some(tables) => {
+                assert!(
+                    tables.len() >= 2,
+                    "jpeg_tables, if given, must be at least 2 bytes long. Got {:?}",
+                    tables
+                );
+
+                assert!(
+                    length >= 2,
+                    "if jpeg_tables is given, length must be at least 2 bytes long, got {}",
+                    length
+                );
+
                 let mut jpeg_data = tables.clone();
                 let truncated_length = jpeg_data.len() - 2;
                 jpeg_data.truncate(truncated_length);
diff --git a/src/error.rs b/src/error.rs
index 67e5f08..ea6819d 100644
--- a/src/error.rs
+++ b/src/error.rs
@@ -64,6 +64,7 @@ pub enum TiffFormatError {
     Format(String),
     RequiredTagEmpty(Tag),
     StripTileTagConflict,
+    CycleInOffsets,
     #[doc(hidden)]
     /// Do not match against this variant. It may get removed.
     __NonExhaustive,
@@ -115,6 +116,7 @@ impl fmt::Display for TiffFormatError {
             Format(ref val) => write!(fmt, "Invalid format: {:?}.", val),
             RequiredTagEmpty(ref val) => write!(fmt, "Required tag {:?} was empty.", val),
             StripTileTagConflict => write!(fmt, "File should contain either (StripByteCounts and StripOffsets) or (TileByteCounts and TileOffsets), other combination was found."),
+            CycleInOffsets => write!(fmt, "File contained a cycle in the list of IFDs"),
             __NonExhaustive => unreachable!(),
         }
     }


Test patch:
diff --git a/tests/decode_images.rs b/tests/decode_images.rs
index cdb0461..7e33d17 100644
--- a/tests/decode_images.rs
+++ b/tests/decode_images.rs
@@ -383,6 +383,142 @@ fn test_stripped_image_overflow() {
     }
 }
 
+#[test]
+fn oom() {
+    let image = [
+        73, 73, 42, 0, 8, 0, 0, 0, 8, 0, 0, 1, 4, 0, 1, 0, 0, 0, 4, 0, 0, 0, 0, 0, 0, 40, 1, 0, 0,
+        0, 158, 0, 0, 251, 3, 1, 3, 0, 1, 0, 0, 0, 7, 0, 0, 0, 6, 1, 3, 0, 1, 0, 0, 0, 2, 0, 0, 0,
+        17, 1, 4, 0, 1, 0, 0, 0, 3, 77, 0, 0, 1, 1, 3, 0, 1, 0, 0, 0, 3, 128, 0, 0, 22, 1, 4, 0, 1,
+        0, 0, 0, 40, 0, 0, 0, 23, 1, 4, 0, 1, 0, 0, 0, 178, 48, 178, 178, 178, 178, 162, 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 oom_2() {
+    let image = [
+        73, 73, 42, 0, 8, 0, 0, 0, 8, 0, 0, 1, 4, 0, 1, 0, 0, 0, 4, 0, 0, 0, 0, 0, 0, 40, 1, 0, 0,
+        0, 158, 0, 0, 251, 3, 1, 3, 0, 1, 0, 0, 0, 5, 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, 1, 1, 3, 0, 1, 0, 0, 0, 3, 128, 0, 0, 22, 1, 4, 0, 1,
+        0, 0, 0, 40, 0, 0, 0, 23, 1, 4, 0, 1, 0, 0, 0, 48, 178, 178, 178, 0, 1, 0, 13, 13,
+    ];
+
+    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 invalid_jpeg_tag() {
+    use tiff::tags;
+    use tiff::{TiffError, TiffFormatError};
+
+    let image = [
+        73, 73, 42, 0, 8, 0, 0, 0, 15, 0, 0, 254, 44, 1, 0, 0, 0, 0, 0, 32, 0, 0, 0, 1, 4, 0, 1, 0,
+        0, 0, 0, 1, 0, 0, 91, 1, 1, 0, 0, 0, 0, 0, 242, 4, 0, 0, 0, 22, 0, 56, 77, 0, 77, 1, 0, 0,
+        73, 42, 0, 1, 4, 0, 1, 0, 0, 0, 4, 0, 8, 0, 0, 1, 4, 0, 1, 0, 0, 0, 158, 0, 0, 251, 3, 1,
+        3, 0, 1, 0, 0, 0, 7, 0, 0, 0, 6, 1, 3, 0, 1, 0, 0, 0, 2, 0, 0, 0, 17, 1, 4, 0, 1, 0, 0, 0,
+        0, 0, 0, 0, 1, 1, 3, 0, 1, 0, 0, 0, 0, 0, 0, 4, 61, 1, 18, 0, 1, 0, 0, 0, 202, 0, 0, 0, 17,
+        1, 100, 0, 129, 0, 0, 0, 0, 0, 0, 0, 232, 254, 252, 255, 254, 255, 255, 255, 1, 29, 0, 0,
+        22, 1, 3, 0, 1, 0, 0, 0, 16, 0, 0, 0, 23, 1, 1, 0, 1, 0, 0, 0, 0, 0, 0, 123, 73, 254, 0,
+        73,
+    ];
+
+    let mut decoder = tiff::decoder::Decoder::new(std::io::Cursor::new(&image)).unwrap();
+
+    let err = decoder.read_image().unwrap_err();
+
+    match err {
+        TiffError::FormatError(TiffFormatError::InvalidTagValueType(tags::Tag::JPEGTables)) => {}
+        unexpected => panic!("Unexpected error {}", unexpected),
+    }
+}
+
+#[test]
+fn invalid_jpeg_tag_2() {
+    use tiff::tags;
+    use tiff::{TiffError, TiffFormatError};
+
+    let image = [
+        73, 73, 42, 0, 8, 0, 0, 0, 16, 0, 254, 0, 4, 0, 1, 0, 0, 0, 0, 0, 0, 242, 0, 1, 4, 0, 1, 0,
+        0, 0, 0, 129, 16, 0, 1, 1, 4, 0, 1, 0, 0, 0, 214, 0, 0, 248, 253, 1, 3, 0, 1, 0, 0, 0, 64,
+        0, 0, 0, 3, 1, 3, 0, 1, 0, 0, 0, 7, 0, 0, 0, 6, 1, 3, 0, 1, 0, 0, 0, 1, 0, 0, 64, 14, 1, 0,
+        2, 0, 0, 148, 0, 206, 0, 0, 0, 17, 1, 4, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0,
+        1, 0, 0, 0, 22, 1, 4, 0, 17, 0, 0, 201, 1, 0, 0, 0, 23, 1, 2, 0, 20, 0, 0, 0, 194, 0, 0, 0,
+        91, 1, 7, 0, 5, 0, 0, 0, 64, 0, 0, 0, 237, 254, 65, 255, 255, 255, 255, 255, 1, 0, 0, 0,
+        22, 1, 4, 0, 1, 0, 0, 0, 42, 0, 0, 0, 23, 1, 255, 255, 255, 255, 255, 36, 36, 0, 0, 0, 0,
+        0, 0, 0, 1, 0, 0, 0, 1, 0, 0, 36, 73, 73, 0, 42, 36, 36, 36, 36, 0, 0, 8, 0,
+    ];
+
+    let mut decoder = tiff::decoder::Decoder::new(std::io::Cursor::new(&image)).unwrap();
+
+    let err = decoder.read_image().unwrap_err();
+
+    match err {
+        TiffError::FormatError(TiffFormatError::InvalidTagValueType(tags::Tag::JPEGTables)) => {}
+        unexpected => panic!("Unexpected error {}", unexpected),
+    }
+}
+
+#[test]
+fn multiple_images_strip_height() {
+    use tiff::TiffError;
+
+    let image = [
+        73, 73, 42, 0, 8, 0, 0, 0, 8, 0, 0, 1, 4, 0, 1, 0, 0, 0, 2, 0, 0, 0, 61, 1, 9, 0, 46, 22,
+        128, 0, 0, 0, 0, 1, 6, 1, 3, 0, 1, 0, 0, 0, 0, 0, 0, 0, 17, 1, 4, 0, 27, 0, 0, 0, 0, 0, 0,
+        0, 1, 1, 3, 0, 1, 0, 0, 0, 17, 1, 0, 231, 22, 1, 1, 0, 1, 0, 0, 0, 130, 0, 0, 0, 23, 1, 4,
+        0, 14, 0, 0, 0, 0, 0, 0, 0, 133, 133, 133, 77, 77, 77, 0, 0, 22, 128, 0, 255, 255, 255,
+        255, 255,
+    ];
+
+    let mut decoder = tiff::decoder::Decoder::new(std::io::Cursor::new(&image)).unwrap();
+
+    let _ = decoder.read_image().unwrap();
+    let err = decoder.read_image().unwrap_err();
+
+    match err {
+        TiffError::IntSizeError => {}
+        unexpected => panic!("Unexpected error {}", unexpected),
+    }
+}
+
+#[test]
+fn timeout() {
+    use tiff::{TiffError, TiffFormatError};
+
+    let image = [
+        73, 73, 42, 0, 8, 0, 0, 0, 16, 0, 254, 0, 4, 0, 1, 68, 0, 0, 0, 2, 0, 32, 254, 252, 0, 109,
+        0, 129, 0, 0, 0, 32, 0, 58, 0, 1, 4, 0, 1, 0, 6, 0, 0, 0, 8, 0, 0, 1, 73, 73, 42, 0, 8, 0,
+        0, 0, 8, 0, 0, 1, 4, 0, 1, 0, 0, 0, 21, 0, 0, 0, 61, 1, 255, 128, 9, 0, 0, 8, 0, 1, 113, 2,
+        3, 1, 3, 0, 1, 0, 0, 0, 5, 0, 65, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 8, 112, 0, 0, 36, 0, 0,
+        0, 112, 56, 200, 0, 5, 0, 0, 64, 0, 0, 1, 0, 4, 0, 0, 0, 2, 0, 6, 1, 3, 0, 1, 0, 0, 0, 0,
+        0, 0, 4, 17, 1, 1, 0, 93, 0, 0, 0, 0, 0, 0, 0, 1, 1, 1, 0, 1, 0, 0, 0, 3, 6, 0, 231, 22, 1,
+        1, 0, 1, 0, 0, 0, 2, 64, 118, 36, 23, 1, 1, 0, 43, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 4, 0, 8,
+        0, 0, 73, 73, 42, 0, 8, 0, 0, 0, 0, 0, 32,
+    ];
+
+    let error = tiff::decoder::Decoder::new(std::io::Cursor::new(&image)).unwrap_err();
+
+    match error {
+        TiffError::FormatError(TiffFormatError::CycleInOffsets) => {}
+        e => panic!("Unexpected error {:?}", e),
+    }
+}
+
 #[test]
 fn test_no_rows_per_strip() {
     test_image_sum_u8("no_rows_per_strip.tiff", ColorType::RGB(8), 99448840);