Skip to content

Commit b8e8b86

Browse files
Tiff decoding robustness improvements (#2550)
* Faster Handling of circular offsets * Handle early EOF during ZLib inflate * more checks --------- Co-authored-by: antonfirsov <[email protected]>
1 parent c5eed0e commit b8e8b86

File tree

7 files changed

+74
-32
lines changed

7 files changed

+74
-32
lines changed

src/ImageSharp/Compression/Zlib/ZlibInflateStream.cs

Lines changed: 20 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -161,29 +161,25 @@ public override int Read(byte[] buffer, int offset, int count)
161161
bytesToRead = Math.Min(count - totalBytesRead, this.currentDataRemaining);
162162
this.currentDataRemaining -= bytesToRead;
163163
bytesRead = this.innerStream.Read(buffer, offset, bytesToRead);
164+
if (bytesRead == 0)
165+
{
166+
return totalBytesRead;
167+
}
168+
164169
totalBytesRead += bytesRead;
165170
}
166171

167172
return totalBytesRead;
168173
}
169174

170175
/// <inheritdoc/>
171-
public override long Seek(long offset, SeekOrigin origin)
172-
{
173-
throw new NotSupportedException();
174-
}
176+
public override long Seek(long offset, SeekOrigin origin) => throw new NotSupportedException();
175177

176178
/// <inheritdoc/>
177-
public override void SetLength(long value)
178-
{
179-
throw new NotSupportedException();
180-
}
179+
public override void SetLength(long value) => throw new NotSupportedException();
181180

182181
/// <inheritdoc/>
183-
public override void Write(byte[] buffer, int offset, int count)
184-
{
185-
throw new NotSupportedException();
186-
}
182+
public override void Write(byte[] buffer, int offset, int count) => throw new NotSupportedException();
187183

188184
/// <inheritdoc/>
189185
protected override void Dispose(bool disposing)
@@ -246,22 +242,17 @@ private bool InitializeInflateStream(bool isCriticalChunk)
246242
// CINFO is not defined in this specification for CM not equal to 8.
247243
throw new ImageFormatException($"Invalid window size for ZLIB header: cinfo={cinfo}");
248244
}
249-
else
250-
{
251-
return false;
252-
}
245+
246+
return false;
253247
}
254248
}
249+
else if (isCriticalChunk)
250+
{
251+
throw new ImageFormatException($"Bad method for ZLIB header: cmf={cmf}");
252+
}
255253
else
256254
{
257-
if (isCriticalChunk)
258-
{
259-
throw new ImageFormatException($"Bad method for ZLIB header: cmf={cmf}");
260-
}
261-
else
262-
{
263-
return false;
264-
}
255+
return false;
265256
}
266257

267258
// The preset dictionary.
@@ -270,7 +261,11 @@ private bool InitializeInflateStream(bool isCriticalChunk)
270261
{
271262
// We don't need this for inflate so simply skip by the next four bytes.
272263
// https://tools.ietf.org/html/rfc1950#page-6
273-
this.innerStream.Read(ChecksumBuffer, 0, 4);
264+
if (this.innerStream.Read(ChecksumBuffer, 0, 4) != 4)
265+
{
266+
return false;
267+
}
268+
274269
this.currentDataRemaining -= 4;
275270
}
276271

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

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ public DirectoryReader(Stream stream, MemoryAllocator allocator)
4040
public IList<ExifProfile> Read()
4141
{
4242
this.ByteOrder = ReadByteOrder(this.stream);
43-
var headerReader = new HeaderReader(this.stream, this.ByteOrder);
43+
HeaderReader headerReader = new(this.stream, this.ByteOrder);
4444
headerReader.ReadFileHeader();
4545

4646
this.nextIfdOffset = headerReader.FirstIfdOffset;
@@ -52,7 +52,12 @@ public IList<ExifProfile> Read()
5252
private static ByteOrder ReadByteOrder(Stream stream)
5353
{
5454
Span<byte> headerBytes = stackalloc byte[2];
55-
stream.Read(headerBytes);
55+
56+
if (stream.Read(headerBytes) != 2)
57+
{
58+
throw TiffThrowHelper.ThrowInvalidHeader();
59+
}
60+
5661
if (headerBytes[0] == TiffConstants.ByteOrderLittleEndian && headerBytes[1] == TiffConstants.ByteOrderLittleEndian)
5762
{
5863
return ByteOrder.LittleEndian;
@@ -68,10 +73,10 @@ private static ByteOrder ReadByteOrder(Stream stream)
6873

6974
private IList<ExifProfile> ReadIfds(bool isBigTiff)
7075
{
71-
var readers = new List<EntryReader>();
76+
List<EntryReader> readers = new();
7277
while (this.nextIfdOffset != 0 && this.nextIfdOffset < (ulong)this.stream.Length)
7378
{
74-
var reader = new EntryReader(this.stream, this.ByteOrder, this.allocator);
79+
EntryReader reader = new(this.stream, this.ByteOrder, this.allocator);
7580
reader.ReadTags(isBigTiff, this.nextIfdOffset);
7681

7782
if (reader.BigValues.Count > 0)
@@ -85,6 +90,11 @@ private IList<ExifProfile> ReadIfds(bool isBigTiff)
8590
}
8691
}
8792

93+
if (this.nextIfdOffset >= reader.NextIfdOffset && reader.NextIfdOffset != 0)
94+
{
95+
TiffThrowHelper.ThrowImageFormatException("TIFF image contains circular directory offsets");
96+
}
97+
8898
this.nextIfdOffset = reader.NextIfdOffset;
8999
readers.Add(reader);
90100

@@ -94,11 +104,11 @@ private IList<ExifProfile> ReadIfds(bool isBigTiff)
94104
}
95105
}
96106

97-
var list = new List<ExifProfile>(readers.Count);
107+
List<ExifProfile> list = new(readers.Count);
98108
foreach (EntryReader reader in readers)
99109
{
100110
reader.ReadBigValues();
101-
var profile = new ExifProfile(reader.Values, reader.InvalidTags);
111+
ExifProfile profile = new(reader.Values, reader.InvalidTags);
102112
list.Add(profile);
103113
}
104114

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

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
// Licensed under the Six Labors Split License.
33

44
// ReSharper disable InconsistentNaming
5-
using System.Runtime.InteropServices;
65
using System.Runtime.Intrinsics.X86;
76
using SixLabors.ImageSharp.Formats;
87
using SixLabors.ImageSharp.Formats.Tiff;
@@ -666,6 +665,33 @@ public void TiffDecoder_CanDecode_Fax4CompressedWithStrips<TPixel>(TestImageProv
666665
public void TiffDecoder_CanDecode_TiledWithNonEqualWidthAndHeight<TPixel>(TestImageProvider<TPixel> provider)
667666
where TPixel : unmanaged, IPixel<TPixel> => TestTiffDecoder(provider);
668667

668+
[Theory]
669+
[WithFile(JpegCompressedGray0000539558, PixelTypes.Rgba32)]
670+
public void TiffDecoder_ThrowsException_WithCircular_IFD_Offsets<TPixel>(TestImageProvider<TPixel> provider)
671+
where TPixel : unmanaged, IPixel<TPixel>
672+
=> Assert.Throws<ImageFormatException>(
673+
() =>
674+
{
675+
using (provider.GetImage(TiffDecoder.Instance))
676+
{
677+
}
678+
});
679+
680+
[Theory]
681+
[WithFile(Tiled0000023664, PixelTypes.Rgba32)]
682+
public void TiffDecoder_CanDecode_TiledWithBadZlib<TPixel>(TestImageProvider<TPixel> provider)
683+
where TPixel : unmanaged, IPixel<TPixel>
684+
{
685+
using Image<TPixel> image = provider.GetImage(TiffDecoder.Instance);
686+
687+
// ImageMagick cannot decode this image.
688+
image.DebugSave(provider);
689+
image.CompareToReferenceOutput(
690+
ImageComparer.Exact,
691+
provider,
692+
appendPixelTypeToFileName: false);
693+
}
694+
669695
[Theory]
670696
[WithFileCollection(nameof(MultiframeTestImages), PixelTypes.Rgba32)]
671697
public void DecodeMultiframe<TPixel>(TestImageProvider<TPixel> provider)

tests/ImageSharp.Tests/TestImages.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -980,6 +980,8 @@ public static class Tiff
980980
public const string Issues2149 = "Tiff/Issues/Group4CompressionWithStrips.tiff";
981981
public const string Issues2255 = "Tiff/Issues/Issue2255.png";
982982
public const string Issues2435 = "Tiff/Issues/Issue2435.tiff";
983+
public const string JpegCompressedGray0000539558 = "Tiff/Issues/JpegCompressedGray-0000539558.tiff";
984+
public const string Tiled0000023664 = "Tiff/Issues/tiled-0000023664.tiff";
983985

984986
public const string SmallRgbDeflate = "Tiff/rgb_small_deflate.tiff";
985987
public const string SmallRgbLzw = "Tiff/rgb_small_lzw.tiff";
Lines changed: 3 additions & 0 deletions
Loading
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
version https://git-lfs.github.com/spec/v1
2+
oid sha256:1f1ca630b5e46c7b5f21100fa8c0fbf27b79ca9da8cd95897667b64aedccf6e5
3+
size 539558
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
version https://git-lfs.github.com/spec/v1
2+
oid sha256:eb28a028b2467b9b42451d9cb30d8170fd91ff4c4046b69cc1ae7f123bf7ba6f
3+
size 23664

0 commit comments

Comments
 (0)