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,10 @@ impl Entry {
  ]))
  },
  (Type::SHORT, n) => {
+ // each value is ~8 bytes
+ if u64::from(n)*8 > limits.decoding_buffer_size {
+ 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 +183,10 @@ impl Entry {
  },
  (Type::LONG, 1) => Ok(Unsigned(try!(self.r(bo).read_u32()))),
  (Type::LONG, n) => {
+ // each value is ~8 bytes
+ if u64::from(n)*8 > limits.decoding_buffer_size {
+ 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 +201,10 @@ impl Entry {
  Ok(Rational(numerator, denominator))
  },
  (Type::RATIONAL, n) => {
+ // each value is ~8 bytes
+ if u64::from(n)*8 > limits.decoding_buffer_size {
+ 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 +215,9 @@ impl Entry {
  Ok(List(v))
  },
  (Type::ASCII, n) => {
+ if u64::from(n) > 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 as u64 > 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 as u64*2 > limits.decoding_buffer_size {
+ 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,13 +126,36 @@ 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: u64,
+ /// The maximum size of any ifd value in bytes, the default is 
+ /// 1MiB.
+ ifd_value_size: u64,
+}
+
+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
 #[derive(Debug)]
 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 > max_uncompressed_length*buffer.byte_len() {
+ 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_size*buffer.byte_len() {
+ 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
+ });
+}