From f36ec126959b2a7fe021aa51678c64e658e9fb2e Mon Sep 17 00:00:00 2001 From: antonfirsov Date: Sun, 15 Oct 2023 14:47:56 +0200 Subject: [PATCH 1/6] Disallow allocation attempts of unrepresentable sizes backport of #2545 --- .../UniformUnmanagedMemoryPoolMemoryAllocator.cs | 5 +++++ tests/ImageSharp.Tests/Image/ImageTests.cs | 4 ++++ .../Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs | 7 +++++++ 3 files changed, 16 insertions(+) diff --git a/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs b/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs index 0da4ff9f8c..7395ec9ac7 100644 --- a/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs +++ b/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs @@ -121,6 +121,11 @@ internal override MemoryGroup AllocateGroup( AllocationOptions options = AllocationOptions.None) { long totalLengthInBytes = totalLength * Unsafe.SizeOf(); + if (totalLengthInBytes < 0) + { + throw new InvalidMemoryOperationException("Attempted to allocate a MemoryGroup of a size that is not representable."); + } + if (totalLengthInBytes <= this.sharedArrayPoolThresholdInBytes) { var buffer = new SharedArrayPoolBuffer((int)totalLength); diff --git a/tests/ImageSharp.Tests/Image/ImageTests.cs b/tests/ImageSharp.Tests/Image/ImageTests.cs index 0a9e2817a5..1bfd307cbd 100644 --- a/tests/ImageSharp.Tests/Image/ImageTests.cs +++ b/tests/ImageSharp.Tests/Image/ImageTests.cs @@ -40,6 +40,10 @@ public void Width_Height() } } + [Fact] + public void Width_Height_SizeNotRepresentable_ThrowsInvalidImageOperationException() + => Assert.Throws(() => new Image(int.MaxValue, int.MaxValue)); + [Fact] public void Configuration_Width_Height() { diff --git a/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs b/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs index 0520c3c1fe..f49dcf05db 100644 --- a/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs +++ b/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs @@ -111,6 +111,13 @@ public void AllocateGroup_MultipleTimes_ExceedPoolLimit() } } + [Fact] + public void AllocateGroup_SizeInBytesOverLongMaxValue_ThrowsInvalidMemoryOperationException() + { + var allocator = new UniformUnmanagedMemoryPoolMemoryAllocator(null); + Assert.Throws(() => allocator.AllocateGroup(int.MaxValue * (long)int.MaxValue, int.MaxValue)); + } + [Fact] public unsafe void Allocate_MemoryIsPinnableMultipleTimes() { From 749b1c04d757be301c2ad8decff13e45774fee8e Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Mon, 16 Oct 2023 04:08:30 +0200 Subject: [PATCH 2/6] [release/2.1] Tiff decoding robustness improvements (#2550) (#2554) * Tiff decoding robustness improvements (#2550) * tiled Tiff is not supported in 2.1, delete TiffDecoder_CanDecode_TiledWithBadZlib * also delete the files * and the constant --- .../Compression/Zlib/ZlibInflateStream.cs | 45 +++++++++---------- .../Formats/Tiff/Ifd/DirectoryReader.cs | 16 +++++-- .../Formats/Tiff/TiffDecoderTests.cs | 12 +++++ tests/ImageSharp.Tests/TestImages.cs | 1 + .../Issues/JpegCompressedGray-0000539558.tiff | 3 ++ 5 files changed, 49 insertions(+), 28 deletions(-) create mode 100644 tests/Images/Input/Tiff/Issues/JpegCompressedGray-0000539558.tiff diff --git a/src/ImageSharp/Compression/Zlib/ZlibInflateStream.cs b/src/ImageSharp/Compression/Zlib/ZlibInflateStream.cs index f4b0543b84..bb97ff79eb 100644 --- a/src/ImageSharp/Compression/Zlib/ZlibInflateStream.cs +++ b/src/ImageSharp/Compression/Zlib/ZlibInflateStream.cs @@ -161,6 +161,11 @@ 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; } @@ -168,22 +173,13 @@ public override int Read(byte[] buffer, int offset, int count) } /// - 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) @@ -245,22 +241,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. @@ -269,7 +260,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; } diff --git a/src/ImageSharp/Formats/Tiff/Ifd/DirectoryReader.cs b/src/ImageSharp/Formats/Tiff/Ifd/DirectoryReader.cs index cce1d278cc..1d7592679c 100644 --- a/src/ImageSharp/Formats/Tiff/Ifd/DirectoryReader.cs +++ b/src/ImageSharp/Formats/Tiff/Ifd/DirectoryReader.cs @@ -43,7 +43,7 @@ public DirectoryReader(Stream stream, MemoryAllocator allocator) public IEnumerable 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; @@ -55,7 +55,12 @@ public IEnumerable Read() 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; @@ -74,7 +79,7 @@ private IEnumerable ReadIfds(bool isBigTiff) var readers = new List(); 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) @@ -88,6 +93,11 @@ private IEnumerable 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); diff --git a/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs index 2a8cbcbf78..a4243c94b6 100644 --- a/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs @@ -668,6 +668,18 @@ public void TiffDecoder_ThrowsException_WithTooManyDirectories(TestImage } }); + [Theory] + [WithFile(JpegCompressedGray0000539558, PixelTypes.Rgba32)] + public void TiffDecoder_ThrowsException_WithCircular_IFD_Offsets(TestImageProvider provider) + where TPixel : unmanaged, IPixel + => Assert.Throws( + () => + { + using (provider.GetImage(TiffDecoder)) + { + } + }); + [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 874a980881..bcae124659 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -916,6 +916,7 @@ public static class Tiff public const string Issues1716Rgb161616BitLittleEndian = "Tiff/Issues/Issue1716.tiff"; public const string Issues1891 = "Tiff/Issues/Issue1891.tiff"; public const string Issues2123 = "Tiff/Issues/Issue2123.tiff"; + public const string JpegCompressedGray0000539558 = "Tiff/Issues/JpegCompressedGray-0000539558.tiff"; public const string SmallRgbDeflate = "Tiff/rgb_small_deflate.tiff"; public const string SmallRgbLzw = "Tiff/rgb_small_lzw.tiff"; 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 From e74a55fbfdf199b3ffce299df0b98288227232f5 Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Mon, 16 Oct 2023 05:14:10 +0200 Subject: [PATCH 3/6] [release/2.1] PBM decoder robustness improvements and BufferedReadStream observability (#2555) * PBM decoder robustness improvements and BufferedReadStream observability Backport of #2551 & #2552 * Remove DoesNotReturn attribute --------- Co-authored-by: James Jackson-South --- .../Formats/ImageDecoderUtilities.cs | 10 +- src/ImageSharp/Formats/Pbm/BinaryDecoder.cs | 45 +++++--- .../Pbm/BufferedReadStreamExtensions.cs | 41 +++++-- src/ImageSharp/Formats/Pbm/PbmDecoderCore.cs | 23 +++- src/ImageSharp/Formats/Pbm/PlainDecoder.cs | 109 ++++++++++++++---- src/ImageSharp/IO/BufferedReadStream.cs | 18 ++- .../Formats/Pbm/PbmDecoderTests.cs | 22 ++++ .../Formats/Pbm/PbmMetadataTests.cs | 7 +- tests/ImageSharp.Tests/TestImages.cs | 1 + .../TestUtilities/EofHitCounter.cs | 39 +++++++ .../Input/Pbm/00000_00000_premature_eof.ppm | 3 + 11 files changed, 256 insertions(+), 62 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 71ecda8938..a2bbe34058 100644 --- a/src/ImageSharp/Formats/ImageDecoderUtilities.cs +++ b/src/ImageSharp/Formats/ImageDecoderUtilities.cs @@ -46,7 +46,8 @@ public static Image Decode( CancellationToken cancellationToken) where TPixel : unmanaged, IPixel { - using var bufferedReadStream = new BufferedReadStream(configuration, stream); + // 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); try { @@ -56,6 +57,13 @@ public static Image Decode( { throw largeImageExceptionFactory(ex, decoder.Dimensions); } + 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 33af30434c..25563d2d95 100644 --- a/src/ImageSharp/Formats/Pbm/BinaryDecoder.cs +++ b/src/ImageSharp/Formats/Pbm/BinaryDecoder.cs @@ -72,7 +72,11 @@ private static void ProcessGrayscale(Configuration configuration, Buffer for (int y = 0; y < height; y++) { - stream.Read(rowSpan); + if (stream.Read(rowSpan) < rowSpan.Length) + { + return; + } + Span pixelSpan = pixels.DangerousGetRowSpan(y); PixelOperations.Instance.FromL8Bytes( configuration, @@ -94,7 +98,11 @@ private static void ProcessWideGrayscale(Configuration configuration, Bu for (int y = 0; y < height; y++) { - stream.Read(rowSpan); + if (stream.Read(rowSpan) < rowSpan.Length) + { + return; + } + Span pixelSpan = pixels.DangerousGetRowSpan(y); PixelOperations.Instance.FromL16Bytes( configuration, @@ -116,7 +124,11 @@ private static void ProcessRgb(Configuration configuration, Buffer2D pixelSpan = pixels.DangerousGetRowSpan(y); PixelOperations.Instance.FromRgb24Bytes( configuration, @@ -138,7 +150,11 @@ private static void ProcessWideRgb(Configuration configuration, Buffer2D for (int y = 0; y < height; y++) { - stream.Read(rowSpan); + if (stream.Read(rowSpan) < rowSpan.Length) + { + return; + } + Span pixelSpan = pixels.DangerousGetRowSpan(y); PixelOperations.Instance.FromRgb48Bytes( configuration, @@ -153,7 +169,6 @@ private static void ProcessBlackAndWhite(Configuration configuration, Bu { int width = pixels.Width; int height = pixels.Height; - int startBit = 0; MemoryAllocator allocator = configuration.MemoryAllocator; using IMemoryOwner row = allocator.Allocate(width); Span rowSpan = row.GetSpan(); @@ -163,23 +178,17 @@ private static void ProcessBlackAndWhite(Configuration configuration, Bu for (int x = 0; x < width;) { int raw = stream.ReadByte(); - int bit = startBit; - startBit = 0; - for (; bit < 8; bit++) + if (raw < 0) + { + return; + } + + int stopBit = Math.Min(8, width - x); + for (int bit = 0; bit < stopBit; bit++) { bool bitValue = (raw & (0x80 >> bit)) != 0; rowSpan[x] = bitValue ? black : white; x++; - if (x == width) - { - startBit = (bit + 1) & 7; // Round off to below 8. - if (startBit != 0) - { - stream.Seek(-1, System.IO.SeekOrigin.Current); - } - - break; - } } } diff --git a/src/ImageSharp/Formats/Pbm/BufferedReadStreamExtensions.cs b/src/ImageSharp/Formats/Pbm/BufferedReadStreamExtensions.cs index a1d61882f3..94468f90aa 100644 --- a/src/ImageSharp/Formats/Pbm/BufferedReadStreamExtensions.cs +++ b/src/ImageSharp/Formats/Pbm/BufferedReadStreamExtensions.cs @@ -12,14 +12,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 SkipWhitespaceAndComments(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) @@ -28,8 +34,12 @@ public static void SkipWhitespaceAndComments(this BufferedReadStream stream) 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; @@ -39,18 +49,31 @@ public static void SkipWhitespaceAndComments(this BufferedReadStream stream) } 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 ReadDecimal(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; @@ -59,7 +82,7 @@ public static int ReadDecimal(this BufferedReadStream stream) 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 749fc0292b..ccd5041239 100644 --- a/src/ImageSharp/Formats/Pbm/PbmDecoderCore.cs +++ b/src/ImageSharp/Formats/Pbm/PbmDecoderCore.cs @@ -90,6 +90,7 @@ public IImageInfo Identify(BufferedReadStream stream, CancellationToken cancella /// 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]; @@ -139,14 +140,22 @@ private void ProcessHeader(BufferedReadStream stream) 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.SkipWhitespaceAndComments() || + !stream.ReadDecimal(out int width) || + !stream.SkipWhitespaceAndComments() || + !stream.ReadDecimal(out int height) || + !stream.SkipWhitespaceAndComments()) + { + ThrowPrematureEof(); + } + if (this.ColorType != PbmColorType.BlackAndWhite) { - this.maxPixelValue = stream.ReadDecimal(); + if (!stream.ReadDecimal(out this.maxPixelValue)) + { + ThrowPrematureEof(); + } + if (this.maxPixelValue > 255) { this.ComponentType = PbmComponentType.Short; @@ -169,6 +178,8 @@ private void ProcessHeader(BufferedReadStream stream) meta.Encoding = this.Encoding; meta.ColorType = this.ColorType; meta.ComponentType = this.ComponentType; + + 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 aeb527dd20..f5e0378cea 100644 --- a/src/ImageSharp/Formats/Pbm/PlainDecoder.cs +++ b/src/ImageSharp/Formats/Pbm/PlainDecoder.cs @@ -66,13 +66,18 @@ private static void ProcessGrayscale(Configuration configuration, Buffer 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.ReadDecimal(out int value); + rowSpan[x] = new L8((byte)value); + eofReached = !stream.SkipWhitespaceAndComments(); + if (eofReached) + { + break; + } } Span pixelSpan = pixels.DangerousGetRowSpan(y); @@ -80,6 +85,11 @@ private static void ProcessGrayscale(Configuration configuration, Buffer configuration, rowSpan, pixelSpan); + + if (eofReached) + { + return; + } } } @@ -92,13 +102,18 @@ private static void ProcessWideGrayscale(Configuration configuration, Bu 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.ReadDecimal(out int value); + rowSpan[x] = new L16((ushort)value); + eofReached = !stream.SkipWhitespaceAndComments(); + if (eofReached) + { + break; + } } Span pixelSpan = pixels.DangerousGetRowSpan(y); @@ -106,6 +121,11 @@ private static void ProcessWideGrayscale(Configuration configuration, Bu configuration, rowSpan, pixelSpan); + + if (eofReached) + { + return; + } } } @@ -118,17 +138,29 @@ private static void ProcessRgb(Configuration configuration, Buffer2D 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.ReadDecimal(out int red) || + !stream.SkipWhitespaceAndComments() || + !stream.ReadDecimal(out int green) || + !stream.SkipWhitespaceAndComments()) + { + // Reached EOF before reading a full RGB value + eofReached = true; + break; + } + + stream.ReadDecimal(out int blue); + + rowSpan[x] = new Rgb24((byte)red, (byte)green, (byte)blue); + eofReached = !stream.SkipWhitespaceAndComments(); + if (eofReached) + { + break; + } } Span pixelSpan = pixels.DangerousGetRowSpan(y); @@ -136,6 +168,11 @@ private static void ProcessRgb(Configuration configuration, Buffer2D(Configuration configuration, Buffer2D 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.ReadDecimal(out int red) || + !stream.SkipWhitespaceAndComments() || + !stream.ReadDecimal(out int green) || + !stream.SkipWhitespaceAndComments()) + { + // Reached EOF before reading a full RGB value + eofReached = true; + break; + } + + stream.ReadDecimal(out int blue); + + rowSpan[x] = new Rgb48((ushort)red, (ushort)green, (ushort)blue); + eofReached = !stream.SkipWhitespaceAndComments(); + if (eofReached) + { + break; + } } Span pixelSpan = pixels.DangerousGetRowSpan(y); @@ -166,6 +215,11 @@ private static void ProcessWideRgb(Configuration configuration, Buffer2D configuration, rowSpan, pixelSpan); + + if (eofReached) + { + return; + } } } @@ -178,13 +232,19 @@ private static void ProcessBlackAndWhite(Configuration configuration, Bu 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.ReadDecimal(out int value); + rowSpan[x] = value == 0 ? White : Black; + eofReached = !stream.SkipWhitespaceAndComments(); + if (eofReached) + { + break; + } } Span pixelSpan = pixels.DangerousGetRowSpan(y); @@ -192,6 +252,11 @@ private static void ProcessBlackAndWhite(Configuration configuration, Bu configuration, rowSpan, pixelSpan); + + if (eofReached) + { + return; + } } } } diff --git a/src/ImageSharp/IO/BufferedReadStream.cs b/src/ImageSharp/IO/BufferedReadStream.cs index 2823b8ed6f..28103359e6 100644 --- a/src/ImageSharp/IO/BufferedReadStream.cs +++ b/src/ImageSharp/IO/BufferedReadStream.cs @@ -65,6 +65,11 @@ public BufferedReadStream(Configuration configuration, Stream stream) this.readBufferIndex = this.BufferSize; } + /// + /// 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. /// @@ -138,6 +143,7 @@ public override int ReadByte() { if (this.readerPosition >= this.Length) { + this.EofHitCount++; return -1; } @@ -303,7 +309,7 @@ private int ReadToBufferViaCopyFast(Span buffer) this.readerPosition += n; this.readBufferIndex += n; - + this.CheckEof(n); return n; } @@ -361,6 +367,7 @@ private int ReadToBufferDirectSlow(Span buffer) this.Position += n; + this.CheckEof(n); return n; } @@ -427,5 +434,14 @@ private unsafe void CopyBytes(byte[] buffer, int offset, int count) 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 eb3bc8c9a5..8708320e09 100644 --- a/tests/ImageSharp.Tests/Formats/Pbm/PbmDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Pbm/PbmDecoderTests.cs @@ -2,8 +2,10 @@ // Licensed under the Apache License, Version 2.0. using System.IO; +using System.Text; using SixLabors.ImageSharp.Formats.Pbm; using SixLabors.ImageSharp.PixelFormats; +using SixLabors.ImageSharp.Tests.TestUtilities; using Xunit; using static SixLabors.ImageSharp.Tests.TestImages.Pbm; @@ -97,5 +99,25 @@ public void DecodeReferenceImage(TestImageProvider provider, str bool isGrayscale = extension is "pgm" or "pbm"; image.CompareToReferenceOutput(provider, grayscale: isGrayscale); } + + + [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 37052dbaf3..8b5381c7ea 100644 --- a/tests/ImageSharp.Tests/Formats/Pbm/PbmMetadataTests.cs +++ b/tests/ImageSharp.Tests/Formats/Pbm/PbmMetadataTests.cs @@ -86,13 +86,10 @@ public void Identify_DetectsCorrectComponentType(string imagePath, PbmComponentT } [Fact] - public void Identify_HandlesCraftedDenialOfServiceString() + public void Identify_EofInHeader_ThrowsInvalidImageContentException() { byte[] bytes = Convert.FromBase64String("UDEjWAAACQAAAAA="); - IImageInfo info = Image.Identify(bytes); - Assert.Equal(default, info.Size()); - IImageFormat format = Configuration.Default.ImageFormatsManager.FindFormatByFileExtension("pbm"); - Assert.Equal("PBM", format.Name); + Assert.Throws(() => Image.Identify(bytes)); } } } diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index bcae124659..d287ecb4cf 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -978,6 +978,7 @@ public static class Pbm 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..b627221f5b --- /dev/null +++ b/tests/ImageSharp.Tests/TestUtilities/EofHitCounter.cs @@ -0,0 +1,39 @@ +// Copyright (c) Six Labors. +// Licensed under the Apache License, Version 2.0. + +using System; +using System.IO; +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 From 3ea257472627dce9eec2813b373776390a5af00b Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Wed, 6 Mar 2024 21:14:55 +1000 Subject: [PATCH 4/6] Update PngDecoderCore.cs --- src/ImageSharp/Formats/Png/PngDecoderCore.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index 12770bc521..316f9385eb 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -464,7 +464,7 @@ private void ReadGammaChunk(PngMetadata pngMetadata, ReadOnlySpan data) private void InitializeImage(ImageMetadata metadata, out Image image) where TPixel : unmanaged, IPixel { - image = Image.CreateUninitialized( + image = new Image( this.Configuration, this.header.Width, this.header.Height, From 94bb7615a1503dc4db94122764b3835a6f8932e6 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Wed, 6 Mar 2024 21:24:03 +1000 Subject: [PATCH 5/6] Update ImageSharp.csproj --- src/ImageSharp/ImageSharp.csproj | 1 + 1 file changed, 1 insertion(+) diff --git a/src/ImageSharp/ImageSharp.csproj b/src/ImageSharp/ImageSharp.csproj index 39c85c4f22..e280e450a7 100644 --- a/src/ImageSharp/ImageSharp.csproj +++ b/src/ImageSharp/ImageSharp.csproj @@ -13,6 +13,7 @@ Image Resize Crop Gif Jpg Jpeg Bitmap Pbm Png Tga Tiff WebP NetCore A new, fully featured, fully managed, cross-platform, 2D graphics API for .NET Debug;Release;Debug-InnerLoop;Release-InnerLoop + false From 36b3533cc376c380be3653bfc5bffed1b1d6e693 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Wed, 6 Mar 2024 21:29:01 +1000 Subject: [PATCH 6/6] Use correct property to disable upstream warnings. --- src/Directory.Build.props | 2 +- src/ImageSharp/ImageSharp.csproj | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Directory.Build.props b/src/Directory.Build.props index faa29865f2..904d404f50 100644 --- a/src/Directory.Build.props +++ b/src/Directory.Build.props @@ -18,7 +18,7 @@ - true + diff --git a/src/ImageSharp/ImageSharp.csproj b/src/ImageSharp/ImageSharp.csproj index e280e450a7..39c85c4f22 100644 --- a/src/ImageSharp/ImageSharp.csproj +++ b/src/ImageSharp/ImageSharp.csproj @@ -13,7 +13,6 @@ Image Resize Crop Gif Jpg Jpeg Bitmap Pbm Png Tga Tiff WebP NetCore A new, fully featured, fully managed, cross-platform, 2D graphics API for .NET Debug;Release;Debug-InnerLoop;Release-InnerLoop - false