Tiff decoding robustness improvements (#2550)

Author: antonfirsov

src/ImageSharp/Compression/Zlib/ZlibInflateStream.cs

@@ -161,29 +161,25 @@ public override int Read(byte[] buffer, int offset, int count)
             bytesToRead = Math.Min(count - totalBytesRead, this.currentDataRemaining);
             this.currentDataRemaining -= bytesToRead;
             bytesRead = this.innerStream.Read(buffer, offset, bytesToRead);
+            if (bytesRead == 0)
+            {
+                return totalBytesRead;
+            }
+
             totalBytesRead += bytesRead;
         }
 
         return totalBytesRead;
     }
 
     /// <inheritdoc/>
-    public override long Seek(long offset, SeekOrigin origin)
-    {
-        throw new NotSupportedException();
-    }
+    public override long Seek(long offset, SeekOrigin origin) => throw new NotSupportedException();
 
     /// <inheritdoc/>
-    public override void SetLength(long value)
-    {
-        throw new NotSupportedException();
-    }
+    public override void SetLength(long value) => throw new NotSupportedException();
 
     /// <inheritdoc/>
-    public override void Write(byte[] buffer, int offset, int count)
-    {
-        throw new NotSupportedException();
-    }
+    public override void Write(byte[] buffer, int offset, int count) => throw new NotSupportedException();
 
     /// <inheritdoc/>
     protected override void Dispose(bool disposing)
@@ -246,22 +242,17 @@ private bool InitializeInflateStream(bool isCriticalChunk)
                     // CINFO is not defined in this specification for CM not equal to 8.
                     throw new ImageFormatException($"Invalid window size for ZLIB header: cinfo={cinfo}");
                 }
-                else
-                {
-                    return false;
-                }
+
+                return false;
             }
         }
+        else if (isCriticalChunk)
+        {
+            throw new ImageFormatException($"Bad method for ZLIB header: cmf={cmf}");
+        }
         else
         {
-            if (isCriticalChunk)
-            {
-                throw new ImageFormatException($"Bad method for ZLIB header: cmf={cmf}");
-            }
-            else
-            {
-                return false;
-            }
+            return false;
         }
 
         // The preset dictionary.
@@ -270,7 +261,11 @@ private bool InitializeInflateStream(bool isCriticalChunk)
         {
             // We don't need this for inflate so simply skip by the next four bytes.
             // https://tools.ietf.org/html/rfc1950#page-6
-            this.innerStream.Read(ChecksumBuffer, 0, 4);
+            if (this.innerStream.Read(ChecksumBuffer, 0, 4) != 4)
+            {
+                return false;
+            }
+
             this.currentDataRemaining -= 4;
         }
 

src/ImageSharp/Formats/Tiff/Ifd/DirectoryReader.cs

@@ -40,7 +40,7 @@ public DirectoryReader(Stream stream, MemoryAllocator allocator)
     public IList<ExifProfile> Read()
     {
         this.ByteOrder = ReadByteOrder(this.stream);
-        var headerReader = new HeaderReader(this.stream, this.ByteOrder);
+        HeaderReader headerReader = new(this.stream, this.ByteOrder);
         headerReader.ReadFileHeader();
 
         this.nextIfdOffset = headerReader.FirstIfdOffset;
@@ -52,7 +52,12 @@ public IList<ExifProfile> Read()
     private static ByteOrder ReadByteOrder(Stream stream)
     {
         Span<byte> headerBytes = stackalloc byte[2];
-        stream.Read(headerBytes);
+
+        if (stream.Read(headerBytes) != 2)
+        {
+            throw TiffThrowHelper.ThrowInvalidHeader();
+        }
+
         if (headerBytes[0] == TiffConstants.ByteOrderLittleEndian && headerBytes[1] == TiffConstants.ByteOrderLittleEndian)
         {
             return ByteOrder.LittleEndian;
@@ -68,10 +73,10 @@ private static ByteOrder ReadByteOrder(Stream stream)
 
     private IList<ExifProfile> ReadIfds(bool isBigTiff)
     {
-        var readers = new List<EntryReader>();
+        List<EntryReader> readers = new();
         while (this.nextIfdOffset != 0 && this.nextIfdOffset < (ulong)this.stream.Length)
         {
-            var reader = new EntryReader(this.stream, this.ByteOrder, this.allocator);
+            EntryReader reader = new(this.stream, this.ByteOrder, this.allocator);
             reader.ReadTags(isBigTiff, this.nextIfdOffset);
 
             if (reader.BigValues.Count > 0)
@@ -85,6 +90,11 @@ private IList<ExifProfile> ReadIfds(bool isBigTiff)
                 }
             }
 
+            if (this.nextIfdOffset >= reader.NextIfdOffset && reader.NextIfdOffset != 0)
+            {
+                TiffThrowHelper.ThrowImageFormatException("TIFF image contains circular directory offsets");
+            }
+
             this.nextIfdOffset = reader.NextIfdOffset;
             readers.Add(reader);
 
@@ -94,11 +104,11 @@ private IList<ExifProfile> ReadIfds(bool isBigTiff)
             }
         }
 
-        var list = new List<ExifProfile>(readers.Count);
+        List<ExifProfile> list = new(readers.Count);
         foreach (EntryReader reader in readers)
         {
             reader.ReadBigValues();
-            var profile = new ExifProfile(reader.Values, reader.InvalidTags);
+            ExifProfile profile = new(reader.Values, reader.InvalidTags);
             list.Add(profile);
         }
 

tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs

@@ -3,6 +3,7 @@
 
 // ReSharper disable InconsistentNaming
 using System.Runtime.InteropServices;
+using System.Runtime.Intrinsics.X86;
 using SixLabors.ImageSharp.Formats;
 using SixLabors.ImageSharp.Formats.Tiff;
 using SixLabors.ImageSharp.Metadata;
@@ -686,6 +687,33 @@ public void TiffDecoder_ThrowsException_WithTooManyDirectories<TPixel>(TestImage
     public void TiffDecoder_CanDecode_Fax4CompressedWithStrips<TPixel>(TestImageProvider<TPixel> provider)
         where TPixel : unmanaged, IPixel<TPixel> => TestTiffDecoder(provider);
 
+
+    [Theory]
+    [WithFile(JpegCompressedGray0000539558, PixelTypes.Rgba32)]
+    public void TiffDecoder_ThrowsException_WithCircular_IFD_Offsets<TPixel>(TestImageProvider<TPixel> provider)
+        where TPixel : unmanaged, IPixel<TPixel>
+        => Assert.Throws<ImageFormatException>(
+            () =>
+            {
+                using (provider.GetImage(TiffDecoder.Instance))
+                {
+                }
+            });
+
+    [Theory]
+    [WithFile(Tiled0000023664, PixelTypes.Rgba32)]
+    public void TiffDecoder_CanDecode_TiledWithBadZlib<TPixel>(TestImageProvider<TPixel> provider)
+        where TPixel : unmanaged, IPixel<TPixel>
+    {
+        using Image<TPixel> image = provider.GetImage(TiffDecoder.Instance);
+
+        // ImageMagick cannot decode this image.
+        image.DebugSave(provider);
+        image.CompareToReferenceOutput(
+            ImageComparer.Exact,
+            provider,
+            appendPixelTypeToFileName: false);
+    }
     [Theory]
     [WithFileCollection(nameof(MultiframeTestImages), PixelTypes.Rgba32)]
     public void DecodeMultiframe<TPixel>(TestImageProvider<TPixel> provider)

tests/ImageSharp.Tests/TestImages.cs

@@ -965,6 +965,8 @@ public static class Tiff
         public const string Issues2123 = "Tiff/Issues/Issue2123.tiff";
         public const string Issues2149 = "Tiff/Issues/Group4CompressionWithStrips.tiff";
         public const string Issues2255 = "Tiff/Issues/Issue2255.png";
+        public const string JpegCompressedGray0000539558 = "Tiff/Issues/JpegCompressedGray-0000539558.tiff";
+        public const string Tiled0000023664 = "Tiff/Issues/tiled-0000023664.tiff";
 
         public const string SmallRgbDeflate = "Tiff/rgb_small_deflate.tiff";
         public const string SmallRgbLzw = "Tiff/rgb_small_lzw.tiff";

tests/Images/External/ReferenceOutput/TiffDecoderTests/TiffDecoder_CanDecode_TiledWithBadZlib_tiled-0000023664.png

@@ -0,0 +1,3 @@
+version https://git-lfs.github.com/spec/v1
+oid sha256:1f1ca630b5e46c7b5f21100fa8c0fbf27b79ca9da8cd95897667b64aedccf6e5
+size 539558

tests/Images/Input/Tiff/Issues/JpegCompressedGray-0000539558.tiff

@@ -0,0 +1,3 @@
+version https://git-lfs.github.com/spec/v1
+oid sha256:eb28a028b2467b9b42451d9cb30d8170fd91ff4c4046b69cc1ae7f123bf7ba6f
+size 23664