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 Image Decode( 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 Image Decode( { 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 @@ private static void ProcessGrayscale(Configuration configuration, Buffer 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 @@ private static void ProcessWideGrayscale(Configuration configuration, Bu 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 @@ private static void ProcessRgb(Configuration configuration, Buffer2D pixelSpan = pixels.DangerousGetRowSpan(y); PixelOperations.Instance.FromRgb24Bytes( configuration, @@ -137,7 +149,11 @@ private static void ProcessWideRgb(Configuration configuration, Buffer2D 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 @@ private static void ProcessBlackAndWhite(Configuration configuration, Bu 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 @@ 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; @@ -38,18 +48,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 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 @@ 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 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 @@ public ImageInfo Identify(BufferedReadStream stream, CancellationToken cancellat /// 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 @@ 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.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 @@ private void ProcessHeader(BufferedReadStream stream) this.componentType = PbmComponentType.Byte; } - stream.SkipWhitespaceAndComments(); + stream.TrySkipWhitespaceAndComments(); } else { @@ -174,6 +184,9 @@ private void ProcessHeader(BufferedReadStream stream) 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 @@ 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.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 @@ private static void ProcessGrayscale(Configuration configuration, Buffer configuration, rowSpan, pixelSpan); + + if (eofReached) + { + return; + } } } @@ -91,13 +101,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.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 @@ private static void ProcessWideGrayscale(Configuration configuration, Bu configuration, rowSpan, pixelSpan); + + if (eofReached) + { + return; + } } } @@ -117,17 +137,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.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 @@ 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.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 @@ private static void ProcessWideRgb(Configuration configuration, Buffer2D configuration, rowSpan, pixelSpan); + + if (eofReached) + { + return; + } } } @@ -177,13 +231,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.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 @@ 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 efa8f6f4be..1aa53d65e1 100644 --- a/src/ImageSharp/IO/BufferedReadStream.cs +++ b/src/ImageSharp/IO/BufferedReadStream.cs @@ -68,6 +68,11 @@ public BufferedReadStream(Configuration configuration, Stream stream, Cancellati 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 @@ public override int ReadByte() { if (this.readerPosition >= this.Length) { + this.EofHitCount++; return -1; } @@ -294,7 +300,7 @@ private int ReadToBufferViaCopyFast(Span buffer) this.readerPosition += n; this.readBufferIndex += n; - + this.CheckEof(n); return n; } @@ -352,6 +358,7 @@ private int ReadToBufferDirectSlow(Span buffer) this.Position += n; + this.CheckEof(n); return n; } @@ -418,4 +425,13 @@ 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 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 void PbmDecoder_Decode_Resize(TestImageProvider provider) 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 void Identify_DetectsCorrectComponentType(string imagePath, PbmComponentT } [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 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..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