From 4a2e76858b976ae1e9838f4b20772543e3940944 Mon Sep 17 00:00:00 2001 From: Jonathan Behrens Date: Wed, 4 Oct 2023 09:45:03 -0700 Subject: [PATCH] Allow arbitrary number of samples as long as all have the same bit depth (#215) --- src/decoder/image.rs | 68 +++++++++++++++++++++++++----------------- src/decoder/mod.rs | 10 ++----- src/error.rs | 6 ++++ tests/decode_images.rs | 15 ++-------- 4 files changed, 51 insertions(+), 48 deletions(-) diff --git a/src/decoder/image.rs b/src/decoder/image.rs index dfcd56e1..55e1e996 100644 --- a/src/decoder/image.rs +++ b/src/decoder/image.rs @@ -7,7 +7,7 @@ use crate::tags::{ CompressionMethod, PhotometricInterpretation, PlanarConfiguration, Predictor, SampleFormat, Tag, }; use crate::{ColorType, TiffError, TiffFormatError, TiffResult, TiffUnsupportedError, UsageError}; -use std::convert::{TryFrom, TryInto}; +use std::convert::TryFrom; use std::io::{self, Cursor, Read, Seek}; use std::sync::Arc; @@ -64,9 +64,9 @@ pub(crate) struct Image { pub ifd: Option, pub width: u32, pub height: u32, - pub bits_per_sample: Vec, + pub bits_per_sample: u8, #[allow(unused)] - pub samples: u8, + pub samples: u16, pub sample_format: Vec, pub photometric_interpretation: PhotometricInterpretation, pub compression_method: CompressionMethod, @@ -134,12 +134,14 @@ impl Image { None }; - let samples = tag_reader + let samples: u16 = tag_reader .find_tag(Tag::SamplesPerPixel)? .map(Value::into_u16) .transpose()? - .unwrap_or(1) - .try_into()?; + .unwrap_or(1); + if samples == 0 { + return Err(TiffFormatError::SamplesPerPixelIsZero.into()); + } let sample_format = match tag_reader.find_tag_uint_vec(Tag::SampleFormat)? { Some(vals) => { @@ -158,12 +160,22 @@ impl Image { None => vec![SampleFormat::Uint], }; - let bits_per_sample = match samples { - 1 | 3 | 4 => tag_reader - .find_tag_uint_vec(Tag::BitsPerSample)? - .unwrap_or_else(|| vec![1]), - _ => return Err(TiffUnsupportedError::UnsupportedSampleDepth(samples).into()), - }; + let bits_per_sample: Vec = tag_reader + .find_tag_uint_vec(Tag::BitsPerSample)? + .unwrap_or_else(|| vec![1]); + + // Technically bits_per_sample.len() should be *equal* to samples, but libtiff also allows + // it to be a single value that applies to all samples. + if bits_per_sample.len() != samples.into() && bits_per_sample.len() != 1 { + return Err(TiffError::FormatError( + TiffFormatError::InconsistentSizesEncountered, + )); + } + + // This library (and libtiff) do not support mixed sample formats. + if bits_per_sample.iter().any(|&b| b != bits_per_sample[0]) { + return Err(TiffUnsupportedError::InconsistentBitsPerSample(bits_per_sample).into()); + } let predictor = tag_reader .find_tag(Tag::Predictor)? @@ -284,7 +296,7 @@ impl Image { ifd: Some(ifd), width, height, - bits_per_sample, + bits_per_sample: bits_per_sample[0], samples, sample_format, photometric_interpretation, @@ -302,9 +314,9 @@ impl Image { pub(crate) fn colortype(&self) -> TiffResult { match self.photometric_interpretation { - PhotometricInterpretation::RGB => match self.bits_per_sample[..] { - [r, g, b] if [r, r] == [g, b] => Ok(ColorType::RGB(r)), - [r, g, b, a] if [r, r, r] == [g, b, a] => Ok(ColorType::RGBA(r)), + PhotometricInterpretation::RGB => match self.samples { + 3 => Ok(ColorType::RGB(self.bits_per_sample)), + 4 => Ok(ColorType::RGBA(self.bits_per_sample)), // FIXME: We should _ignore_ other components. In particular: // > Beware of extra components. Some TIFF files may have more components per pixel // than you think. A Baseline TIFF reader must skip over them gracefully,using the @@ -313,39 +325,39 @@ impl Image { _ => Err(TiffError::UnsupportedError( TiffUnsupportedError::InterpretationWithBits( self.photometric_interpretation, - self.bits_per_sample.clone(), + vec![self.bits_per_sample; self.samples as usize], ), )), }, - PhotometricInterpretation::CMYK => match self.bits_per_sample[..] { - [c, m, y, k] if [c, c, c] == [m, y, k] => Ok(ColorType::CMYK(c)), + PhotometricInterpretation::CMYK => match self.samples { + 4 => Ok(ColorType::CMYK(self.bits_per_sample)), _ => Err(TiffError::UnsupportedError( TiffUnsupportedError::InterpretationWithBits( self.photometric_interpretation, - self.bits_per_sample.clone(), + vec![self.bits_per_sample; self.samples as usize], ), )), }, - PhotometricInterpretation::YCbCr => match self.bits_per_sample[..] { - [y, cb, cr] if [y, y] == [cb, cr] => Ok(ColorType::YCbCr(y)), + PhotometricInterpretation::YCbCr => match self.samples { + 3 => Ok(ColorType::YCbCr(self.bits_per_sample)), _ => Err(TiffError::UnsupportedError( TiffUnsupportedError::InterpretationWithBits( self.photometric_interpretation, - self.bits_per_sample.clone(), + vec![self.bits_per_sample; self.samples as usize], ), )), }, PhotometricInterpretation::BlackIsZero | PhotometricInterpretation::WhiteIsZero - if self.bits_per_sample.len() == 1 => + if self.samples == 1 => { - Ok(ColorType::Gray(self.bits_per_sample[0])) + Ok(ColorType::Gray(self.bits_per_sample)) } // TODO: this is bad we should not fail at this point _ => Err(TiffError::UnsupportedError( TiffUnsupportedError::InterpretationWithBits( self.photometric_interpretation, - self.bits_per_sample.clone(), + vec![self.bits_per_sample; self.samples as usize], ), )), } @@ -448,7 +460,7 @@ impl Image { /// * `PlanarConfiguration::Planar` -> 1 (RRR...) (GGG...) (BBB...) pub(crate) fn samples_per_pixel(&self) -> usize { match self.planar_config { - PlanarConfiguration::Chunky => self.bits_per_sample.len(), + PlanarConfiguration::Chunky => self.samples.into(), PlanarConfiguration::Planar => 1, } } @@ -457,7 +469,7 @@ impl Image { pub(crate) fn strips_per_pixel(&self) -> usize { match self.planar_config { PlanarConfiguration::Chunky => 1, - PlanarConfiguration::Planar => self.bits_per_sample.len(), + PlanarConfiguration::Planar => self.samples.into(), } } diff --git a/src/decoder/mod.rs b/src/decoder/mod.rs index f3a847cf..cf2c18ed 100644 --- a/src/decoder/mod.rs +++ b/src/decoder/mod.rs @@ -572,7 +572,7 @@ impl Decoder { ifd: None, width: 0, height: 0, - bits_per_sample: vec![1], + bits_per_sample: 1, samples: 1, sample_format: vec![SampleFormat::Uint], photometric_interpretation: PhotometricInterpretation::BlackIsZero, @@ -1064,13 +1064,7 @@ impl Decoder { None => return Err(TiffError::LimitsExceeded), }; - let max_sample_bits = self - .image() - .bits_per_sample - .iter() - .cloned() - .max() - .unwrap_or(8); + let max_sample_bits = self.image().bits_per_sample; match self .image() .sample_format diff --git a/src/error.rs b/src/error.rs index fe001049..c6fe8ed5 100644 --- a/src/error.rs +++ b/src/error.rs @@ -75,6 +75,7 @@ pub enum TiffFormatError { StripTileTagConflict, CycleInOffsets, JpegDecoder(JpegDecoderError), + SamplesPerPixelIsZero, } impl fmt::Display for TiffFormatError { @@ -129,6 +130,7 @@ impl fmt::Display for TiffFormatError { 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"), JpegDecoder(ref error) => write!(fmt, "{}", error), + SamplesPerPixelIsZero => write!(fmt, "Samples per pixel is zero"), } } } @@ -146,6 +148,7 @@ impl fmt::Display for TiffFormatError { pub enum TiffUnsupportedError { FloatingPointPredictor(ColorType), HorizontalPredictor(ColorType), + InconsistentBitsPerSample(Vec), InterpretationWithBits(PhotometricInterpretation, Vec), UnknownInterpretation, UnknownCompressionMethod, @@ -174,6 +177,9 @@ impl fmt::Display for TiffUnsupportedError { "Horizontal predictor for {:?} is unsupported.", color_type ), + InconsistentBitsPerSample(ref bits_per_sample) => { + write!(fmt, "Inconsistent bits per sample: {:?}.", bits_per_sample) + } InterpretationWithBits(ref photometric_interpretation, ref bits_per_sample) => write!( fmt, "{:?} with {:?} bits per sample is unsupported", diff --git a/tests/decode_images.rs b/tests/decode_images.rs index f82c6ec0..77f48e9d 100644 --- a/tests/decode_images.rs +++ b/tests/decode_images.rs @@ -375,9 +375,7 @@ fn fuzzer_testcase5() { 178, 178, 178, ]; - let mut decoder = tiff::decoder::Decoder::new(std::io::Cursor::new(&image)).unwrap(); - - let _ = decoder.read_image().unwrap_err(); + let _ = tiff::decoder::Decoder::new(std::io::Cursor::new(&image)).unwrap_err(); } #[test] @@ -394,7 +392,7 @@ fn fuzzer_testcase1() { } #[test] -fn test_stripped_image_overflow() { +fn fuzzer_testcase6() { 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, @@ -403,14 +401,7 @@ fn test_stripped_image_overflow() { 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), - } + let _ = tiff::decoder::Decoder::new(std::io::Cursor::new(&image)).unwrap_err(); } #[test]