From 9394f6d04319d09ef9c072c0aa4d7e808bbed5b0 Mon Sep 17 00:00:00 2001 From: Jonathan Behrens Date: Thu, 14 Apr 2022 20:56:37 -0400 Subject: [PATCH 1/6] Make LZWReader incrementally decompress input --- src/decoder/mod.rs | 2 +- src/decoder/stream.rs | 125 ++++++++++++++++++++++++++---------------- 2 files changed, 79 insertions(+), 48 deletions(-) diff --git a/src/decoder/mod.rs b/src/decoder/mod.rs index 8ba323b..faea344 100644 --- a/src/decoder/mod.rs +++ b/src/decoder/mod.rs @@ -1163,7 +1163,7 @@ impl Decoder { return Err(TiffError::LimitsExceeded); } - Box::new(LZWReader::new(reader, clen, samples * byte_len)?.1) + Box::new(LZWReader::new(reader, clen, samples * byte_len)) } CompressionMethod::PackBits => { Box::new(PackBitsReader::new(reader, usize::try_from(compressed_length)?)?.1) diff --git a/src/decoder/stream.rs b/src/decoder/stream.rs index bd247a4..c9c60fd 100644 --- a/src/decoder/stream.rs +++ b/src/decoder/stream.rs @@ -131,63 +131,94 @@ pub type DeflateReader<'r, R> = flate2::read::ZlibDecoder<&'r mut SmartReader /// /// Reader that decompresses LZW streams -pub struct LZWReader { - buffer: io::Cursor>, +pub struct LZWReader { + compressed_buf: Box<[u8]>, + compressed_buf_length: usize, + compressed_bytes_read: usize, + + reader: R, + decoder: weezl::decode::Decoder, + input_remaining: usize, + output_remaining: usize, } -impl LZWReader { +impl LZWReader { /// Wraps a reader - pub fn new( - reader: &mut SmartReader, + pub fn new( + reader: R, compressed_length: usize, max_uncompressed_length: usize, - ) -> io::Result<(usize, LZWReader)> - where - R: Read + Seek, - { - let mut compressed = vec![0; compressed_length as usize]; - reader.read_exact(&mut compressed[..])?; - let mut uncompressed = vec![0; max_uncompressed_length]; - let mut decoder = weezl::decode::Decoder::with_tiff_size_switch(weezl::BitOrder::Msb, 8); - - let mut bytes_read = 0; - let mut bytes_written = 0; - - while bytes_written < max_uncompressed_length { - let result = decoder.decode_bytes( - &compressed[bytes_read..], - &mut uncompressed[bytes_written..], - ); - bytes_read += result.consumed_in; - bytes_written += result.consumed_out; - - match result.status { - Ok(weezl::LzwStatus::Ok) => {} - Ok(weezl::LzwStatus::Done) => break, - Ok(weezl::LzwStatus::NoProgress) => { - return Err(io::Error::new( - io::ErrorKind::UnexpectedEof, - "no lzw end code found", - )) - } - Err(err) => return Err(io::Error::new(io::ErrorKind::InvalidData, err)), - } + ) -> LZWReader { + Self { + compressed_buf: vec![0; (32 * 1024).min(compressed_length)].into_boxed_slice(), + compressed_buf_length: 0, + compressed_bytes_read: 0, + reader, + decoder: weezl::decode::Decoder::with_tiff_size_switch(weezl::BitOrder::Msb, 8), + input_remaining: compressed_length, + output_remaining: max_uncompressed_length, } - - uncompressed.resize(bytes_written, 0); - Ok(( - uncompressed.len(), - LZWReader { - buffer: io::Cursor::new(uncompressed), - }, - )) } } -impl Read for LZWReader { - #[inline] +impl Read for LZWReader { fn read(&mut self, buf: &mut [u8]) -> io::Result { - self.buffer.read(buf) + if self.output_remaining == 0 { + return Ok(0); + } + + if self.compressed_buf_length == 0 && self.input_remaining > 0 { + let max_input_length = self.compressed_buf.len().min(self.input_remaining); + self.compressed_bytes_read = 0; + self.compressed_buf_length = self + .reader + .read(&mut self.compressed_buf[..max_input_length])?; + self.input_remaining -= self.compressed_buf_length; + } + + let max_output_length = self.output_remaining.min(buf.len()); + let result = self.decoder.decode_bytes( + &self.compressed_buf[self.compressed_bytes_read..self.compressed_buf_length], + &mut buf[..max_output_length], + ); + + self.compressed_bytes_read += result.consumed_in; + self.output_remaining -= result.consumed_out; + + match result.status { + Ok(weezl::LzwStatus::Ok) => { + if result.consumed_out == 0 { + return self.read(buf); + } + } + Ok(weezl::LzwStatus::NoProgress) => { + assert_eq!(result.consumed_out, 0); + if self.compressed_bytes_read != 0 { + self.compressed_buf + .copy_within(self.compressed_bytes_read..self.compressed_buf_length, 0); + self.compressed_buf_length -= self.compressed_bytes_read; + self.compressed_bytes_read = 0; + } + if self.compressed_buf_length < self.compressed_buf.len() + && self.input_remaining > 0 + { + self.compressed_buf_length += self + .reader + .read(&mut self.compressed_buf[self.compressed_buf_length..])?; + return self.read(buf); + } else { + return Err(io::Error::new( + io::ErrorKind::UnexpectedEof, + "no lzw end code found", + )); + } + } + Ok(weezl::LzwStatus::Done) => { + self.output_remaining = 0; + } + Err(err) => return Err(io::Error::new(io::ErrorKind::InvalidData, err)), + } + Ok(result.consumed_out) } } From e4eac36c1c52ca0b2100fed093306be2483e6c9a Mon Sep 17 00:00:00 2001 From: Jonathan Behrens Date: Thu, 14 Apr 2022 21:17:06 -0400 Subject: [PATCH 2/6] Remove now unneeded limit check --- src/decoder/mod.rs | 18 +++++------------- tests/decode_images.rs | 2 +- 2 files changed, 6 insertions(+), 14 deletions(-) diff --git a/src/decoder/mod.rs b/src/decoder/mod.rs index faea344..d229700 100644 --- a/src/decoder/mod.rs +++ b/src/decoder/mod.rs @@ -1038,7 +1038,6 @@ impl Decoder { length, buffer.len(), buffer.byte_len(), - self.limits.intermediate_buffer_size, )?; // Read into output buffer. @@ -1106,7 +1105,6 @@ impl Decoder { compressed_length, tile_samples, byte_len, - self.limits.intermediate_buffer_size, )?; for row in 0..(tile_length - padding_down) { @@ -1151,20 +1149,14 @@ impl Decoder { 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> { Ok(match compression_method { CompressionMethod::None => Box::new(reader), - 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)) - } + CompressionMethod::LZW => Box::new(LZWReader::new( + reader, + usize::try_from(compressed_length)?, + samples * byte_len, + )), CompressionMethod::PackBits => { Box::new(PackBitsReader::new(reader, usize::try_from(compressed_length)?)?.1) } diff --git a/tests/decode_images.rs b/tests/decode_images.rs index 5f32de1..dc0ab22 100644 --- a/tests/decode_images.rs +++ b/tests/decode_images.rs @@ -409,7 +409,7 @@ fn oom_2() { let err = decoder.read_image().unwrap_err(); match err { - tiff::TiffError::LimitsExceeded => {} + tiff::TiffError::IoError(_) => {} unexpected => panic!("Unexpected error {}", unexpected), } } From 1063d6dd27b9c7dd6bba4d8597f8223b2601a316 Mon Sep 17 00:00:00 2001 From: Jonathan Behrens Date: Fri, 15 Apr 2022 09:49:13 -0400 Subject: [PATCH 3/6] Have LZWReader internally use a BufReader> --- src/decoder/stream.rs | 84 ++++++++++++++++--------------------------- 1 file changed, 30 insertions(+), 54 deletions(-) diff --git a/src/decoder/stream.rs b/src/decoder/stream.rs index c9c60fd..67d91e6 100644 --- a/src/decoder/stream.rs +++ b/src/decoder/stream.rs @@ -1,6 +1,7 @@ //! All IO functionality needed for TIFF decoding -use std::io::{self, Read, Seek}; +use std::convert::TryFrom; +use std::io::{self, BufRead, BufReader, Read, Seek, Take}; /// Byte order of the TIFF file. #[derive(Clone, Copy, Debug)] @@ -132,13 +133,8 @@ pub type DeflateReader<'r, R> = flate2::read::ZlibDecoder<&'r mut SmartReader /// Reader that decompresses LZW streams pub struct LZWReader { - compressed_buf: Box<[u8]>, - compressed_buf_length: usize, - compressed_bytes_read: usize, - - reader: R, + reader: BufReader>, decoder: weezl::decode::Decoder, - input_remaining: usize, output_remaining: usize, } @@ -150,12 +146,11 @@ impl LZWReader { max_uncompressed_length: usize, ) -> LZWReader { Self { - compressed_buf: vec![0; (32 * 1024).min(compressed_length)].into_boxed_slice(), - compressed_buf_length: 0, - compressed_bytes_read: 0, - reader, + reader: BufReader::with_capacity( + (32 * 1024).min(compressed_length), + reader.take(u64::try_from(compressed_length).unwrap()), + ), decoder: weezl::decode::Decoder::with_tiff_size_switch(weezl::BitOrder::Msb, 8), - input_remaining: compressed_length, output_remaining: max_uncompressed_length, } } @@ -167,58 +162,39 @@ impl Read for LZWReader { return Ok(0); } - if self.compressed_buf_length == 0 && self.input_remaining > 0 { - let max_input_length = self.compressed_buf.len().min(self.input_remaining); - self.compressed_bytes_read = 0; - self.compressed_buf_length = self - .reader - .read(&mut self.compressed_buf[..max_input_length])?; - self.input_remaining -= self.compressed_buf_length; - } + loop { + let max_output_length = self.output_remaining.min(buf.len()); + let result = self + .decoder + .decode_bytes(self.reader.fill_buf()?, &mut buf[..max_output_length]); - let max_output_length = self.output_remaining.min(buf.len()); - let result = self.decoder.decode_bytes( - &self.compressed_buf[self.compressed_bytes_read..self.compressed_buf_length], - &mut buf[..max_output_length], - ); + self.reader.consume(result.consumed_in); + self.output_remaining -= result.consumed_out; - self.compressed_bytes_read += result.consumed_in; - self.output_remaining -= result.consumed_out; - - match result.status { - Ok(weezl::LzwStatus::Ok) => { - if result.consumed_out == 0 { - return self.read(buf); + match result.status { + Ok(weezl::LzwStatus::Ok) => { + if result.consumed_out == 0 { + continue; + } else { + return Ok(result.consumed_out); + } } - } - Ok(weezl::LzwStatus::NoProgress) => { - assert_eq!(result.consumed_out, 0); - if self.compressed_bytes_read != 0 { - self.compressed_buf - .copy_within(self.compressed_bytes_read..self.compressed_buf_length, 0); - self.compressed_buf_length -= self.compressed_bytes_read; - self.compressed_bytes_read = 0; - } - if self.compressed_buf_length < self.compressed_buf.len() - && self.input_remaining > 0 - { - self.compressed_buf_length += self - .reader - .read(&mut self.compressed_buf[self.compressed_buf_length..])?; - return self.read(buf); - } else { + Ok(weezl::LzwStatus::NoProgress) => { + assert_eq!(result.consumed_in, 0); + assert_eq!(result.consumed_out, 0); + assert!(self.reader.buffer().is_empty()); return Err(io::Error::new( io::ErrorKind::UnexpectedEof, "no lzw end code found", )); } + Ok(weezl::LzwStatus::Done) => { + self.output_remaining = 0; + return Ok(result.consumed_out); + } + Err(err) => return Err(io::Error::new(io::ErrorKind::InvalidData, err)), } - Ok(weezl::LzwStatus::Done) => { - self.output_remaining = 0; - } - Err(err) => return Err(io::Error::new(io::ErrorKind::InvalidData, err)), } - Ok(result.consumed_out) } } From c910ef8534d97e14f00ecc78ebfd27c28dbcb7be Mon Sep 17 00:00:00 2001 From: Jonathan Behrens Date: Fri, 15 Apr 2022 10:02:08 -0400 Subject: [PATCH 4/6] Remove redundant limit on LZWReader output length --- src/decoder/mod.rs | 15 +++------------ src/decoder/stream.rs | 13 +------------ 2 files changed, 4 insertions(+), 24 deletions(-) diff --git a/src/decoder/mod.rs b/src/decoder/mod.rs index d229700..04f00a7 100644 --- a/src/decoder/mod.rs +++ b/src/decoder/mod.rs @@ -1036,8 +1036,6 @@ impl Decoder { &mut self.reader, compression_method, length, - buffer.len(), - buffer.byte_len(), )?; // Read into output buffer. @@ -1088,7 +1086,6 @@ impl Decoder { let tile_attrs = self.image().tile_attributes.as_ref().unwrap(); let (padding_right, padding_down) = tile_attrs.get_padding(tile); - let tile_samples = tile_attrs.tile_samples(); let tile_length = tile_attrs.tile_length; let row_samples = tile_attrs.row_samples(); let padding_right_samples = padding_right * self.image().bits_per_sample.len(); @@ -1103,8 +1100,6 @@ impl Decoder { &mut self.reader, compression_method, compressed_length, - tile_samples, - byte_len, )?; for row in 0..(tile_length - padding_down) { @@ -1147,16 +1142,12 @@ impl Decoder { reader: &'r mut SmartReader, compression_method: CompressionMethod, compressed_length: u64, - samples: usize, // Expected chunk length in samples - byte_len: usize, // Byte length of the samples in result buffer ) -> TiffResult> { Ok(match compression_method { CompressionMethod::None => Box::new(reader), - CompressionMethod::LZW => Box::new(LZWReader::new( - reader, - usize::try_from(compressed_length)?, - samples * byte_len, - )), + CompressionMethod::LZW => { + Box::new(LZWReader::new(reader, usize::try_from(compressed_length)?)) + } CompressionMethod::PackBits => { Box::new(PackBitsReader::new(reader, usize::try_from(compressed_length)?)?.1) } diff --git a/src/decoder/stream.rs b/src/decoder/stream.rs index 67d91e6..37a1a1d 100644 --- a/src/decoder/stream.rs +++ b/src/decoder/stream.rs @@ -135,7 +135,6 @@ pub type DeflateReader<'r, R> = flate2::read::ZlibDecoder<&'r mut SmartReader pub struct LZWReader { reader: BufReader>, decoder: weezl::decode::Decoder, - output_remaining: usize, } impl LZWReader { @@ -143,7 +142,6 @@ impl LZWReader { pub fn new( reader: R, compressed_length: usize, - max_uncompressed_length: usize, ) -> LZWReader { Self { reader: BufReader::with_capacity( @@ -151,25 +149,17 @@ impl LZWReader { reader.take(u64::try_from(compressed_length).unwrap()), ), decoder: weezl::decode::Decoder::with_tiff_size_switch(weezl::BitOrder::Msb, 8), - output_remaining: max_uncompressed_length, } } } impl Read for LZWReader { fn read(&mut self, buf: &mut [u8]) -> io::Result { - if self.output_remaining == 0 { - return Ok(0); - } - loop { - let max_output_length = self.output_remaining.min(buf.len()); let result = self .decoder - .decode_bytes(self.reader.fill_buf()?, &mut buf[..max_output_length]); - + .decode_bytes(self.reader.fill_buf()?, buf); self.reader.consume(result.consumed_in); - self.output_remaining -= result.consumed_out; match result.status { Ok(weezl::LzwStatus::Ok) => { @@ -189,7 +179,6 @@ impl Read for LZWReader { )); } Ok(weezl::LzwStatus::Done) => { - self.output_remaining = 0; return Ok(result.consumed_out); } Err(err) => return Err(io::Error::new(io::ErrorKind::InvalidData, err)), From 4097bf37b1433c304c34f70cf3b439b51d19d758 Mon Sep 17 00:00:00 2001 From: Jonathan Behrens Date: Fri, 15 Apr 2022 10:03:50 -0400 Subject: [PATCH 5/6] Rename test oom_2 -> fuzzer_testcase4 --- tests/decode_images.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/decode_images.rs b/tests/decode_images.rs index dc0ab22..ee6ccd1 100644 --- a/tests/decode_images.rs +++ b/tests/decode_images.rs @@ -396,7 +396,7 @@ fn oom() { } #[test] -fn oom_2() { +fn fuzzer_testcase4() { 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, From ff33343372ee38d544111049bf8aecb3cf144116 Mon Sep 17 00:00:00 2001 From: Jonathan Behrens Date: Fri, 15 Apr 2022 11:28:56 -0400 Subject: [PATCH 6/6] Fix formatting --- src/decoder/mod.rs | 13 +++---------- src/decoder/stream.rs | 9 ++------- 2 files changed, 5 insertions(+), 17 deletions(-) diff --git a/src/decoder/mod.rs b/src/decoder/mod.rs index 04f00a7..8884c87 100644 --- a/src/decoder/mod.rs +++ b/src/decoder/mod.rs @@ -1032,11 +1032,7 @@ impl Decoder { let byte_order = self.reader.byte_order; let compression_method = self.image().compression_method; - let reader = Self::create_reader( - &mut self.reader, - compression_method, - length, - )?; + let reader = Self::create_reader(&mut self.reader, compression_method, length)?; // Read into output buffer. { @@ -1096,11 +1092,8 @@ impl Decoder { let compression_method = self.image().compression_method; let photometric_interpretation = self.image().photometric_interpretation; - let mut reader = Self::create_reader( - &mut self.reader, - compression_method, - compressed_length, - )?; + let mut reader = + Self::create_reader(&mut self.reader, compression_method, compressed_length)?; for row in 0..(tile_length - padding_down) { let buf = match &mut buffer { diff --git a/src/decoder/stream.rs b/src/decoder/stream.rs index 37a1a1d..75d45fd 100644 --- a/src/decoder/stream.rs +++ b/src/decoder/stream.rs @@ -139,10 +139,7 @@ pub struct LZWReader { impl LZWReader { /// Wraps a reader - pub fn new( - reader: R, - compressed_length: usize, - ) -> LZWReader { + pub fn new(reader: R, compressed_length: usize) -> LZWReader { Self { reader: BufReader::with_capacity( (32 * 1024).min(compressed_length), @@ -156,9 +153,7 @@ impl LZWReader { impl Read for LZWReader { fn read(&mut self, buf: &mut [u8]) -> io::Result { loop { - let result = self - .decoder - .decode_bytes(self.reader.fill_buf()?, buf); + let result = self.decoder.decode_bytes(self.reader.fill_buf()?, buf); self.reader.consume(result.consumed_in); match result.status {