diff --git a/src/ImageSharp/Common/Extensions/BufferedReadStreamExtensions.cs b/src/ImageSharp/Common/Extensions/BufferedReadStreamExtensions.cs new file mode 100644 index 0000000000..e90aba7138 --- /dev/null +++ b/src/ImageSharp/Common/Extensions/BufferedReadStreamExtensions.cs @@ -0,0 +1,72 @@ +// Copyright (c) Six Labors. +// Licensed under the Six Labors Split License. + +using SixLabors.ImageSharp.IO; + +namespace SixLabors.ImageSharp; + +/// +/// Extension methods for the type. +/// +internal static class BufferedReadStreamExtensions +{ + /// + /// Determines whether the complete read range is contained in the stream. + /// + /// The stream containing the data. + /// The absolute start of the range. + /// The number of bytes in the range. + /// Whether the range is contained in the stream. + public static bool IsReadRangeValid(this BufferedReadStream stream, long offset, ulong length) + { + // Compare the offset first so subtraction cannot underflow, and avoid + // adding an untrusted length to the offset where it could wrap around. + ulong streamLength = (ulong)stream.Length; + return (ulong)offset <= streamLength && length <= streamLength - (ulong)offset; + } + + /// + /// Gets a buffer length when the complete read fits in both the stream and an integer-sized buffer. + /// + /// The stream containing the data. + /// The declared length in bytes. + /// The validated length, or zero when the range is invalid. + /// Whether the complete read is valid. + public static bool TryGetReadLength(this BufferedReadStream stream, ulong length, out int bufferLength) + { + if (length > int.MaxValue || !stream.IsReadRangeValid(stream.Position, length)) + { + bufferLength = 0; + return false; + } + + bufferLength = (int)length; + return true; + } + + /// + /// Reads data from the stream into a slice of the provided buffer. + /// + /// The stream. + /// The buffer. + /// The offset within the buffer where bytes are read into. + /// The number of bytes, if available, to read. + /// The actual number of bytes read. + public static int Read(this BufferedReadStream stream, Span buffer, int offset, int count) + => stream.Read(buffer.Slice(offset, count)); + + /// + /// Advances the stream by the specified number of bytes. Nonpositive counts are ignored. + /// + /// The stream. + /// The number of bytes to skip. + public static void Skip(this BufferedReadStream stream, int count) + { + if (count > 0) + { + // BufferedReadStream is always seekable; its position setter preserves + // buffered data when the destination is inside the current buffer. + stream.Position += count; + } + } +} diff --git a/src/ImageSharp/Common/Extensions/StreamExtensions.cs b/src/ImageSharp/Common/Extensions/StreamExtensions.cs index 7ed3348240..13761d4d2a 100644 --- a/src/ImageSharp/Common/Extensions/StreamExtensions.cs +++ b/src/ImageSharp/Common/Extensions/StreamExtensions.cs @@ -1,8 +1,6 @@ // Copyright (c) Six Labors. // Licensed under the Six Labors Split License. -using System.Buffers; - namespace SixLabors.ImageSharp; /// @@ -19,53 +17,4 @@ internal static class StreamExtensions /// The number of bytes to write to the stream. public static void Write(this Stream stream, Span buffer, int offset, int count) => stream.Write(buffer.Slice(offset, count)); - - /// - /// Reads data from a stream into the provided buffer. - /// - /// The stream. - /// The buffer. - /// The offset within the buffer where the bytes are read into. - /// The number of bytes, if available, to read. - /// The actual number of bytes read. - public static int Read(this Stream stream, Span buffer, int offset, int count) - => stream.Read(buffer.Slice(offset, count)); - - /// - /// Skips the number of bytes in the given stream. - /// - /// The stream. - /// A byte offset relative to the origin parameter. - public static void Skip(this Stream stream, int count) - { - if (count < 1) - { - return; - } - - if (stream.CanSeek) - { - stream.Seek(count, SeekOrigin.Current); - return; - } - - byte[] buffer = ArrayPool.Shared.Rent(count); - try - { - while (count > 0) - { - int bytesRead = stream.Read(buffer, 0, count); - if (bytesRead == 0) - { - break; - } - - count -= bytesRead; - } - } - finally - { - ArrayPool.Shared.Return(buffer); - } - } } diff --git a/src/ImageSharp/Formats/Bmp/BmpDecoderCore.cs b/src/ImageSharp/Formats/Bmp/BmpDecoderCore.cs index b5ca4c26e9..95f02503e9 100644 --- a/src/ImageSharp/Formats/Bmp/BmpDecoderCore.cs +++ b/src/ImageSharp/Formats/Bmp/BmpDecoderCore.cs @@ -1429,7 +1429,7 @@ internal sealed class BmpDecoderCore : ImageDecoderCore // > 108 bytes infoHeaderType = BmpInfoHeaderType.WinVersion5; this.infoHeader = BmpInfoHeader.ParseV5(buffer); - if (this.infoHeader.ProfileData != 0 && this.infoHeader.ProfileSize != 0) + if (!this.Options.SkipMetadata && this.infoHeader.ProfileData != 0 && this.infoHeader.ProfileSize != 0) { long streamPosition = stream.Position; this.ExecuteAncillarySegmentAction(() => this.ReadIccProfile(stream, this.metadata, infoHeaderStart)); @@ -1477,8 +1477,7 @@ internal sealed class BmpDecoderCore : ImageDecoderCore long profileStart = infoHeaderStart + this.infoHeader.ProfileData; if (this.infoHeader.ProfileData < 0 || this.infoHeader.ProfileSize <= 0 || - profileStart > stream.Length || - this.infoHeader.ProfileSize > stream.Length - profileStart) + !stream.IsReadRangeValid(profileStart, (uint)this.infoHeader.ProfileSize)) { BmpThrowHelper.ThrowInvalidImageContentException("Not enough data to read BMP ICC profile."); } diff --git a/src/ImageSharp/Formats/Exr/Compression/ExrBaseDecompressor.cs b/src/ImageSharp/Formats/Exr/Compression/ExrBaseDecompressor.cs index 52a8164c7a..afd32ed663 100644 --- a/src/ImageSharp/Formats/Exr/Compression/ExrBaseDecompressor.cs +++ b/src/ImageSharp/Formats/Exr/Compression/ExrBaseDecompressor.cs @@ -59,7 +59,7 @@ internal abstract class ExrBaseDecompressor : ExrBaseCompression int totalRead = 0; while (totalRead < uncompressedBytes) { - int bytesRead = dataStream.Read(uncompressed, totalRead, (int)uncompressedBytes - totalRead); + int bytesRead = dataStream.Read(uncompressed.Slice(totalRead, (int)uncompressedBytes - totalRead)); if (bytesRead <= 0) { break; diff --git a/src/ImageSharp/Formats/Gif/Sections/GifXmpApplicationExtension.cs b/src/ImageSharp/Formats/Gif/Sections/GifXmpApplicationExtension.cs index 85c4959a98..aa3df4a4fd 100644 --- a/src/ImageSharp/Formats/Gif/Sections/GifXmpApplicationExtension.cs +++ b/src/ImageSharp/Formats/Gif/Sections/GifXmpApplicationExtension.cs @@ -28,7 +28,7 @@ internal readonly struct GifXmpApplicationExtension : IGifExtension /// The stream to read from. /// The memory allocator. /// The XMP metadata - public static GifXmpApplicationExtension Read(Stream stream, MemoryAllocator allocator) + public static GifXmpApplicationExtension Read(BufferedReadStream stream, MemoryAllocator allocator) { byte[] xmpBytes = ReadXmpData(stream, allocator, out bool terminated); if (!terminated) @@ -75,7 +75,7 @@ internal readonly struct GifXmpApplicationExtension : IGifExtension return this.ContentLength; } - private static byte[] ReadXmpData(Stream stream, MemoryAllocator allocator, out bool terminated) + private static byte[] ReadXmpData(BufferedReadStream stream, MemoryAllocator allocator, out bool terminated) { using ChunkedMemoryStream bytes = new(allocator); diff --git a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs index ae26914e8e..05cdd9cc77 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs @@ -274,10 +274,9 @@ internal sealed class JpegDecoderCore : ImageDecoderCore, IRawJpegData // Get the marker length. int markerContentByteSize = ReadUint16(stream, markerBuffer) - 2; - // Check whether the stream actually has enough bytes to read - // markerContentByteSize is always positive so we cast - // to uint to avoid sign extension - if (stream.RemainingBytes < (uint)markerContentByteSize) + // Validate the entire segment before parsing it. Casting directly + // to ulong also rejects lengths smaller than the two-byte length field. + if (!stream.IsReadRangeValid(stream.Position, (ulong)markerContentByteSize)) { JpegThrowHelper.ThrowNotEnoughBytesForMarker(fileMarker.Marker); } @@ -351,10 +350,9 @@ internal sealed class JpegDecoderCore : ImageDecoderCore, IRawJpegData // Get the marker length. int markerContentByteSize = ReadUint16(stream, markerBuffer) - 2; - // Check whether stream actually has enough bytes to read - // markerContentByteSize is always positive so we cast - // to uint to avoid sign extension. - if (stream.RemainingBytes < (uint)markerContentByteSize) + // Validate the entire segment before parsing it. Casting directly + // to ulong also rejects lengths smaller than the two-byte length field. + if (!stream.IsReadRangeValid(stream.Position, (ulong)markerContentByteSize)) { if (metadataOnly && this.Metadata != null && this.Frame != null) { @@ -841,7 +839,7 @@ internal sealed class JpegDecoderCore : ImageDecoderCore, IRawJpegData // TODO: thumbnail if (remaining > 0) { - if (stream.Position + remaining >= stream.Length) + if (!stream.IsReadRangeValid(stream.Position, (ulong)remaining + 1)) { this.ThrowOrIgnoreNonStrictSegmentError("Bad App0 Marker length."); stream.Skip(remaining); @@ -877,7 +875,7 @@ internal sealed class JpegDecoderCore : ImageDecoderCore, IRawJpegData return; } - if (stream.Position + remaining >= stream.Length) + if (!stream.IsReadRangeValid(stream.Position, (ulong)remaining + 1)) { this.ThrowOrIgnoreNonStrictSegmentError("Bad App1 Marker length."); stream.Skip(remaining); diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index d4b3828453..7693c78d45 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -884,7 +884,7 @@ internal sealed class PngDecoderCore : ImageDecoderCore while (currentRowBytesRead < bytesPerFrameScanline) { - int bytesRead = compressedStream.Read(scanSpan, currentRowBytesRead, bytesPerFrameScanline - currentRowBytesRead); + int bytesRead = compressedStream.Read(scanSpan.Slice(currentRowBytesRead, bytesPerFrameScanline - currentRowBytesRead)); if (bytesRead <= 0) { goto EXIT; @@ -1016,7 +1016,7 @@ internal sealed class PngDecoderCore : ImageDecoderCore cancellationToken.ThrowIfCancellationRequested(); while (currentRowBytesRead < bytesPerInterlaceScanline) { - int bytesRead = compressedStream.Read(this.scanline.GetSpan(), currentRowBytesRead, bytesPerInterlaceScanline - currentRowBytesRead); + int bytesRead = compressedStream.Read(this.scanline.GetSpan().Slice(currentRowBytesRead, bytesPerInterlaceScanline - currentRowBytesRead)); if (bytesRead <= 0) { goto EXIT; @@ -1984,7 +1984,7 @@ internal sealed class PngDecoderCore : ImageDecoderCore return false; } - int bytesRead = inflateStream.CompressedStream.Read(destUncompressedData, 0, destUncompressedData.Length); + int bytesRead = inflateStream.CompressedStream.Read(destUncompressedData); while (bytesRead != 0) { if (memoryStreamOutput.Length > maxLength) @@ -1994,7 +1994,7 @@ internal sealed class PngDecoderCore : ImageDecoderCore } memoryStreamOutput.Write(destUncompressedData[..bytesRead]); - bytesRead = inflateStream.CompressedStream.Read(destUncompressedData, 0, destUncompressedData.Length); + bytesRead = inflateStream.CompressedStream.Read(destUncompressedData); } uncompressedBytesArray = memoryStreamOutput.ToArray(); diff --git a/src/ImageSharp/Formats/Tiff/Compression/Decompressors/DeflateTiffCompression.cs b/src/ImageSharp/Formats/Tiff/Compression/Decompressors/DeflateTiffCompression.cs index d3b65c537c..a09a39b52b 100644 --- a/src/ImageSharp/Formats/Tiff/Compression/Decompressors/DeflateTiffCompression.cs +++ b/src/ImageSharp/Formats/Tiff/Compression/Decompressors/DeflateTiffCompression.cs @@ -69,7 +69,7 @@ internal sealed class DeflateTiffCompression : TiffBaseDecompressor int totalRead = 0; while (totalRead < buffer.Length) { - int bytesRead = dataStream.Read(buffer, totalRead, buffer.Length - totalRead); + int bytesRead = dataStream.Read(buffer[totalRead..]); if (bytesRead <= 0) { break; diff --git a/src/ImageSharp/Formats/Webp/BitReader/BitReaderBase.cs b/src/ImageSharp/Formats/Webp/BitReader/BitReaderBase.cs index 2b843cc8f6..4276199ee0 100644 --- a/src/ImageSharp/Formats/Webp/BitReader/BitReaderBase.cs +++ b/src/ImageSharp/Formats/Webp/BitReader/BitReaderBase.cs @@ -34,7 +34,7 @@ internal abstract class BitReaderBase : IDisposable { IMemoryOwner data = memoryAllocator.Allocate(bytesToRead, AllocationOptions.Clean); Span dataSpan = data.Memory.Span; - input.Read(dataSpan[..bytesToRead], 0, bytesToRead); + input.Read(dataSpan[..bytesToRead]); return data; } diff --git a/src/ImageSharp/Formats/Webp/Chunks/WebpFrameData.cs b/src/ImageSharp/Formats/Webp/Chunks/WebpFrameData.cs index 7d22f7f2b3..e7b135dc96 100644 --- a/src/ImageSharp/Formats/Webp/Chunks/WebpFrameData.cs +++ b/src/ImageSharp/Formats/Webp/Chunks/WebpFrameData.cs @@ -1,6 +1,8 @@ // Copyright (c) Six Labors. // Licensed under the Six Labors Split License. +using SixLabors.ImageSharp.IO; + namespace SixLabors.ImageSharp.Formats.Webp.Chunks; internal readonly struct WebpFrameData @@ -120,7 +122,7 @@ internal readonly struct WebpFrameData /// /// The stream to read from. /// Animation frame data. - public static WebpFrameData Parse(Stream stream) + public static WebpFrameData Parse(BufferedReadStream stream) { Span buffer = stackalloc byte[4]; diff --git a/src/ImageSharp/Formats/Webp/WebpAnimationDecoder.cs b/src/ImageSharp/Formats/Webp/WebpAnimationDecoder.cs index 8a8ad823dc..9ea86d0c9f 100644 --- a/src/ImageSharp/Formats/Webp/WebpAnimationDecoder.cs +++ b/src/ImageSharp/Formats/Webp/WebpAnimationDecoder.cs @@ -381,10 +381,7 @@ internal class WebpAnimationDecoder : IDisposable switch (chunkType) { case WebpChunkType.Iccp: - - // While ICC profiles are optional, an invalid ICC profile cannot be ignored because it must - // precede the frame data, and we cannot safely skip it without successfully reading its size. - WebpChunkParsingUtils.ReadIccProfile(stream, imageMetadata, ignoreMetadata); + WebpChunkParsingUtils.ReadIccProfile(stream, imageMetadata, ignoreMetadata, this.executeAncillarySegmentAction); break; case WebpChunkType.Exif: this.executeAncillarySegmentAction(() => WebpChunkParsingUtils.ReadExifProfile(stream, imageMetadata, ignoreMetadata)); diff --git a/src/ImageSharp/Formats/Webp/WebpChunkParsingUtils.cs b/src/ImageSharp/Formats/Webp/WebpChunkParsingUtils.cs index 1e334057e7..958f4df80b 100644 --- a/src/ImageSharp/Formats/Webp/WebpChunkParsingUtils.cs +++ b/src/ImageSharp/Formats/Webp/WebpChunkParsingUtils.cs @@ -262,7 +262,7 @@ internal static class WebpChunkParsingUtils /// /// Thrown if the input stream is not valid. /// - public static uint ReadUInt24LittleEndian(Stream stream, Span buffer) + public static uint ReadUInt24LittleEndian(BufferedReadStream stream, Span buffer) { if (stream.Read(buffer, 0, 3) == 3) { @@ -306,12 +306,33 @@ internal static class WebpChunkParsingUtils /// If true, the chunk size is required to be read, otherwise it can be skipped. /// The chunk size in bytes. /// Thrown if the input stream is not valid. - public static uint ReadChunkSize(Stream stream, Span buffer, bool required = true) + public static uint ReadChunkSize(BufferedReadStream stream, Span buffer, bool required = true) + { + ulong chunkSize = ReadPaddedChunkSize(stream, buffer, required); + + // Structural chunk sizes must remain representable by their uint-sized consumers. + // Metadata readers retain the wider extent so their recovery can skip it safely. + if (chunkSize > uint.MaxValue) + { + WebpThrowHelper.ThrowInvalidImageContentException("WebP chunk size exceeds the supported maximum."); + } + + return (uint)chunkSize; + } + + /// + /// Reads a chunk's complete padded extent without wrapping a uint-sized payload length. + /// + /// The input stream. + /// The four-byte size buffer. + /// Whether an incomplete size field is an error. + /// The padded extent, or remaining bytes when an optional size field is incomplete. + private static ulong ReadPaddedChunkSize(BufferedReadStream stream, Span buffer, bool required) { if (stream.Read(buffer) is 4) { uint chunkSize = BinaryPrimitives.ReadUInt32LittleEndian(buffer); - return chunkSize % 2 is 0 ? chunkSize : chunkSize + 1; + return (ulong)chunkSize + (chunkSize & 1); } if (required) @@ -320,7 +341,7 @@ internal static class WebpChunkParsingUtils } // Return the size of the remaining data in the stream. - return (uint)(stream.Length - stream.Position); + return (ulong)stream.RemainingBytes; } /// @@ -349,34 +370,36 @@ internal static class WebpChunkParsingUtils /// The stream to decode from. /// The image metadata. /// If true, metadata will be ignored. + /// Executes profile parsing under the decoder's integrity policy. public static void ReadIccProfile( BufferedReadStream stream, ImageMetadata metadata, - bool ignoreMetadata) + bool ignoreMetadata, + Action executeAncillarySegmentAction) { - Span buffer = stackalloc byte[4]; - int iccpChunkSize = ValidateMetadataChunkSize(stream, ReadChunkSize(stream, buffer), "ICCP"); - if (ignoreMetadata || metadata.IccProfile != null) + ulong chunkSize = ReadPaddedChunkSize(stream, stackalloc byte[4], true); + + // ICCP precedes image/frame data. Its framing must be readable even when + // metadata is skipped; otherwise there is no safe location to resume decoding. + if (!stream.IsReadRangeValid(stream.Position, chunkSize)) { - stream.Skip(iccpChunkSize); + WebpThrowHelper.ThrowInvalidImageContentException("Not enough data to read the ICCP chunk."); } - else + + executeAncillarySegmentAction(() => { - byte[] iccpData = new byte[iccpChunkSize]; - int bytesRead = stream.Read(iccpData, 0, iccpChunkSize); - if (bytesRead != iccpChunkSize) + byte[]? iccpData = ReadMetadataChunk(stream, chunkSize, ignoreMetadata || metadata.IccProfile != null); + if (iccpData is not null) { - WebpThrowHelper.ThrowInvalidImageContentException("Not enough data to read the iccp chunk"); - } + IccProfile profile = new(iccpData); + if (!profile.CheckIsValid()) + { + throw new InvalidIccProfileException("Invalid ICC profile."); + } - IccProfile profile = new(iccpData); - if (!profile.CheckIsValid()) - { - throw new InvalidIccProfileException("Invalid ICC profile."); + metadata.IccProfile = profile; } - - metadata.IccProfile = profile; - } + }); } /// @@ -390,21 +413,10 @@ internal static class WebpChunkParsingUtils ImageMetadata metadata, bool ignoreMetadata) { - Span buffer = stackalloc byte[4]; - int exifChunkSize = ValidateMetadataChunkSize(stream, ReadChunkSize(stream, buffer), "EXIF"); - if (ignoreMetadata || metadata.ExifProfile != null) - { - stream.Skip(exifChunkSize); - } - else + ulong chunkSize = ReadPaddedChunkSize(stream, stackalloc byte[4], !ignoreMetadata); + byte[]? exifData = ReadMetadataChunk(stream, chunkSize, ignoreMetadata || metadata.ExifProfile != null); + if (exifData is not null) { - byte[] exifData = new byte[exifChunkSize]; - int bytesRead = stream.Read(exifData, 0, exifChunkSize); - if (bytesRead != exifChunkSize) - { - WebpThrowHelper.ThrowInvalidImageContentException("Could not read enough data for the EXIF profile"); - } - ExifProfile exifProfile = new(exifData); // Set the resolution from the metadata. @@ -433,33 +445,46 @@ internal static class WebpChunkParsingUtils ImageMetadata metadata, bool ignoreMetadata) { - Span buffer = stackalloc byte[4]; - int xmpChunkSize = ValidateMetadataChunkSize(stream, ReadChunkSize(stream, buffer), "XMP"); - if (ignoreMetadata || metadata.XmpProfile != null) + ulong chunkSize = ReadPaddedChunkSize(stream, stackalloc byte[4], !ignoreMetadata); + byte[]? xmpData = ReadMetadataChunk(stream, chunkSize, ignoreMetadata || metadata.XmpProfile != null); + if (xmpData is not null) { - stream.Skip(xmpChunkSize); - } - else - { - byte[] xmpData = new byte[xmpChunkSize]; - int bytesRead = stream.Read(xmpData, 0, xmpChunkSize); - if (bytesRead != xmpChunkSize) - { - WebpThrowHelper.ThrowInvalidImageContentException("Could not read enough data for the XMP profile"); - } - metadata.XmpProfile = new XmpProfile(xmpData); } } - private static int ValidateMetadataChunkSize(BufferedReadStream stream, uint chunkSize, string chunkName) + /// + /// Reads a metadata payload, leaving the stream at the next chunk or EOF on a recoverable error. + /// Callers execute metadata parsing under the decoder's ancillary integrity policy. + /// + /// The input stream positioned at the chunk payload. + /// The declared extent including its padding byte. + /// Whether to skip the payload without parsing it. + /// The payload, or null when metadata is skipped. + private static byte[]? ReadMetadataChunk(BufferedReadStream stream, ulong paddedLength, bool ignoreMetadata) { - if (chunkSize > int.MaxValue || chunkSize > stream.Length - stream.Position) + long chunkEnd = stream.Position + (long)Math.Min(paddedLength, (ulong)stream.RemainingBytes); + if (ignoreMetadata) + { + stream.Position = chunkEnd; + return null; + } + + if (!stream.TryGetReadLength(paddedLength, out int bufferLength)) + { + // Ignoring an ancillary error must not make the next parser interpret + // this payload as another chunk header. A truncated chunk consumes EOF. + stream.Position = chunkEnd; + WebpThrowHelper.ThrowInvalidImageContentException("Not enough data to read the metadata chunk."); + } + + byte[] data = new byte[bufferLength]; + if (stream.Read(data) != bufferLength) { - WebpThrowHelper.ThrowInvalidImageContentException($"Not enough data to read the {chunkName} chunk"); + WebpThrowHelper.ThrowInvalidImageContentException("Not enough data to read the metadata chunk."); } - return (int)chunkSize; + return data; } private static double GetExifResolutionValue(ExifProfile exifProfile, ExifTag tag) diff --git a/src/ImageSharp/Formats/Webp/WebpDecoderCore.cs b/src/ImageSharp/Formats/Webp/WebpDecoderCore.cs index 55bdca69cb..cbff54e669 100644 --- a/src/ImageSharp/Formats/Webp/WebpDecoderCore.cs +++ b/src/ImageSharp/Formats/Webp/WebpDecoderCore.cs @@ -92,6 +92,13 @@ internal sealed class WebpDecoderCore : ImageDecoderCore, IDisposable return animationDecoder.Decode(stream, this.webImageInfo.Features, this.webImageInfo.Width, this.webImageInfo.Height, fileSize); } + // A VP8X header alone describes a canvas, not decodable image data. + // Ignoring a truncated optional chunk must not bypass this requirement. + if (this.webImageInfo.Vp8BitReader is null && this.webImageInfo.Vp8LBitReader is null) + { + WebpThrowHelper.ThrowInvalidImageContentException("Missing WebP image data."); + } + image = new Image(this.configuration, (int)this.webImageInfo.Width, (int)this.webImageInfo.Height, metadata); Buffer2D pixels = image.GetRootFramePixelBuffer(); if (this.webImageInfo.IsLossless) @@ -278,10 +285,7 @@ internal sealed class WebpDecoderCore : ImageDecoderCore, IDisposable switch (chunkType) { case WebpChunkType.Iccp: - - // While ICC profiles are optional, an invalid ICC profile cannot be ignored because it must - // precede the image data, and we cannot safely skip it without successfully reading its size. - WebpChunkParsingUtils.ReadIccProfile(stream, metadata, ignoreMetadata); + WebpChunkParsingUtils.ReadIccProfile(stream, metadata, ignoreMetadata, this.ExecuteAncillarySegmentAction); break; case WebpChunkType.Exif: @@ -330,17 +334,17 @@ internal sealed class WebpDecoderCore : ImageDecoderCore, IDisposable { // Read chunk header. WebpChunkType chunkType = WebpChunkParsingUtils.ReadChunkType(stream, buffer); - if (chunkType == WebpChunkType.Exif && metadata.ExifProfile == null) + if (chunkType == WebpChunkType.Exif) { this.ExecuteAncillarySegmentAction(() => WebpChunkParsingUtils.ReadExifProfile(stream, metadata, ignoreMetadata)); } - else if (chunkType == WebpChunkType.Xmp && metadata.XmpProfile == null) + else if (chunkType == WebpChunkType.Xmp) { this.ExecuteAncillarySegmentAction(() => WebpChunkParsingUtils.ReadXmpProfile(stream, metadata, ignoreMetadata)); } else { - // Skip duplicate XMP or EXIF chunk. + // Skip unknown chunks. uint chunkLength = WebpChunkParsingUtils.ReadChunkSize(stream, buffer, false); stream.Skip((int)chunkLength); } diff --git a/tests/ImageSharp.Tests/Common/BufferedReadStreamExtensionsTests.cs b/tests/ImageSharp.Tests/Common/BufferedReadStreamExtensionsTests.cs new file mode 100644 index 0000000000..6bef711931 --- /dev/null +++ b/tests/ImageSharp.Tests/Common/BufferedReadStreamExtensionsTests.cs @@ -0,0 +1,60 @@ +// Copyright (c) Six Labors. +// Licensed under the Six Labors Split License. + +using SixLabors.ImageSharp.IO; + +namespace SixLabors.ImageSharp.Tests.Common; + +public class BufferedReadStreamExtensionsTests +{ + [Theory] + [InlineData(0L, 8UL, true)] + [InlineData(8L, 0UL, true)] + [InlineData(7L, 2UL, false)] + [InlineData(9L, 0UL, false)] + [InlineData(-1L, 1UL, false)] + [InlineData(long.MaxValue, ulong.MaxValue, false)] + [InlineData(0L, ulong.MaxValue, false)] + public void IsReadRangeValid_ChecksCompleteExtent(long offset, ulong length, bool expected) + { + using MemoryStream input = new(new byte[8]); + using BufferedReadStream stream = new(Configuration.Default, input); + + Assert.Equal(expected, stream.IsReadRangeValid(offset, length)); + Assert.Equal(0, stream.Position); + } + + [Theory] + [InlineData(0UL, true, 0)] + [InlineData(6UL, true, 6)] + [InlineData(7UL, false, 0)] + [InlineData(1073741824UL, false, 0)] + [InlineData(4294967294UL, false, 0)] + [InlineData(4294967296UL, false, 0)] + [InlineData(ulong.MaxValue, false, 0)] + public void TryGetReadLength_ReturnsResultWithoutMovingStream(ulong length, bool expected, int expectedLength) + { + using MemoryStream input = new(new byte[8]); + using BufferedReadStream stream = new(Configuration.Default, input); + stream.Position = 2; + + Assert.Equal(expected, stream.TryGetReadLength(length, out int bufferLength)); + Assert.Equal(expectedLength, bufferLength); + Assert.Equal(2, stream.Position); + } + + [Theory] + [InlineData(0)] + [InlineData(-1)] + public void Skip_CountZeroOrLower_PositionNotChanged(int count) + { + using MemoryStream input = new(new byte[8]); + using BufferedReadStream stream = new(Configuration.Default, input); + stream.Position = 4; + + stream.Skip(count); + + Assert.Equal(4, stream.Position); + Assert.Equal(0, stream.ReadByte()); + } +} diff --git a/tests/ImageSharp.Tests/Common/StreamExtensionsTests.cs b/tests/ImageSharp.Tests/Common/StreamExtensionsTests.cs deleted file mode 100644 index 5ea7afaf80..0000000000 --- a/tests/ImageSharp.Tests/Common/StreamExtensionsTests.cs +++ /dev/null @@ -1,111 +0,0 @@ -// Copyright (c) Six Labors. -// Licensed under the Six Labors Split License. - -namespace SixLabors.ImageSharp.Tests.Common; - -public class StreamExtensionsTests -{ - [Theory] - [InlineData(0)] - [InlineData(-1)] - public void Skip_CountZeroOrLower_PositionNotChanged(int count) - { - using (MemoryStream memStream = new(5)) - { - memStream.Position = 4; - memStream.Skip(count); - - Assert.Equal(4, memStream.Position); - } - } - - [Fact] - public void Skip_SeekableStream_SeekIsCalled() - { - using (SeekableStream seekableStream = new(4)) - { - seekableStream.Skip(4); - - Assert.Equal(4, seekableStream.Offset); - Assert.Equal(SeekOrigin.Current, seekableStream.Loc); - } - } - - [Fact] - public void Skip_NonSeekableStream_BytesAreRead() - { - using (NonSeekableStream nonSeekableStream = new()) - { - nonSeekableStream.Skip(5); - - Assert.Equal(3, nonSeekableStream.Counts.Count); - - Assert.Equal(5, nonSeekableStream.Counts[0]); - Assert.Equal(3, nonSeekableStream.Counts[1]); - Assert.Equal(1, nonSeekableStream.Counts[2]); - } - } - - [Fact] - public void Skip_EofStream_NoExceptionIsThrown() - { - using (EofStream eofStream = new(7)) - { - eofStream.Skip(7); - - Assert.Equal(0, eofStream.Position); - } - } - - private class SeekableStream : MemoryStream - { - public long Offset; - public SeekOrigin Loc; - - public SeekableStream(int capacity) - : base(capacity) - { - } - - public override long Seek(long offset, SeekOrigin loc) - { - this.Offset = offset; - this.Loc = loc; - return base.Seek(offset, loc); - } - } - - private class NonSeekableStream : MemoryStream - { - public override bool CanSeek => false; - - public List Counts = new(); - - public NonSeekableStream() - : base(4) - { - } - - public override int Read(byte[] buffer, int offset, int count) - { - this.Counts.Add(count); - - return Math.Min(2, count); - } - } - - private class EofStream : MemoryStream - { - public override bool CanSeek => false; - - public EofStream(int capacity) - : base(capacity) - { - } - - public override int Read(byte[] buffer, int offset, int count) - { - return 0; - } - } -} diff --git a/tests/ImageSharp.Tests/Formats/Bmp/BmpDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Bmp/BmpDecoderTests.cs index ce15c91f0b..1702657892 100644 --- a/tests/ImageSharp.Tests/Formats/Bmp/BmpDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Bmp/BmpDecoderTests.cs @@ -34,8 +34,14 @@ public class BmpDecoderTests { RLE8, 2835, 2835, PixelResolutionUnit.PixelsPerMeter } }; - [Fact] - public void Decode_WithProfileLargerThanRemainingData_ThrowsInStrictMode() + [Theory] + [InlineData(SegmentIntegrityHandling.Strict, false)] + [InlineData(SegmentIntegrityHandling.IgnoreAncillary, false)] + [InlineData(SegmentIntegrityHandling.IgnoreImageData, false)] + [InlineData(SegmentIntegrityHandling.Strict, true)] + [InlineData(SegmentIntegrityHandling.IgnoreAncillary, true)] + [InlineData(SegmentIntegrityHandling.IgnoreImageData, true)] + public void Decode_WithProfileLargerThanRemainingData_RespectsOptions(SegmentIntegrityHandling integrityHandling, bool skipMetadata) { byte[] payload = Convert.FromHexString( "424D8E000000000000008A0000007C0000000100000001000000010018000000" + @@ -43,9 +49,24 @@ public class BmpDecoderTests "0000000000000000000000000000000000000000000000000000000000000000" + "000000000000000000000000000000000000000000000000000000000000C800" + "00000000004000000000000000"); - DecoderOptions options = new() { SegmentIntegrityHandling = SegmentIntegrityHandling.Strict }; + DecoderOptions options = new() { SegmentIntegrityHandling = integrityHandling, SkipMetadata = skipMetadata }; - Assert.Throws(() => Image.Load(options, payload)); + if (integrityHandling is SegmentIntegrityHandling.Strict && !skipMetadata) + { + Assert.Throws(() => Image.Load(options, payload)); + Assert.Throws(() => Image.Identify(options, payload)); + } + else + { + using Image image = Image.Load(options, payload); + Assert.Equal(new Size(1, 1), image.Size); + Assert.Equal(new Rgba32(0, 0, 0), image[0, 0]); + Assert.Null(image.Metadata.IccProfile); + + ImageInfo info = Image.Identify(options, payload); + Assert.Equal(image.Size, info.Size); + Assert.Null(info.Metadata.IccProfile); + } } [Theory] diff --git a/tests/ImageSharp.Tests/Formats/WebP/WebpMetaDataTests.cs b/tests/ImageSharp.Tests/Formats/WebP/WebpMetaDataTests.cs index 10678273bb..324070e4a5 100644 --- a/tests/ImageSharp.Tests/Formats/WebP/WebpMetaDataTests.cs +++ b/tests/ImageSharp.Tests/Formats/WebP/WebpMetaDataTests.cs @@ -1,9 +1,12 @@ // Copyright (c) Six Labors. // Licensed under the Six Labors Split License. +using System.Buffers.Binary; +using System.Text; using SixLabors.ImageSharp.Formats; using SixLabors.ImageSharp.Formats.Webp; using SixLabors.ImageSharp.Metadata.Profiles.Exif; +using SixLabors.ImageSharp.Metadata.Profiles.Icc; using SixLabors.ImageSharp.PixelFormats; using SixLabors.ImageSharp.Tests.TestUtilities; @@ -13,6 +16,35 @@ namespace SixLabors.ImageSharp.Tests.Formats.Webp; [Trait("Format", "Webp")] public class WebpMetaDataTests { + public static IEnumerable IccMetadataOptions() + { + foreach (SegmentIntegrityHandling integrity in new[] { SegmentIntegrityHandling.Strict, SegmentIntegrityHandling.IgnoreAncillary, SegmentIntegrityHandling.IgnoreImageData }) + { + foreach (bool skipMetadata in new[] { false, true }) + { + foreach (bool animated in new[] { false, true }) + { + yield return new object[] { integrity, skipMetadata, animated }; + } + } + } + } + + public static IEnumerable TruncatedMetadataOptions() + { + foreach (string chunkType in new[] { "EXIF", "XMP " }) + { + foreach (uint length in new[] { 0x40000000U, 0xFFFFFFFEU, uint.MaxValue }) + { + foreach (SegmentIntegrityHandling integrity in new[] { SegmentIntegrityHandling.Strict, SegmentIntegrityHandling.IgnoreAncillary, SegmentIntegrityHandling.IgnoreImageData }) + { + yield return new object[] { chunkType, length, integrity, false }; + yield return new object[] { chunkType, length, integrity, true }; + } + } + } + } + [Theory] [WithFile(TestImages.Webp.Lossy.BikeWithExif, PixelTypes.Rgba32, false)] [WithFile(TestImages.Webp.Lossy.BikeWithExif, PixelTypes.Rgba32, true)] @@ -229,23 +261,151 @@ public class WebpMetaDataTests }); } - [Fact] - public void Decode_WithOversizedIccChunk_ThrowsInvalidImageContentException() + [Theory] + [InlineData("ICCP", 0xFFFFFFFEU)] + [InlineData("EXIF", 0xFFFFFFFEU)] + [InlineData("XMP ", 0xFFFFFFFEU)] + [InlineData("ICCP", uint.MaxValue)] + [InlineData("EXIF", uint.MaxValue)] + [InlineData("XMP ", uint.MaxValue)] + public void Decode_WithOversizedMetadataChunk_ThrowsInvalidImageContentException(string chunkType, uint length) { byte[] payload = Convert.FromHexString( "524946462200000057454250565038580A0000002000000000000000000049434350FEFFFFFF01020304"); + Encoding.ASCII.GetBytes(chunkType, payload.AsSpan(30, 4)); + BinaryPrimitives.WriteUInt32LittleEndian(payload.AsSpan(34), length); + DecoderOptions options = new() { SegmentIntegrityHandling = SegmentIntegrityHandling.Strict }; - Assert.Throws(() => Image.Load(payload)); - Assert.Throws(() => Image.Identify(payload)); + Assert.Throws(() => Image.Load(options, payload)); + Assert.Throws(() => Image.Identify(options, payload)); } - [Fact] - public void Decode_WithIccChunkLargerThanRemainingData_ThrowsInvalidImageContentException() + [Theory] + [InlineData("ICCP")] + [InlineData("EXIF")] + [InlineData("XMP ")] + public void Decode_WithMetadataChunkLargerThanRemainingData_ThrowsInStrictMode(string chunkType) { byte[] payload = Convert.FromHexString( "524946460000000057454250565038580A00000020000000010000010000494343500000004000000000"); + Encoding.ASCII.GetBytes(chunkType, payload.AsSpan(30, 4)); + DecoderOptions options = new() { SegmentIntegrityHandling = SegmentIntegrityHandling.Strict }; + + Assert.Throws(() => Image.Load(options, payload)); + Assert.Throws(() => Image.Identify(options, payload)); + } + + [Theory] + [MemberData(nameof(TruncatedMetadataOptions))] + public void Decode_TruncatedTrailingMetadata_RespectsOptions(string chunkType, uint length, SegmentIntegrityHandling integrity, bool skipMetadata) + { + byte[] payload = CreateWebpWithMetadata(chunkType, length, false, false); + DecoderOptions options = new() { SegmentIntegrityHandling = integrity, SkipMetadata = skipMetadata }; + + if (integrity is SegmentIntegrityHandling.Strict && !skipMetadata) + { + Assert.Throws(() => Image.Load(options, payload)); + Assert.Throws(() => Image.Identify(options, payload)); + } + else + { + using Image image = Image.Load(options, payload); + Assert.Equal(new Size(2, 2), image.Size); + for (int y = 0; y < image.Height; y++) + { + for (int x = 0; x < image.Width; x++) + { + Assert.Equal(new Rgba32(17, 34, 51), image[x, y]); + } + } + + Assert.Null(image.Metadata.ExifProfile); + Assert.Null(image.Metadata.XmpProfile); + + ImageInfo info = Image.Identify(options, payload); + Assert.Equal(image.Size, info.Size); + Assert.Null(info.Metadata.ExifProfile); + Assert.Null(info.Metadata.XmpProfile); + } + } + + [Theory] + [MemberData(nameof(IccMetadataOptions))] + public void Decode_InvalidIccPayload_RespectsOptionsAndReadsFollowingImage(SegmentIntegrityHandling integrity, bool skipMetadata, bool animated) + { + byte[] payload = CreateWebpWithMetadata("ICCP", 4, true, animated); + DecoderOptions options = new() { SegmentIntegrityHandling = integrity, SkipMetadata = skipMetadata }; + + if (integrity is SegmentIntegrityHandling.Strict && !skipMetadata) + { + Assert.Throws(() => Image.Load(options, payload)); + Assert.Throws(() => Image.Identify(options, payload)); + } + else + { + using Image image = Image.Load(options, payload); + Assert.Equal(new Size(2, 2), image.Size); + Assert.Equal(animated ? 2 : 1, image.Frames.Count); + Assert.Equal(new Rgba32(17, 34, 51), image[0, 0]); + Assert.Null(image.Metadata.IccProfile); + + ImageInfo info = Image.Identify(options, payload); + Assert.Equal(image.Size, info.Size); + Assert.Null(info.Metadata.IccProfile); + } + } + + [Theory] + [MemberData(nameof(IccMetadataOptions))] + public void Decode_TruncatedIccFraming_RemainsFatal(SegmentIntegrityHandling integrity, bool skipMetadata, bool animated) + { + byte[] payload = CreateWebpWithMetadata("ICCP", 0x40000000, true, animated); + DecoderOptions options = new() { SegmentIntegrityHandling = integrity, SkipMetadata = skipMetadata }; + + Assert.Throws(() => Image.Load(options, payload)); + Assert.Throws(() => Image.Identify(options, payload)); + } + + /// + /// Places a metadata declaration around a complete lossless image to test recovery independently of pixel truncation. + /// + private static byte[] CreateWebpWithMetadata(string chunkType, uint length, bool beforeImage, bool animated) + { + byte[] header = Convert.FromHexString( + "524946460000000057454250565038580A00000020000000010000010000494343500000004000000000"); + header[20] = chunkType switch { "ICCP" => 0x20, "EXIF" => 0x08, _ => 0x04 }; + Encoding.ASCII.GetBytes(chunkType, header.AsSpan(30, 4)); + BinaryPrimitives.WriteUInt32LittleEndian(header.AsSpan(34), length); + + using Image source = new(2, 2, new Rgba32(17, 34, 51)); + if (animated) + { + header[20] |= 0x02; + using Image secondFrame = new(2, 2, new Rgba32(51, 34, 17)); + source.Frames.AddFrame(secondFrame.Frames.RootFrame); + } + + using MemoryStream encoded = new(); + source.Save(encoded, new WebpEncoder { FileFormat = WebpFileFormatType.Lossless }); + byte[] imageData = encoded.ToArray(); + using MemoryStream combined = new(); + combined.Write(header.AsSpan(0, 30)); + if (beforeImage) + { + combined.Write(header.AsSpan(30)); + } + + // Replace the encoder's extended header when present, keeping its complete + // image or animation chunks and the deliberately chosen metadata declaration. + int imageChunkOffset = imageData.AsSpan(12, 4).SequenceEqual("VP8X"u8) ? 30 : 12; + combined.Write(imageData.AsSpan(imageChunkOffset)); + if (!beforeImage) + { + combined.Write(header.AsSpan(30)); + } - Assert.Throws(() => Image.Load(payload)); - Assert.Throws(() => Image.Identify(payload)); + byte[] payload = combined.ToArray(); + BinaryPrimitives.WriteUInt32LittleEndian(payload.AsSpan(4), (uint)payload.Length - 8); + return payload; } } diff --git a/tests/ImageSharp.Tests/IO/ChunkedMemoryStreamTests.cs b/tests/ImageSharp.Tests/IO/ChunkedMemoryStreamTests.cs index 89256507ed..63c1f7fa31 100644 --- a/tests/ImageSharp.Tests/IO/ChunkedMemoryStreamTests.cs +++ b/tests/ImageSharp.Tests/IO/ChunkedMemoryStreamTests.cs @@ -197,7 +197,7 @@ public class ChunkedMemoryStreamTests readonlyStream.Position = 0; bytArrRet = new byte[(int)readonlyStream.Length]; - readonlyStream.Read(bytArrRet, 0, (int)readonlyStream.Length); + readonlyStream.Read(bytArrRet); for (int i = 0; i < bytArr.Length; i++) { Assert.Equal(bytArr[i], bytArrRet[i]); @@ -216,7 +216,7 @@ public class ChunkedMemoryStreamTests ms2.WriteTo(ms3); ms3.Position = 0; bytArrRet = new byte[(int)ms3.Length]; - ms3.Read(bytArrRet, 0, (int)ms3.Length); + ms3.Read(bytArrRet); for (int i = 0; i < bytArr.Length; i++) { Assert.Equal(bytArr[i], bytArrRet[i]);