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