From b8e8b8613d2a326739f49f899465c6d7ad7e202c Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Sat, 14 Oct 2023 14:53:25 +1000 Subject: [PATCH 1/2] Tiff decoding robustness improvements (#2550) * Faster Handling of circular offsets * Handle early EOF during ZLib inflate * more checks --------- Co-authored-by: antonfirsov --- .../Compression/Zlib/ZlibInflateStream.cs | 45 +++++++++---------- .../Formats/Tiff/Ifd/DirectoryReader.cs | 22 ++++++--- .../Formats/Tiff/TiffDecoderTests.cs | 28 +++++++++++- tests/ImageSharp.Tests/TestImages.cs | 2 + ...code_TiledWithBadZlib_tiled-0000023664.png | 3 ++ .../Issues/JpegCompressedGray-0000539558.tiff | 3 ++ .../Input/Tiff/Issues/tiled-0000023664.tiff | 3 ++ 7 files changed, 74 insertions(+), 32 deletions(-) create mode 100644 tests/Images/External/ReferenceOutput/TiffDecoderTests/TiffDecoder_CanDecode_TiledWithBadZlib_tiled-0000023664.png create mode 100644 tests/Images/Input/Tiff/Issues/JpegCompressedGray-0000539558.tiff create mode 100644 tests/Images/Input/Tiff/Issues/tiled-0000023664.tiff diff --git a/src/ImageSharp/Compression/Zlib/ZlibInflateStream.cs b/src/ImageSharp/Compression/Zlib/ZlibInflateStream.cs index 06a7c3928c..f05f237576 100644 --- a/src/ImageSharp/Compression/Zlib/ZlibInflateStream.cs +++ b/src/ImageSharp/Compression/Zlib/ZlibInflateStream.cs @@ -161,6 +161,11 @@ internal sealed class ZlibInflateStream : Stream bytesToRead = Math.Min(count - totalBytesRead, this.currentDataRemaining); this.currentDataRemaining -= bytesToRead; bytesRead = this.innerStream.Read(buffer, offset, bytesToRead); + if (bytesRead == 0) + { + return totalBytesRead; + } + totalBytesRead += bytesRead; } @@ -168,22 +173,13 @@ internal sealed class ZlibInflateStream : Stream } /// - public override long Seek(long offset, SeekOrigin origin) - { - throw new NotSupportedException(); - } + public override long Seek(long offset, SeekOrigin origin) => throw new NotSupportedException(); /// - public override void SetLength(long value) - { - throw new NotSupportedException(); - } + public override void SetLength(long value) => throw new NotSupportedException(); /// - 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(); /// protected override void Dispose(bool disposing) @@ -246,22 +242,17 @@ internal sealed class ZlibInflateStream : Stream // 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 @@ internal sealed class ZlibInflateStream : Stream { // 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; } diff --git a/src/ImageSharp/Formats/Tiff/Ifd/DirectoryReader.cs b/src/ImageSharp/Formats/Tiff/Ifd/DirectoryReader.cs index 755e79e42e..086eef0585 100644 --- a/src/ImageSharp/Formats/Tiff/Ifd/DirectoryReader.cs +++ b/src/ImageSharp/Formats/Tiff/Ifd/DirectoryReader.cs @@ -40,7 +40,7 @@ internal class DirectoryReader public IList 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 @@ internal class DirectoryReader private static ByteOrder ReadByteOrder(Stream stream) { Span 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 @@ internal class DirectoryReader private IList ReadIfds(bool isBigTiff) { - var readers = new List(); + List 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 @@ internal class DirectoryReader } } + 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 @@ internal class DirectoryReader } } - var list = new List(readers.Count); + List 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); } diff --git a/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs index 2c8268d413..decc0069a5 100644 --- a/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs @@ -2,7 +2,6 @@ // Licensed under the Six Labors Split License. // ReSharper disable InconsistentNaming -using System.Runtime.InteropServices; using System.Runtime.Intrinsics.X86; using SixLabors.ImageSharp.Formats; using SixLabors.ImageSharp.Formats.Tiff; @@ -666,6 +665,33 @@ public class TiffDecoderTests : TiffDecoderBaseTester public void TiffDecoder_CanDecode_TiledWithNonEqualWidthAndHeight(TestImageProvider provider) where TPixel : unmanaged, IPixel => TestTiffDecoder(provider); + [Theory] + [WithFile(JpegCompressedGray0000539558, PixelTypes.Rgba32)] + public void TiffDecoder_ThrowsException_WithCircular_IFD_Offsets(TestImageProvider provider) + where TPixel : unmanaged, IPixel + => Assert.Throws( + () => + { + using (provider.GetImage(TiffDecoder.Instance)) + { + } + }); + + [Theory] + [WithFile(Tiled0000023664, PixelTypes.Rgba32)] + public void TiffDecoder_CanDecode_TiledWithBadZlib(TestImageProvider provider) + where TPixel : unmanaged, IPixel + { + using Image 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(TestImageProvider provider) diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index 470a670a4d..82f4d0462f 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -980,6 +980,8 @@ public static class TestImages public const string Issues2149 = "Tiff/Issues/Group4CompressionWithStrips.tiff"; public const string Issues2255 = "Tiff/Issues/Issue2255.png"; public const string Issues2435 = "Tiff/Issues/Issue2435.tiff"; + 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"; diff --git a/tests/Images/External/ReferenceOutput/TiffDecoderTests/TiffDecoder_CanDecode_TiledWithBadZlib_tiled-0000023664.png b/tests/Images/External/ReferenceOutput/TiffDecoderTests/TiffDecoder_CanDecode_TiledWithBadZlib_tiled-0000023664.png new file mode 100644 index 0000000000..d93f6ef3cd --- /dev/null +++ b/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:456f0699fbba95953fbdba0164168583cc7d2efe1f858a6570938e8797b398cd +size 15586 diff --git a/tests/Images/Input/Tiff/Issues/JpegCompressedGray-0000539558.tiff b/tests/Images/Input/Tiff/Issues/JpegCompressedGray-0000539558.tiff new file mode 100644 index 0000000000..934bf3c9a3 --- /dev/null +++ b/tests/Images/Input/Tiff/Issues/JpegCompressedGray-0000539558.tiff @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:1f1ca630b5e46c7b5f21100fa8c0fbf27b79ca9da8cd95897667b64aedccf6e5 +size 539558 diff --git a/tests/Images/Input/Tiff/Issues/tiled-0000023664.tiff b/tests/Images/Input/Tiff/Issues/tiled-0000023664.tiff new file mode 100644 index 0000000000..5106a027cc --- /dev/null +++ b/tests/Images/Input/Tiff/Issues/tiled-0000023664.tiff @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:eb28a028b2467b9b42451d9cb30d8170fd91ff4c4046b69cc1ae7f123bf7ba6f +size 23664 From d76fe6f6aed22b14d9744e321534b8ad23260cc6 Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Sat, 14 Oct 2023 07:59:38 +0200 Subject: [PATCH 2/2] PBM decoder robustness improvements and BufferedReadStream observability (#2551) * handle premature EOF in the PBM decoder * BufferedReadStreamExtensions: remove the 'Try' prefix * count EOF hits in BufferedReadStream * use EofHitCounter in pbm tests * Naming convention tweaks --------- Co-authored-by: James Jackson-South --- .../Formats/ImageDecoderUtilities.cs | 10 +- src/ImageSharp/Formats/Pbm/BinaryDecoder.cs | 29 ++++- .../Pbm/BufferedReadStreamExtensions.cs | 41 +++++-- src/ImageSharp/Formats/Pbm/PbmDecoderCore.cs | 27 +++-- src/ImageSharp/Formats/Pbm/PlainDecoder.cs | 109 ++++++++++++++---- src/ImageSharp/IO/BufferedReadStream.cs | 18 ++- .../Formats/Pbm/PbmDecoderTests.cs | 21 ++++ .../Formats/Pbm/PbmMetadataTests.cs | 7 +- tests/ImageSharp.Tests/TestImages.cs | 1 + .../TestUtilities/EofHitCounter.cs | 36 ++++++ .../Input/Pbm/00000_00000_premature_eof.ppm | 3 + 11 files changed, 253 insertions(+), 49 deletions(-) create mode 100644 tests/ImageSharp.Tests/TestUtilities/EofHitCounter.cs create mode 100644 tests/Images/Input/Pbm/00000_00000_premature_eof.ppm diff --git a/src/ImageSharp/Formats/ImageDecoderUtilities.cs b/src/ImageSharp/Formats/ImageDecoderUtilities.cs index e2c61c8eb3..a1abd7dc30 100644 --- a/src/ImageSharp/Formats/ImageDecoderUtilities.cs +++ b/src/ImageSharp/Formats/ImageDecoderUtilities.cs @@ -50,7 +50,8 @@ internal static class ImageDecoderUtilities CancellationToken cancellationToken) where TPixel : unmanaged, IPixel { - using BufferedReadStream bufferedReadStream = new(configuration, stream, cancellationToken); + // Test may pass a BufferedReadStream in order to monitor EOF hits, if so, use the existing instance. + BufferedReadStream bufferedReadStream = stream as BufferedReadStream ?? new BufferedReadStream(configuration, stream, cancellationToken); try { @@ -64,6 +65,13 @@ internal static class ImageDecoderUtilities { throw; } + finally + { + if (bufferedReadStream != stream) + { + bufferedReadStream.Dispose(); + } + } } private static InvalidImageContentException DefaultLargeImageExceptionFactory( diff --git a/src/ImageSharp/Formats/Pbm/BinaryDecoder.cs b/src/ImageSharp/Formats/Pbm/BinaryDecoder.cs index f629282340..96564df0ff 100644 --- a/src/ImageSharp/Formats/Pbm/BinaryDecoder.cs +++ b/src/ImageSharp/Formats/Pbm/BinaryDecoder.cs @@ -71,7 +71,11 @@ internal class BinaryDecoder for (int y = 0; y < height; y++) { - stream.Read(rowSpan); + if (stream.Read(rowSpan) == 0) + { + return; + } + Span pixelSpan = pixels.DangerousGetRowSpan(y); PixelOperations.Instance.FromL8Bytes( configuration, @@ -93,7 +97,11 @@ internal class BinaryDecoder for (int y = 0; y < height; y++) { - stream.Read(rowSpan); + if (stream.Read(rowSpan) == 0) + { + return; + } + Span pixelSpan = pixels.DangerousGetRowSpan(y); PixelOperations.Instance.FromL16Bytes( configuration, @@ -115,7 +123,11 @@ internal class BinaryDecoder for (int y = 0; y < height; y++) { - stream.Read(rowSpan); + if (stream.Read(rowSpan) == 0) + { + return; + } + Span pixelSpan = pixels.DangerousGetRowSpan(y); PixelOperations.Instance.FromRgb24Bytes( configuration, @@ -137,7 +149,11 @@ internal class BinaryDecoder for (int y = 0; y < height; y++) { - stream.Read(rowSpan); + if (stream.Read(rowSpan) == 0) + { + return; + } + Span pixelSpan = pixels.DangerousGetRowSpan(y); PixelOperations.Instance.FromRgb48Bytes( configuration, @@ -161,6 +177,11 @@ internal class BinaryDecoder for (int x = 0; x < width;) { int raw = stream.ReadByte(); + if (raw < 0) + { + return; + } + int stopBit = Math.Min(8, width - x); for (int bit = 0; bit < stopBit; bit++) { diff --git a/src/ImageSharp/Formats/Pbm/BufferedReadStreamExtensions.cs b/src/ImageSharp/Formats/Pbm/BufferedReadStreamExtensions.cs index 5d5537e398..8fd1d797d0 100644 --- a/src/ImageSharp/Formats/Pbm/BufferedReadStreamExtensions.cs +++ b/src/ImageSharp/Formats/Pbm/BufferedReadStreamExtensions.cs @@ -11,14 +11,20 @@ namespace SixLabors.ImageSharp.Formats.Pbm; internal static class BufferedReadStreamExtensions { /// - /// Skip over any whitespace or any comments. + /// Skip over any whitespace or any comments and signal if EOF has been reached. /// - public static void SkipWhitespaceAndComments(this BufferedReadStream stream) + /// The buffered read stream. + /// if EOF has been reached while reading the stream; see langword="true"/> otherwise. + public static bool TrySkipWhitespaceAndComments(this BufferedReadStream stream) { bool isWhitespace; do { int val = stream.ReadByte(); + if (val < 0) + { + return false; + } // Comments start with '#' and end at the next new-line. if (val == 0x23) @@ -27,8 +33,12 @@ internal static class BufferedReadStreamExtensions do { innerValue = stream.ReadByte(); + if (innerValue < 0) + { + return false; + } } - while (innerValue is not 0x0a and not -0x1); + while (innerValue is not 0x0a); // Continue searching for whitespace. val = innerValue; @@ -38,18 +48,31 @@ internal static class BufferedReadStreamExtensions } while (isWhitespace); stream.Seek(-1, SeekOrigin.Current); + return true; } /// - /// Read a decimal text value. + /// Read a decimal text value and signal if EOF has been reached. /// - /// The integer value of the decimal. - public static int ReadDecimal(this BufferedReadStream stream) + /// The buffered read stream. + /// The read value. + /// if EOF has been reached while reading the stream; otherwise. + /// + /// A 'false' return value doesn't mean that the parsing has been failed, since it's possible to reach EOF while reading the last decimal in the file. + /// It's up to the call site to handle such a situation. + /// + public static bool TryReadDecimal(this BufferedReadStream stream, out int value) { - int value = 0; + value = 0; while (true) { - int current = stream.ReadByte() - 0x30; + int current = stream.ReadByte(); + if (current < 0) + { + return false; + } + + current -= 0x30; if ((uint)current > 9) { break; @@ -58,6 +81,6 @@ internal static class BufferedReadStreamExtensions value = (value * 10) + current; } - return value; + return true; } } diff --git a/src/ImageSharp/Formats/Pbm/PbmDecoderCore.cs b/src/ImageSharp/Formats/Pbm/PbmDecoderCore.cs index e1bc5be6e8..84d0acb1b4 100644 --- a/src/ImageSharp/Formats/Pbm/PbmDecoderCore.cs +++ b/src/ImageSharp/Formats/Pbm/PbmDecoderCore.cs @@ -1,6 +1,7 @@ // Copyright (c) Six Labors. // Licensed under the Six Labors Split License. +using System.Diagnostics.CodeAnalysis; using SixLabors.ImageSharp.IO; using SixLabors.ImageSharp.Memory; using SixLabors.ImageSharp.Metadata; @@ -95,6 +96,7 @@ internal sealed class PbmDecoderCore : IImageDecoderInternals /// Processes the ppm header. /// /// The input stream. + /// An EOF marker has been read before the image has been decoded. private void ProcessHeader(BufferedReadStream stream) { Span buffer = stackalloc byte[2]; @@ -144,14 +146,22 @@ internal sealed class PbmDecoderCore : IImageDecoderInternals throw new InvalidImageContentException("Unknown of not implemented image type encountered."); } - stream.SkipWhitespaceAndComments(); - int width = stream.ReadDecimal(); - stream.SkipWhitespaceAndComments(); - int height = stream.ReadDecimal(); - stream.SkipWhitespaceAndComments(); + if (!stream.TrySkipWhitespaceAndComments() || + !stream.TryReadDecimal(out int width) || + !stream.TrySkipWhitespaceAndComments() || + !stream.TryReadDecimal(out int height) || + !stream.TrySkipWhitespaceAndComments()) + { + ThrowPrematureEof(); + } + if (this.colorType != PbmColorType.BlackAndWhite) { - this.maxPixelValue = stream.ReadDecimal(); + if (!stream.TryReadDecimal(out this.maxPixelValue)) + { + ThrowPrematureEof(); + } + if (this.maxPixelValue > 255) { this.componentType = PbmComponentType.Short; @@ -161,7 +171,7 @@ internal sealed class PbmDecoderCore : IImageDecoderInternals this.componentType = PbmComponentType.Byte; } - stream.SkipWhitespaceAndComments(); + stream.TrySkipWhitespaceAndComments(); } else { @@ -174,6 +184,9 @@ internal sealed class PbmDecoderCore : IImageDecoderInternals meta.Encoding = this.encoding; meta.ColorType = this.colorType; meta.ComponentType = this.componentType; + + [DoesNotReturn] + static void ThrowPrematureEof() => throw new InvalidImageContentException("Reached EOF while reading the header."); } private void ProcessPixels(BufferedReadStream stream, Buffer2D pixels) diff --git a/src/ImageSharp/Formats/Pbm/PlainDecoder.cs b/src/ImageSharp/Formats/Pbm/PlainDecoder.cs index f6d803684c..5a01c9a8af 100644 --- a/src/ImageSharp/Formats/Pbm/PlainDecoder.cs +++ b/src/ImageSharp/Formats/Pbm/PlainDecoder.cs @@ -65,13 +65,18 @@ internal class PlainDecoder using IMemoryOwner row = allocator.Allocate(width); Span rowSpan = row.GetSpan(); + bool eofReached = false; for (int y = 0; y < height; y++) { for (int x = 0; x < width; x++) { - byte value = (byte)stream.ReadDecimal(); - stream.SkipWhitespaceAndComments(); - rowSpan[x] = new L8(value); + stream.TryReadDecimal(out int value); + rowSpan[x] = new L8((byte)value); + eofReached = !stream.TrySkipWhitespaceAndComments(); + if (eofReached) + { + break; + } } Span pixelSpan = pixels.DangerousGetRowSpan(y); @@ -79,6 +84,11 @@ internal class PlainDecoder configuration, rowSpan, pixelSpan); + + if (eofReached) + { + return; + } } } @@ -91,13 +101,18 @@ internal class PlainDecoder using IMemoryOwner row = allocator.Allocate(width); Span rowSpan = row.GetSpan(); + bool eofReached = false; for (int y = 0; y < height; y++) { for (int x = 0; x < width; x++) { - ushort value = (ushort)stream.ReadDecimal(); - stream.SkipWhitespaceAndComments(); - rowSpan[x] = new L16(value); + stream.TryReadDecimal(out int value); + rowSpan[x] = new L16((ushort)value); + eofReached = !stream.TrySkipWhitespaceAndComments(); + if (eofReached) + { + break; + } } Span pixelSpan = pixels.DangerousGetRowSpan(y); @@ -105,6 +120,11 @@ internal class PlainDecoder configuration, rowSpan, pixelSpan); + + if (eofReached) + { + return; + } } } @@ -117,17 +137,29 @@ internal class PlainDecoder using IMemoryOwner row = allocator.Allocate(width); Span rowSpan = row.GetSpan(); + bool eofReached = false; for (int y = 0; y < height; y++) { for (int x = 0; x < width; x++) { - byte red = (byte)stream.ReadDecimal(); - stream.SkipWhitespaceAndComments(); - byte green = (byte)stream.ReadDecimal(); - stream.SkipWhitespaceAndComments(); - byte blue = (byte)stream.ReadDecimal(); - stream.SkipWhitespaceAndComments(); - rowSpan[x] = new Rgb24(red, green, blue); + if (!stream.TryReadDecimal(out int red) || + !stream.TrySkipWhitespaceAndComments() || + !stream.TryReadDecimal(out int green) || + !stream.TrySkipWhitespaceAndComments()) + { + // Reached EOF before reading a full RGB value + eofReached = true; + break; + } + + stream.TryReadDecimal(out int blue); + + rowSpan[x] = new Rgb24((byte)red, (byte)green, (byte)blue); + eofReached = !stream.TrySkipWhitespaceAndComments(); + if (eofReached) + { + break; + } } Span pixelSpan = pixels.DangerousGetRowSpan(y); @@ -135,6 +167,11 @@ internal class PlainDecoder configuration, rowSpan, pixelSpan); + + if (eofReached) + { + return; + } } } @@ -147,17 +184,29 @@ internal class PlainDecoder using IMemoryOwner row = allocator.Allocate(width); Span rowSpan = row.GetSpan(); + bool eofReached = false; for (int y = 0; y < height; y++) { for (int x = 0; x < width; x++) { - ushort red = (ushort)stream.ReadDecimal(); - stream.SkipWhitespaceAndComments(); - ushort green = (ushort)stream.ReadDecimal(); - stream.SkipWhitespaceAndComments(); - ushort blue = (ushort)stream.ReadDecimal(); - stream.SkipWhitespaceAndComments(); - rowSpan[x] = new Rgb48(red, green, blue); + if (!stream.TryReadDecimal(out int red) || + !stream.TrySkipWhitespaceAndComments() || + !stream.TryReadDecimal(out int green) || + !stream.TrySkipWhitespaceAndComments()) + { + // Reached EOF before reading a full RGB value + eofReached = true; + break; + } + + stream.TryReadDecimal(out int blue); + + rowSpan[x] = new Rgb48((ushort)red, (ushort)green, (ushort)blue); + eofReached = !stream.TrySkipWhitespaceAndComments(); + if (eofReached) + { + break; + } } Span pixelSpan = pixels.DangerousGetRowSpan(y); @@ -165,6 +214,11 @@ internal class PlainDecoder configuration, rowSpan, pixelSpan); + + if (eofReached) + { + return; + } } } @@ -177,13 +231,19 @@ internal class PlainDecoder using IMemoryOwner row = allocator.Allocate(width); Span rowSpan = row.GetSpan(); + bool eofReached = false; for (int y = 0; y < height; y++) { for (int x = 0; x < width; x++) { - int value = stream.ReadDecimal(); - stream.SkipWhitespaceAndComments(); + stream.TryReadDecimal(out int value); + rowSpan[x] = value == 0 ? White : Black; + eofReached = !stream.TrySkipWhitespaceAndComments(); + if (eofReached) + { + break; + } } Span pixelSpan = pixels.DangerousGetRowSpan(y); @@ -191,6 +251,11 @@ internal class PlainDecoder configuration, rowSpan, pixelSpan); + + if (eofReached) + { + return; + } } } } diff --git a/src/ImageSharp/IO/BufferedReadStream.cs b/src/ImageSharp/IO/BufferedReadStream.cs index efa8f6f4be..1aa53d65e1 100644 --- a/src/ImageSharp/IO/BufferedReadStream.cs +++ b/src/ImageSharp/IO/BufferedReadStream.cs @@ -68,6 +68,11 @@ internal sealed class BufferedReadStream : Stream this.readBufferIndex = int.MinValue; } + /// + /// Gets the number indicating the EOF hits occured while reading from this instance. + /// + public int EofHitCount { get; private set; } + /// /// Gets the size, in bytes, of the underlying buffer. /// @@ -142,6 +147,7 @@ internal sealed class BufferedReadStream : Stream { if (this.readerPosition >= this.Length) { + this.EofHitCount++; return -1; } @@ -294,7 +300,7 @@ internal sealed class BufferedReadStream : Stream this.readerPosition += n; this.readBufferIndex += n; - + this.CheckEof(n); return n; } @@ -352,6 +358,7 @@ internal sealed class BufferedReadStream : Stream this.Position += n; + this.CheckEof(n); return n; } @@ -418,4 +425,13 @@ internal sealed class BufferedReadStream : Stream Buffer.BlockCopy(this.readBuffer, this.readBufferIndex, buffer, offset, count); } } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private void CheckEof(int read) + { + if (read == 0) + { + this.EofHitCount++; + } + } } diff --git a/tests/ImageSharp.Tests/Formats/Pbm/PbmDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Pbm/PbmDecoderTests.cs index 1b57663f3a..11dd1cd58c 100644 --- a/tests/ImageSharp.Tests/Formats/Pbm/PbmDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Pbm/PbmDecoderTests.cs @@ -1,9 +1,11 @@ // Copyright (c) Six Labors. // Licensed under the Six Labors Split License. +using System.Text; using SixLabors.ImageSharp.Formats; using SixLabors.ImageSharp.Formats.Pbm; using SixLabors.ImageSharp.PixelFormats; +using SixLabors.ImageSharp.Tests.TestUtilities; using SixLabors.ImageSharp.Tests.TestUtilities.ImageComparison; using static SixLabors.ImageSharp.Tests.TestImages.Pbm; @@ -120,4 +122,23 @@ public class PbmDecoderTests testOutputDetails: details, appendPixelTypeToFileName: false); } + + [Fact] + public void PlainText_PrematureEof() + { + byte[] bytes = Encoding.ASCII.GetBytes($"P1\n100 100\n1 0 1 0 1 0"); + using EofHitCounter eofHitCounter = EofHitCounter.RunDecoder(bytes); + + Assert.True(eofHitCounter.EofHitCount <= 2); + Assert.Equal(new Size(100, 100), eofHitCounter.Image.Size); + } + + [Fact] + public void Binary_PrematureEof() + { + using EofHitCounter eofHitCounter = EofHitCounter.RunDecoder(RgbBinaryPrematureEof); + + Assert.True(eofHitCounter.EofHitCount <= 2); + Assert.Equal(new Size(29, 30), eofHitCounter.Image.Size); + } } diff --git a/tests/ImageSharp.Tests/Formats/Pbm/PbmMetadataTests.cs b/tests/ImageSharp.Tests/Formats/Pbm/PbmMetadataTests.cs index c40ec7318a..a69d9d9ba7 100644 --- a/tests/ImageSharp.Tests/Formats/Pbm/PbmMetadataTests.cs +++ b/tests/ImageSharp.Tests/Formats/Pbm/PbmMetadataTests.cs @@ -83,12 +83,9 @@ public class PbmMetadataTests } [Fact] - public void Identify_HandlesCraftedDenialOfServiceString() + public void Identify_EofInHeader_ThrowsInvalidImageContentException() { byte[] bytes = Convert.FromBase64String("UDEjWAAACQAAAAA="); - ImageInfo info = Image.Identify(bytes); - Assert.Equal(default, info.Size); - Configuration.Default.ImageFormatsManager.TryFindFormatByFileExtension("pbm", out IImageFormat format); - Assert.Equal(format!, info.Metadata.DecodedImageFormat); + Assert.Throws(() => Image.Identify(bytes)); } } diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index 82f4d0462f..c5565bbd85 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -1043,6 +1043,7 @@ public static class TestImages public const string GrayscalePlainNormalized = "Pbm/grayscale_plain_normalized.pgm"; public const string GrayscalePlainMagick = "Pbm/grayscale_plain_magick.pgm"; public const string RgbBinary = "Pbm/00000_00000.ppm"; + public const string RgbBinaryPrematureEof = "Pbm/00000_00000_premature_eof.ppm"; public const string RgbPlain = "Pbm/rgb_plain.ppm"; public const string RgbPlainNormalized = "Pbm/rgb_plain_normalized.ppm"; public const string RgbPlainMagick = "Pbm/rgb_plain_magick.ppm"; diff --git a/tests/ImageSharp.Tests/TestUtilities/EofHitCounter.cs b/tests/ImageSharp.Tests/TestUtilities/EofHitCounter.cs new file mode 100644 index 0000000000..d949ce0f2d --- /dev/null +++ b/tests/ImageSharp.Tests/TestUtilities/EofHitCounter.cs @@ -0,0 +1,36 @@ +// Copyright (c) Six Labors. +// Licensed under the Six Labors Split License. + +using SixLabors.ImageSharp.IO; + +namespace SixLabors.ImageSharp.Tests.TestUtilities; + +internal class EofHitCounter : IDisposable +{ + private readonly BufferedReadStream stream; + + public EofHitCounter(BufferedReadStream stream, Image image) + { + this.stream = stream; + this.Image = image; + } + + public int EofHitCount => this.stream.EofHitCount; + + public Image Image { get; private set; } + + public static EofHitCounter RunDecoder(string testImage) => RunDecoder(TestFile.Create(testImage).Bytes); + + public static EofHitCounter RunDecoder(byte[] imageData) + { + BufferedReadStream stream = new(Configuration.Default, new MemoryStream(imageData)); + Image image = Image.Load(stream); + return new EofHitCounter(stream, image); + } + + public void Dispose() + { + this.stream.Dispose(); + this.Image.Dispose(); + } +} diff --git a/tests/Images/Input/Pbm/00000_00000_premature_eof.ppm b/tests/Images/Input/Pbm/00000_00000_premature_eof.ppm new file mode 100644 index 0000000000..445cd0059a --- /dev/null +++ b/tests/Images/Input/Pbm/00000_00000_premature_eof.ppm @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:39cf6ca5b2f9d428c0c33e0fc7ab5e92c31e0c8a7d9e0276b9285f51a8ff547c +size 69