Merge pull request #32 from birktj/fix-bugs

Fix bugs, compiler warnings and add tests

Cargo.toml

@@ -9,7 +9,8 @@ authors = [
     "HeroicKatora",
     "Calum",
     "CensoredUsername <cens.username@gmail.com>",
-    "Robzz"]
+    "Robzz",
+    "birktj"]
 license = "MIT"
 description = "TIFF decoding and encoding library in pure Rust"
 repository = "https://github.com/PistonDevelopers/image-tiff"

src/decoder/ifd.rs

@@ -154,7 +154,7 @@ impl Entry {
         )
     }
 
-    pub fn val<R: Read + Seek>(&self, decoder: &mut super::Decoder<R>)
+    pub fn val<R: Read + Seek>(&self, limits: &super::Limits, decoder: &mut super::Decoder<R>)
     -> TiffResult<Value> {
         let bo = decoder.byte_order();
         match (self.type_, self.count) {
@@ -170,6 +170,9 @@ impl Entry {
                 ]))
             },
             (Type::SHORT, n) => {
+                if n as usize > limits.decoding_buffer_size / std::mem::size_of::<Value>() {
+                    return Err(TiffError::LimitsExceeded)
+                }
                 let mut v = Vec::with_capacity(n as usize);
                 try!(decoder.goto_offset(try!(self.r(bo).read_u32())));
                 for _ in 0 .. n {
@@ -179,6 +182,9 @@ impl Entry {
             },
             (Type::LONG, 1) => Ok(Unsigned(try!(self.r(bo).read_u32()))),
             (Type::LONG, n) => {
+                if n as usize > limits.decoding_buffer_size / std::mem::size_of::<Value>() {
+                    return Err(TiffError::LimitsExceeded)
+                }
                 let mut v = Vec::with_capacity(n as usize);
                 try!(decoder.goto_offset(try!(self.r(bo).read_u32())));
                 for _ in 0 .. n {
@@ -193,6 +199,9 @@ impl Entry {
                 Ok(Rational(numerator, denominator))
             },
             (Type::RATIONAL, n) => {
+                if n as usize > limits.decoding_buffer_size / std::mem::size_of::<Value>() {
+                    return Err(TiffError::LimitsExceeded)
+                }
                 let mut v = Vec::with_capacity(n as usize);
                 try!(decoder.goto_offset(try!(self.r(bo).read_u32())));
                 for _ in 0 .. n {
@@ -203,6 +212,9 @@ impl Entry {
                 Ok(List(v))
             },
             (Type::ASCII, n) => {
+                if n as usize > limits.decoding_buffer_size {
+                    return Err(TiffError::LimitsExceeded)
+                }
                 try!(decoder.goto_offset(try!(self.r(bo).read_u32())));
                 let string = try!(decoder.read_string(n as usize));
                 Ok(Ascii(string))

src/decoder/mod.rs

@@ -1,7 +1,6 @@
 use std::io::{self, Read, Seek};
 use num_traits::{FromPrimitive, Num};
 use std::collections::HashMap;
-use std::string::FromUtf8Error;
 
 use ::{ColorType, TiffError, TiffFormatError, TiffUnsupportedError, TiffResult};
 
@@ -28,7 +27,25 @@ pub enum DecodingResult {
 }
 
 impl DecodingResult {
-    pub fn to_buffer(&mut self, start: usize) -> DecodingBuffer {
+    fn new_u8(size: usize, limits: &Limits) -> TiffResult<DecodingResult> {
+        if size > limits.decoding_buffer_size {
+            Err(TiffError::LimitsExceeded)
+        }
+        else {
+            Ok(DecodingResult::U8(vec![0; size]))
+        }
+    }
+
+    fn new_u16(size: usize, limits: &Limits) -> TiffResult<DecodingResult> {
+        if size > limits.decoding_buffer_size / 2 {
+            Err(TiffError::LimitsExceeded)
+        }
+        else {
+            Ok(DecodingResult::U16(vec![0; size]))
+        }
+    }
+
+    pub fn as_buffer(&mut self, start: usize) -> DecodingBuffer {
         match self {
             DecodingResult::U8(ref mut buf) => DecodingBuffer::U8(&mut buf[start..]),
             DecodingResult::U16(ref mut buf) => DecodingBuffer::U16(&mut buf[start..]),
@@ -45,6 +62,20 @@ pub enum DecodingBuffer<'a> {
 }
 
 impl<'a> DecodingBuffer<'a> {
+    fn len(&self) -> usize {
+        match self {
+            DecodingBuffer::U8(ref buf) => buf.len(),
+            DecodingBuffer::U16(ref buf) => buf.len(),
+        }
+    }
+
+    fn byte_len(&self) -> usize {
+        match self {
+            DecodingBuffer::U8(_) => 1,
+            DecodingBuffer::U16(_) => 2,
+        }
+    }
+
     fn copy<'b>(&'b mut self) -> DecodingBuffer<'b> where 'a: 'b {
         match self {
             DecodingBuffer::U8(ref mut buf) => DecodingBuffer::U8(buf),
@@ -95,6 +126,28 @@ struct StripDecodeState {
     strip_bytes: Vec<u32>,
 }
 
+/// Decoding limits
+#[derive(Clone, Debug)]
+pub struct Limits {
+    /// The maximum size of any `DecodingResult` in bytes, the default is 
+    /// 256MiB. If the entire image is decoded at once, then this will
+    /// be the maximum size of the image. If it is decoded one strip at a
+    /// time, this will be the maximum size of a strip.
+    decoding_buffer_size: usize,
+    /// The maximum size of any ifd value in bytes, the default is 
+    /// 1MiB.
+    ifd_value_size: usize,
+}
+
+impl Default for Limits {
+    fn default() -> Limits {
+        Limits {
+            decoding_buffer_size: 256*1024*1024,
+            ifd_value_size: 1024*1024,
+        }
+    }
+}
+
 /// The representation of a TIFF decoder
 ///
 /// Currently does not support decoding of interlaced images
@@ -102,6 +155,7 @@ struct StripDecodeState {
 pub struct Decoder<R> where R: Read + Seek {
     reader: SmartReader<R>,
     byte_order: ByteOrder,
+    limits: Limits,
     next_ifd: Option<u32>,
     ifd: Option<Directory>,
     width: u32,
@@ -151,14 +205,15 @@ fn rev_hpredict(image: DecodingBuffer, size: (u32, u32), color_type: ColorType)
         ColorType::RGBA(8) | ColorType::RGBA(16) | ColorType::CMYK(8) => 4,
         _ => return Err(TiffError::UnsupportedError(TiffUnsupportedError::HorizontalPredictor(color_type)))
     };
-    Ok(match image {
+    match image {
         DecodingBuffer::U8(buf) => {
-            rev_hpredict_nsamp(buf, size, samples)
+            rev_hpredict_nsamp(buf, size, samples);
         },
         DecodingBuffer::U16(buf) => {
-            rev_hpredict_nsamp(buf, size, samples)
+            rev_hpredict_nsamp(buf, size, samples);
         }
-    })
+    }
+    Ok(())
 }
 
 impl<R: Read + Seek> Decoder<R> {
@@ -167,6 +222,7 @@ impl<R: Read + Seek> Decoder<R> {
         Decoder {
             reader: SmartReader::wrap(r, ByteOrder::LittleEndian),
             byte_order: ByteOrder::LittleEndian,
+            limits: Default::default(),
             next_ifd: None,
             ifd: None,
             width: 0,
@@ -179,6 +235,11 @@ impl<R: Read + Seek> Decoder<R> {
         }.init()
     }
 
+    pub fn with_limits(mut self, limits: Limits) -> Decoder<R> {
+        self.limits = limits;
+        self
+    }
+
     pub fn dimensions(&mut self) -> TiffResult<(u32, u32)> {
         Ok((self.width, self.height))
     }
@@ -271,10 +332,7 @@ impl<R: Read + Seek> Decoder<R> {
 
     /// Returns `true` if there is at least one more image available.
     pub fn more_images(&self) -> bool {
-        match self.next_ifd {
-            Some(_) => true,
-            None => false
-        }
+        self.next_ifd.is_some()
     }
 
     /// Returns the byte_order
@@ -296,12 +354,12 @@ impl<R: Read + Seek> Decoder<R> {
 
     /// Reads a string
     #[inline]
-    pub fn read_string(&mut self, length: usize) -> Result<String, FromUtf8Error> {
+    pub fn read_string(&mut self, length: usize) -> TiffResult<String> {
         let mut out = String::with_capacity(length);
-        self.reader.read_to_string(&mut out);
+        self.reader.read_to_string(&mut out)?;
         // Strings may be null-terminated, so we trim anything downstream of the null byte
         let trimmed = out.bytes().take_while(|&n| n != 0).collect::<Vec<u8>>();
-        String::from_utf8(trimmed)
+        Ok(String::from_utf8(trimmed)?)
     }
 
     /// Reads a TIFF IFA offset/value field
@@ -373,7 +431,9 @@ impl<R: Read + Seek> Decoder<R> {
             Some(entry) => entry.clone(),
         };
 
-        Ok(Some(try!(entry.val(self))))
+        let limits = self.limits.clone();
+
+        Ok(Some(try!(entry.val(&limits, self))))
     }
 
     /// Tries to retrieve a tag and convert it to the desired type.
@@ -432,6 +492,11 @@ impl<R: Read + Seek> Decoder<R> {
             },
             method => return Err(TiffError::UnsupportedError(TiffUnsupportedError::UnsupportedCompressionMethod(method)))
         };
+        
+        if bytes / buffer.byte_len() > max_uncompressed_length {
+            return Err(TiffError::FormatError(TiffFormatError::InconsistentSizesEncountered));
+        }
+
         Ok(match (color_type, buffer) {
             (ColorType:: RGB(8), DecodingBuffer::U8(ref mut buffer)) |
             (ColorType::RGBA(8), DecodingBuffer::U8(ref mut buffer)) |
@@ -492,15 +557,21 @@ impl<R: Read + Seek> Decoder<R> {
         self.initialize_strip_decoder()?;
 
         let index = self.strip_decoder.as_ref().unwrap().strip_index;
-        let offset = self.strip_decoder.as_ref().unwrap().strip_offsets[index];
-        let byte_count = self.strip_decoder.as_ref().unwrap().strip_bytes[index];
+        let offset = *self.strip_decoder.as_ref().unwrap().strip_offsets.get(index)
+            .ok_or(TiffError::FormatError(TiffFormatError::InconsistentSizesEncountered))?;
+        let byte_count = *self.strip_decoder.as_ref().unwrap().strip_bytes.get(index)
+            .ok_or(TiffError::FormatError(TiffFormatError::InconsistentSizesEncountered))?;
 
         let rows_per_strip = self.get_tag_u32(ifd::Tag::RowsPerStrip)
             .unwrap_or(self.height) as usize;
 
         let strip_height = std::cmp::min(rows_per_strip, self.height as usize - index * rows_per_strip);
 
-        let buffer_size = self.width as usize * strip_height * self.bits_per_sample.iter().count();
+        let buffer_size = self.width as usize * strip_height * self.bits_per_sample.len();
+
+        if buffer.len() < buffer_size || byte_count as usize / buffer.byte_len() > buffer_size {
+            return Err(TiffError::FormatError(TiffFormatError::InconsistentSizesEncountered));
+        }
 
         let units_read = self.expand_strip(buffer.copy(), offset, byte_count, buffer_size)?;
 
@@ -542,12 +613,12 @@ impl<R: Read + Seek> Decoder<R> {
         let buffer_size = self.width as usize * strip_height * self.bits_per_sample.iter().count();
 
         let mut result = match self.bits_per_sample.iter().cloned().max().unwrap_or(8) {
-            n if n <= 8 => DecodingResult::U8(vec![0; buffer_size]),
-            n if n <= 16 => DecodingResult::U16(vec![0; buffer_size]),
+            n if n <= 8 => DecodingResult::new_u8(buffer_size, &self.limits)?,
+            n if n <= 16 => DecodingResult::new_u16(buffer_size, &self.limits)?,
             n => return Err(TiffError::UnsupportedError(TiffUnsupportedError::UnsupportedBitsPerChannel(n))),
         };
 
-        self.read_strip_to_buffer(result.to_buffer(0))?;
+        self.read_strip_to_buffer(result.as_buffer(0))?;
 
         Ok(result)
     }
@@ -563,13 +634,13 @@ impl<R: Read + Seek> Decoder<R> {
         let buffer_size = self.width as usize * self.height as usize * self.bits_per_sample.iter().count();
 
         let mut result = match self.bits_per_sample.iter().cloned().max().unwrap_or(8) {
-            n if n <= 8 => DecodingResult::U8(vec![0; buffer_size]),
-            n if n <= 16 => DecodingResult::U16(vec![0; buffer_size]),
+            n if n <= 8 => DecodingResult::new_u8(buffer_size, &self.limits)?,
+            n if n <= 16 => DecodingResult::new_u16(buffer_size, &self.limits)?,
             n => return Err(TiffError::UnsupportedError(TiffUnsupportedError::UnsupportedBitsPerChannel(n))),
         };
 
         for i in 0..self.strip_count()? as usize {
-            self.read_strip_to_buffer(result.to_buffer(samples_per_strip * i))?;
+            self.read_strip_to_buffer(result.as_buffer(samples_per_strip * i))?;
         }
 
         Ok(result)

src/decoder/stream.rs

@@ -101,7 +101,11 @@ impl PackBitsReader {
         let mut buffer = Vec::new();
         let mut read: usize = 0;
         while read < length {
-            read += try!(read_packbits_run(&mut reader, &mut buffer));
+            let lread = read_packbits_run(&mut reader, &mut buffer)?;
+            if lread == 0 {
+                return Err(io::ErrorKind::UnexpectedEof.into())
+            }
+            read += lread;
         }
         Ok((buffer.len(), PackBitsReader {
             buffer: io::Cursor::new(buffer),

src/error.rs

@@ -18,6 +18,9 @@ pub enum TiffError {
 
     /// An I/O Error occurred while decoding the image
     IoError(io::Error),
+
+    /// The Limits of the Decoder is exceeded,
+    LimitsExceeded,
 }
 
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]
@@ -89,6 +92,7 @@ impl fmt::Display for TiffError {
             TiffError::UnsupportedError(ref f) => write!(fmt, "The Decoder does not support the \
                                                                  image format `{}`", f),
             TiffError::IoError(ref e) => e.fmt(fmt),
+            TiffError::LimitsExceeded => write!(fmt, "The Decoder limits are exceeded"),
         }
     }
 }
@@ -99,6 +103,7 @@ impl Error for TiffError {
             TiffError::FormatError(..) => "Format error",
             TiffError::UnsupportedError(..) => "Unsupported error",
             TiffError::IoError(..) => "IO error",
+            TiffError::LimitsExceeded => "Decoder limits exceeded",
         }
     }
 

tests/encode_images.rs

@@ -139,8 +139,6 @@ fn test_rgb_u16_roundtrip()
     let img_file = File::open("./tests/images/rgb-3c-16b.tiff").expect("Cannot find test image!");
     let mut decoder = Decoder::new(img_file).expect("Cannot create decoder");
     assert_eq!(decoder.colortype().unwrap(), ColorType::RGB(16));
-    let img_res = decoder.read_image();
-    assert!(img_res.is_ok());
 
     let image_data = match decoder.read_image().unwrap() {
         DecodingResult::U16(res) => res,

tests/fuzz_tests.rs

@@ -0,0 +1,42 @@
+extern crate tiff;
+
+use tiff::TiffResult;
+use tiff::decoder::Decoder;
+
+use std::fs::File;
+
+fn test_directory<F: Fn(File) -> bool>(path: &str, f: F) {
+    for entry in std::fs::read_dir(path).unwrap() {
+        let file = File::open(entry.unwrap().path()).unwrap();
+        assert!(f(file));
+    }
+}
+
+fn decode_tiff(file: File) -> TiffResult<()> {
+    let mut decoder = Decoder::new(file)?;
+    decoder.read_image()?;
+    Ok(())
+}
+
+#[test]
+fn oor_panic() {
+    test_directory("./tests/fuzz_images/oor_panic", |file| {
+        decode_tiff(file).ok();
+        true
+    });
+}
+
+#[test]
+fn oom_crash() {
+    test_directory("./tests/fuzz_images/oom_crash", |file| {
+        decode_tiff(file).is_err()
+    });
+}
+
+#[test]
+fn inf_loop() {
+    test_directory("./tests/fuzz_images/inf_loop", |file| {
+        decode_tiff(file).ok();
+        true
+    });
+}