diff --git a/src/ImageSharp/Formats/Webp/WebpChunkParsingUtils.cs b/src/ImageSharp/Formats/Webp/WebpChunkParsingUtils.cs index 958f4df80b..5a094304a5 100644 --- a/src/ImageSharp/Formats/Webp/WebpChunkParsingUtils.cs +++ b/src/ImageSharp/Formats/Webp/WebpChunkParsingUtils.cs @@ -150,6 +150,14 @@ public static WebpImageInfo ReadVp8LHeader(MemoryAllocator memoryAllocator, Buff // VP8 data size. uint imageDataSize = ReadChunkSize(stream, buffer); + // The header is a one byte signature followed by 32 bits holding the width, height, alpha flag and version. + // The bit reader zero-fills data missing from a truncated stream, so check that the header itself is present + // rather than reporting the zeros as an invalid signature. + if (stream.RemainingBytes < 5) + { + WebpThrowHelper.ThrowInvalidImageContentException("Not enough data to read the VP8L header"); + } + Vp8LBitReader bitReader = new(stream, imageDataSize, memoryAllocator); // One byte signature, should be 0x2f. @@ -209,14 +217,10 @@ public static WebpImageInfo ReadVp8XHeader(BufferedReadStream stream, Span uint fileSize = ReadChunkSize(stream, buffer); // The first byte contains information about the image features used. + // Its two most significant bits and its least significant bit are reserved. Writers set them to zero, + // but the container specification requires readers to ignore them, so they are not validated. byte imageFeatures = (byte)stream.ReadByte(); - // The first two bit of it are reserved and should be 0. - if (imageFeatures >> 6 != 0) - { - WebpThrowHelper.ThrowImageFormatException("first two bits of the VP8X header are expected to be zero"); - } - // If bit 3 is set, a ICC Profile Chunk should be present. features.IccProfile = (imageFeatures & (1 << 5)) != 0; @@ -259,8 +263,8 @@ public static WebpImageInfo ReadVp8XHeader(BufferedReadStream stream, Span /// The stream to read from. /// The buffer to store the read data into. /// A unsigned 24 bit integer. - /// - /// Thrown if the input stream is not valid. + /// + /// Thrown if the stream ends before the integer. /// public static uint ReadUInt24LittleEndian(BufferedReadStream stream, Span buffer) { @@ -270,7 +274,7 @@ public static uint ReadUInt24LittleEndian(BufferedReadStream stream, Span return BinaryPrimitives.ReadUInt32LittleEndian(buffer); } - throw new ImageFormatException("Invalid Webp data, could not read unsigned 24 bit integer."); + throw new InvalidImageContentException("Invalid Webp data, could not read unsigned 24 bit integer."); } /// @@ -303,12 +307,13 @@ public static unsafe void WriteUInt24LittleEndian(Stream stream, uint data) /// /// The stream to read the data from. /// Buffer to store the data read from the stream. - /// 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(BufferedReadStream stream, Span buffer, bool required = true) + /// Thrown if the stream ends inside the size field. + public static uint ReadChunkSize(BufferedReadStream stream, Span buffer) { - ulong chunkSize = ReadPaddedChunkSize(stream, buffer, required); + // Widen before padding so an odd uint.MaxValue payload cannot wrap to zero. + ulong chunkSize = ReadChunkPayloadSize(stream, buffer, true); + chunkSize += chunkSize & 1; // Structural chunk sizes must remain representable by their uint-sized consumers. // Metadata readers retain the wider extent so their recovery can skip it safely. @@ -321,23 +326,24 @@ public static uint ReadChunkSize(BufferedReadStream stream, Span buffer, b } /// - /// Reads a chunk's complete padded extent without wrapping a uint-sized payload length. + /// Reads the payload size stored in a chunk header. RIFF follows an odd-sized payload with a padding byte + /// that belongs to the chunk but not to its payload, so the returned size excludes it. /// /// 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) + /// The payload size, or remaining bytes when an optional size field is incomplete. + private static ulong ReadChunkPayloadSize(BufferedReadStream stream, Span buffer, bool required) { if (stream.Read(buffer) is 4) { - uint chunkSize = BinaryPrimitives.ReadUInt32LittleEndian(buffer); - return (ulong)chunkSize + (chunkSize & 1); + return BinaryPrimitives.ReadUInt32LittleEndian(buffer); } if (required) { - throw new ImageFormatException("Invalid Webp data, could not read chunk size."); + // Only a truncated stream leaves fewer than four bytes for the size field. + throw new InvalidImageContentException("Invalid Webp data, could not read chunk size."); } // Return the size of the remaining data in the stream. @@ -349,8 +355,8 @@ private static ulong ReadPaddedChunkSize(BufferedReadStream stream, Span b /// /// The stream to read the data from. /// Buffer to store the data read from the stream. - /// - /// Thrown if the input stream is not valid. + /// + /// Thrown if the stream ends inside the chunk type. /// public static WebpChunkType ReadChunkType(BufferedReadStream stream, Span buffer) { @@ -360,8 +366,8 @@ public static WebpChunkType ReadChunkType(BufferedReadStream stream, Span } // While we ignore unknown chunks we still need a to be a ble to read a chunk type - // known or otherwise from the stream. - throw new ImageFormatException("Invalid Webp data, could not read chunk type."); + // known or otherwise from the stream. Only a truncated stream leaves fewer than four bytes. + throw new InvalidImageContentException("Invalid Webp data, could not read chunk type."); } /// @@ -377,18 +383,18 @@ public static void ReadIccProfile( bool ignoreMetadata, Action executeAncillarySegmentAction) { - ulong chunkSize = ReadPaddedChunkSize(stream, stackalloc byte[4], true); + ulong payloadSize = ReadChunkPayloadSize(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)) + // ICCP precedes image/frame data. Its framing, including the padding byte of an odd-sized payload, + // must be readable even when metadata is skipped; otherwise there is no safe location to resume decoding. + if (!stream.IsReadRangeValid(stream.Position, payloadSize + (payloadSize & 1))) { WebpThrowHelper.ThrowInvalidImageContentException("Not enough data to read the ICCP chunk."); } executeAncillarySegmentAction(() => { - byte[]? iccpData = ReadMetadataChunk(stream, chunkSize, ignoreMetadata || metadata.IccProfile != null); + byte[]? iccpData = ReadMetadataChunk(stream, payloadSize, ignoreMetadata || metadata.IccProfile != null); if (iccpData is not null) { IccProfile profile = new(iccpData); @@ -413,8 +419,8 @@ public static void ReadExifProfile( ImageMetadata metadata, bool ignoreMetadata) { - ulong chunkSize = ReadPaddedChunkSize(stream, stackalloc byte[4], !ignoreMetadata); - byte[]? exifData = ReadMetadataChunk(stream, chunkSize, ignoreMetadata || metadata.ExifProfile != null); + ulong payloadSize = ReadChunkPayloadSize(stream, stackalloc byte[4], !ignoreMetadata); + byte[]? exifData = ReadMetadataChunk(stream, payloadSize, ignoreMetadata || metadata.ExifProfile != null); if (exifData is not null) { ExifProfile exifProfile = new(exifData); @@ -445,8 +451,8 @@ public static void ReadXmpProfile( ImageMetadata metadata, bool ignoreMetadata) { - ulong chunkSize = ReadPaddedChunkSize(stream, stackalloc byte[4], !ignoreMetadata); - byte[]? xmpData = ReadMetadataChunk(stream, chunkSize, ignoreMetadata || metadata.XmpProfile != null); + ulong payloadSize = ReadChunkPayloadSize(stream, stackalloc byte[4], !ignoreMetadata); + byte[]? xmpData = ReadMetadataChunk(stream, payloadSize, ignoreMetadata || metadata.XmpProfile != null); if (xmpData is not null) { metadata.XmpProfile = new XmpProfile(xmpData); @@ -458,11 +464,14 @@ public static void ReadXmpProfile( /// 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. + /// The declared payload length, excluding 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) + private static byte[]? ReadMetadataChunk(BufferedReadStream stream, ulong payloadLength, bool ignoreMetadata) { + // An odd-sized payload is followed by a padding byte that is part of the chunk but not of the payload. + // The next chunk starts after it, clamped to the end of a truncated stream. + ulong paddedLength = payloadLength + (payloadLength & 1); long chunkEnd = stream.Position + (long)Math.Min(paddedLength, (ulong)stream.RemainingBytes); if (ignoreMetadata) { @@ -470,7 +479,7 @@ public static void ReadXmpProfile( return null; } - if (!stream.TryGetReadLength(paddedLength, out int bufferLength)) + if (!stream.TryGetReadLength(payloadLength, 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. @@ -484,6 +493,8 @@ public static void ReadXmpProfile( WebpThrowHelper.ThrowInvalidImageContentException("Not enough data to read the metadata chunk."); } + // Skip the padding byte, so it is neither returned as payload nor read as the next chunk header. + stream.Position = chunkEnd; return data; } diff --git a/src/ImageSharp/Formats/Webp/WebpDecoderCore.cs b/src/ImageSharp/Formats/Webp/WebpDecoderCore.cs index c2a4e44653..98cc447a89 100644 --- a/src/ImageSharp/Formats/Webp/WebpDecoderCore.cs +++ b/src/ImageSharp/Formats/Webp/WebpDecoderCore.cs @@ -52,6 +52,11 @@ internal sealed class WebpDecoderCore : ImageDecoderCore, IDisposable /// private readonly BackgroundColorHandling backgroundColorHandling; + /// + /// The stream position where the RIFF payload ends. Chunk parsing stops here, so bytes after the payload are ignored. + /// + private long riffEnd; + /// /// Initializes a new instance of the class. /// @@ -75,7 +80,7 @@ protected override Image Decode(BufferedReadStream stream, Cance ImageMetadata metadata = new(); Span buffer = stackalloc byte[4]; - uint fileSize = ReadImageHeader(stream, buffer); + uint fileSize = this.ReadImageHeader(stream, buffer); using (this.webImageInfo = this.ReadVp8Info(stream, metadata)) { @@ -140,7 +145,7 @@ protected override Image Decode(BufferedReadStream stream, Cance /// protected override ImageInfo Identify(BufferedReadStream stream, CancellationToken cancellationToken) { - uint fileSize = ReadImageHeader(stream, stackalloc byte[4]); + uint fileSize = this.ReadImageHeader(stream, stackalloc byte[4]); ImageMetadata metadata = new(); using (this.webImageInfo = this.ReadVp8Info(stream, metadata, true)) @@ -170,13 +175,15 @@ protected override ImageInfo Identify(BufferedReadStream stream, CancellationTok } /// - /// Reads and skips over the image header. + /// Reads and skips over the image header, and records where the RIFF payload ends. /// /// The stream to decode from. /// Temporary buffer. /// The file size in bytes. - private static uint ReadImageHeader(BufferedReadStream stream, Span buffer) + private uint ReadImageHeader(BufferedReadStream stream, Span buffer) { + long riffStart = stream.Position; + // Skip FourCC header, we already know its a RIFF file at this point. stream.Skip(4); @@ -185,6 +192,11 @@ private static uint ReadImageHeader(BufferedReadStream stream, Span buffer // The file size in the header is the total size of the chunks that follow plus 4 bytes for the ‘WEBP’ FourCC. uint fileSize = WebpChunkParsingUtils.ReadChunkSize(stream, buffer); + // Bytes after the RIFF payload are not part of the image, so chunks are only read up to its end, + // as libwebp does. A declared size larger than the stream describes a truncated file, so reading + // then stops at the end of the stream instead. + this.riffEnd = Math.Min(riffStart + WebpConstants.ChunkHeaderSize + fileSize, stream.Length); + // Skip 'WEBP' from the header. stream.Skip(4); @@ -221,7 +233,9 @@ private WebpImageInfo ReadVp8Info(BufferedReadStream stream, ImageMetadata metad return info; case WebpChunkType.Vp8X: info = WebpChunkParsingUtils.ReadVp8XHeader(stream, buffer, features); - while (stream.Position < stream.Length) + + // Fewer bytes than a chunk header before the end of the RIFF payload cannot start another chunk. + while (this.riffEnd - stream.Position >= WebpConstants.ChunkHeaderSize) { chunkType = WebpChunkParsingUtils.ReadChunkType(stream, buffer); if (chunkType == WebpChunkType.Vp8) @@ -250,8 +264,9 @@ private WebpImageInfo ReadVp8Info(BufferedReadStream stream, ImageMetadata metad { // Ignore unknown chunks. // These must always fall after the image data so we are safe to always skip them. - uint chunkSize = WebpChunkParsingUtils.ReadChunkSize(stream, buffer, false); - stream.Skip((int)chunkSize); + // A chunk extending past the RIFF payload ends the parsing instead of wrapping to a negative skip. + uint chunkSize = WebpChunkParsingUtils.ReadChunkSize(stream, buffer); + stream.Position = Math.Min(stream.Position + chunkSize, this.riffEnd); } } @@ -334,8 +349,8 @@ private void ParseOptionalChunks(BufferedReadStream stream, ImageMetadata metada return; } - long streamLength = stream.Length; - while (stream.Position < streamLength) + // Fewer bytes than a chunk header before the end of the RIFF payload cannot start another chunk. + while (this.riffEnd - stream.Position >= WebpConstants.ChunkHeaderSize) { // Read chunk header. WebpChunkType chunkType = WebpChunkParsingUtils.ReadChunkType(stream, buffer); @@ -350,8 +365,9 @@ private void ParseOptionalChunks(BufferedReadStream stream, ImageMetadata metada else { // Skip unknown chunks. - uint chunkLength = WebpChunkParsingUtils.ReadChunkSize(stream, buffer, false); - stream.Skip((int)chunkLength); + // A chunk extending past the RIFF payload ends the parsing instead of wrapping to a negative skip. + uint chunkLength = WebpChunkParsingUtils.ReadChunkSize(stream, buffer); + stream.Position = Math.Min(stream.Position + chunkLength, this.riffEnd); } } } diff --git a/tests/ImageSharp.Tests/Formats/WebP/WebpDecoderTests.cs b/tests/ImageSharp.Tests/Formats/WebP/WebpDecoderTests.cs index a3e3b81cf0..6c6b4145df 100644 --- a/tests/ImageSharp.Tests/Formats/WebP/WebpDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/WebP/WebpDecoderTests.cs @@ -1,12 +1,17 @@ // Copyright (c) Six Labors. // Licensed under the Six Labors Split License. +using System.Buffers.Binary; using System.Runtime.Intrinsics.X86; +using System.Text; using SixLabors.ImageSharp.Formats; using SixLabors.ImageSharp.Formats.Webp; using SixLabors.ImageSharp.Metadata; using SixLabors.ImageSharp.Metadata.Profiles.Exif; +using SixLabors.ImageSharp.Metadata.Profiles.Icc; +using SixLabors.ImageSharp.Metadata.Profiles.Xmp; using SixLabors.ImageSharp.PixelFormats; +using SixLabors.ImageSharp.Tests.TestDataIcc; using SixLabors.ImageSharp.Tests.TestUtilities; using SixLabors.ImageSharp.Tests.TestUtilities.ImageComparison; using SixLabors.ImageSharp.Tests.TestUtilities.ReferenceCodecs; @@ -19,6 +24,42 @@ namespace SixLabors.ImageSharp.Tests.Formats.Webp; [ValidateDisposedMemoryAllocations] public class WebpDecoderTests { + /// + /// The kinds of encoded files the container tests mutate. + /// + public enum ContainerSample + { + /// + /// A simple lossy file with only a VP8 chunk. + /// + Lossy, + + /// + /// A simple lossless file with only a VP8L chunk. + /// + Lossless, + + /// + /// An extended lossy file with VP8X, ALPH and VP8 chunks. + /// + LossyWithAlpha, + + /// + /// An extended lossless file with VP8X, VP8L and EXIF chunks. + /// + LosslessWithExif, + + /// + /// An extended lossy file with VP8X, VP8, EXIF and XMP chunks. + /// + LossyWithExifAndXmp, + + /// + /// An extended lossless file with VP8X, ICCP and VP8L chunks. + /// + LosslessWithIcc + } + private static MagickReferenceDecoder ReferenceDecoder => MagickReferenceDecoder.WebP; private static string TestImageLossyHorizontalFilterPath => Path.Combine(TestEnvironment.InputImagesDirectoryFullPath, Lossy.AlphaCompressedHorizontalFilter); @@ -621,4 +662,225 @@ public void Decode_WhenColorProfileHandlingIsConvert_ApplyIccProfile(Tes image.CompareToReferenceOutput(provider); Assert.Null(image.Metadata.IccProfile); } + + [Theory] + [InlineData(20, 0x40)] + [InlineData(20, 0x80)] + [InlineData(20, 0x01)] + [InlineData(21, 0xFF)] + [InlineData(23, 0x80)] + public void Decode_Vp8XWithNonZeroReservedFields_MatchesOriginal(int offset, byte bits) + { + // Offset 20 is the VP8X feature flags byte, whose two top bits and lowest bit are reserved. + // Offsets 21 to 23 are the reserved bytes that follow it. + byte[] original = EncodeContainerSample(ContainerSample.LosslessWithExif); + byte[] patched = (byte[])original.Clone(); + patched[offset] |= bits; + + AssertSameDecodedImage(original, patched); + } + + [Theory] + [InlineData(ContainerSample.Lossy)] + [InlineData(ContainerSample.Lossless)] + [InlineData(ContainerSample.LossyWithAlpha)] + [InlineData(ContainerSample.LosslessWithExif)] + [InlineData(ContainerSample.LossyWithExifAndXmp)] + [InlineData(ContainerSample.LosslessWithIcc)] + public void Decode_WithBytesAfterRiffPayload_MatchesOriginal(ContainerSample sample) + { + byte[] original = EncodeContainerSample(sample); + Random random = new(42); + for (int count = 1; count <= 64; count++) + { + byte[] patched = new byte[original.Length + count]; + original.CopyTo(patched, 0); + random.NextBytes(patched.AsSpan(original.Length)); + + AssertSameDecodedImage(original, patched); + } + } + + [Fact] + public void Decode_WithImageChunkAfterRiffPayload_IgnoresIt() + { + // A complete chunk after the RIFF payload is not part of the file, even when it is valid image data. + byte[] original = EncodeContainerSample(ContainerSample.LossyWithAlpha); + using Image other = new(5, 3, Color.White.ToPixel()); + using MemoryStream otherStream = new(); + other.Save(otherStream, new WebpEncoder { FileFormat = WebpFileFormatType.Lossless }); + byte[] otherData = otherStream.ToArray(); + Assert.Equal(12, FindChunk(otherData, "VP8L")); + + byte[] patched = [.. original, .. otherData.AsSpan(12)]; + + AssertSameDecodedImage(original, patched); + } + + [Theory] + [InlineData(0x7FFFFFF0U)] + [InlineData(0xFFFFFFF0U)] + public void Decode_UnknownChunkLargerThanRiffPayload_MatchesOriginal(uint size) + { + // The declared size runs past the end of the RIFF payload, and the second one is negative as an int. + byte[] original = EncodeContainerSample(ContainerSample.LosslessWithExif); + byte[] patched = [.. original, .. "ABCD"u8, 0, 0, 0, 0, 1, 2, 3]; + BinaryPrimitives.WriteUInt32LittleEndian(patched.AsSpan(original.Length + 4), size); + BinaryPrimitives.WriteUInt32LittleEndian(patched.AsSpan(4), (uint)patched.Length - 8); + + AssertSameDecodedImage(original, patched); + } + + [Theory] + [InlineData(ContainerSample.Lossy)] + [InlineData(ContainerSample.Lossless)] + [InlineData(ContainerSample.LossyWithAlpha)] + [InlineData(ContainerSample.LosslessWithExif)] + [InlineData(ContainerSample.LossyWithExifAndXmp)] + [InlineData(ContainerSample.LosslessWithIcc)] + public void Decode_TruncatedContainer_DecodesOrThrowsInvalidImageContentException(ContainerSample sample) + { + byte[] original = EncodeContainerSample(sample); + + // Data missing from the compressed image data is reported by the bitstream decoders rather than the + // container parser, so Load is only checked for cuts outside that data. Identify only reads its header. + bool isLossless = TryFindChunk(original, "VP8L", out int imageChunk); + if (!isLossless) + { + imageChunk = FindChunk(original, "VP8 "); + } + + int bitstreamStart = imageChunk + WebpConstants.ChunkHeaderSize + (isLossless ? 5 : WebpConstants.Vp8FrameHeaderSize); + int bitstreamEnd = imageChunk + WebpConstants.ChunkHeaderSize + BinaryPrimitives.ReadInt32LittleEndian(original.AsSpan(imageChunk + 4)); + + // The format detector needs the complete 12 byte RIFF header to recognize the file. + for (int length = WebpConstants.RiffHeaderSize; length < original.Length; length++) + { + byte[] truncated = original.AsSpan(0, length).ToArray(); + AssertDecodesOrThrowsInvalidImageContent(() => _ = Image.Identify(truncated), length); + if (length < bitstreamStart || length >= bitstreamEnd) + { + AssertDecodesOrThrowsInvalidImageContent( + () => + { + using Image image = Image.Load(truncated); + }, + length); + } + } + } + + /// + /// Encodes a small image whose container has the chunks described by . + /// + /// The kind of file to encode. + /// The encoded file. + private static byte[] EncodeContainerSample(ContainerSample sample) + { + bool lossless = sample is ContainerSample.Lossless or ContainerSample.LosslessWithExif or ContainerSample.LosslessWithIcc; + bool alpha = sample is ContainerSample.LossyWithAlpha or ContainerSample.LosslessWithExif; + + using Image image = new(23, 17); + image.ProcessPixelRows(accessor => + { + for (int y = 0; y < accessor.Height; y++) + { + Span row = accessor.GetRowSpan(y); + for (int x = 0; x < row.Length; x++) + { + byte a = alpha ? (byte)(255 - (x * 9)) : (byte)255; + row[x] = new Rgba32((byte)(x * 11), (byte)(y * 15), (byte)((x + y) * 6), a); + } + } + }); + + if (sample is ContainerSample.LosslessWithExif or ContainerSample.LossyWithExifAndXmp) + { + ExifProfile exifProfile = new(); + exifProfile.SetValue(ExifTag.Software, "ImageSharp"); + image.Metadata.ExifProfile = exifProfile; + } + + if (sample is ContainerSample.LossyWithExifAndXmp) + { + image.Metadata.XmpProfile = new XmpProfile(""u8.ToArray()); + } + + if (sample is ContainerSample.LosslessWithIcc) + { + image.Metadata.IccProfile = new IccProfile(IccTestDataProfiles.ProfileRandomArray); + } + + using MemoryStream stream = new(); + image.Save(stream, new WebpEncoder { FileFormat = lossless ? WebpFileFormatType.Lossless : WebpFileFormatType.Lossy }); + return stream.ToArray(); + } + + /// + /// Asserts that both files decode and identify to the same pixels, metadata and image information. + /// + /// The unmodified file. + /// The modified file. + private static void AssertSameDecodedImage(byte[] expectedData, byte[] actualData) + { + using Image expected = Image.Load(expectedData); + using Image actual = Image.Load(actualData); + ImageComparer.Exact.VerifySimilarity(expected, actual); + Assert.Equal(expected.Metadata.ExifProfile?.ToByteArray(), actual.Metadata.ExifProfile?.ToByteArray()); + Assert.Equal(expected.Metadata.XmpProfile?.ToByteArray(), actual.Metadata.XmpProfile?.ToByteArray()); + Assert.Equal(expected.Metadata.IccProfile?.ToByteArray(), actual.Metadata.IccProfile?.ToByteArray()); + + ImageInfo expectedInfo = Image.Identify(expectedData); + ImageInfo actualInfo = Image.Identify(actualData); + Assert.Equal(expectedInfo.Size, actualInfo.Size); + Assert.Equal(expectedInfo.PixelType.BitsPerPixel, actualInfo.PixelType.BitsPerPixel); + } + + /// + /// Asserts that an operation on a truncated file either succeeds or reports invalid image content. + /// + /// The decoding operation. + /// The truncated file length, reported on failure. + private static void AssertDecodesOrThrowsInvalidImageContent(Action operation, int length) + { + Exception exception = Record.Exception(operation); + Assert.True(exception is null or InvalidImageContentException, $"Truncated to {length} bytes: {exception}"); + } + + /// + /// Gets the offset of the first chunk with the given FourCC. + /// + /// The encoded file. + /// The chunk FourCC. + /// The offset of the chunk header. + private static int FindChunk(byte[] data, string fourCc) + { + Assert.True(TryFindChunk(data, fourCc, out int offset), $"Chunk '{fourCc}' not found."); + return offset; + } + + /// + /// Walks the chunks after the RIFF header to find the first chunk with the given FourCC. + /// + /// The encoded file. + /// The chunk FourCC. + /// The offset of the chunk header, when found. + /// Whether the chunk was found. + private static bool TryFindChunk(byte[] data, string fourCc, out int offset) + { + offset = WebpConstants.RiffHeaderSize; + while (offset + WebpConstants.ChunkHeaderSize <= data.Length) + { + if (Encoding.ASCII.GetString(data, offset, 4) == fourCc) + { + return true; + } + + // Odd-sized payloads are followed by a padding byte. + int size = BinaryPrimitives.ReadInt32LittleEndian(data.AsSpan(offset + 4)); + offset += WebpConstants.ChunkHeaderSize + size + (size & 1); + } + + return false; + } } diff --git a/tests/ImageSharp.Tests/Formats/WebP/WebpMetaDataTests.cs b/tests/ImageSharp.Tests/Formats/WebP/WebpMetaDataTests.cs index 324070e4a5..50eb60d90a 100644 --- a/tests/ImageSharp.Tests/Formats/WebP/WebpMetaDataTests.cs +++ b/tests/ImageSharp.Tests/Formats/WebP/WebpMetaDataTests.cs @@ -8,6 +8,7 @@ using SixLabors.ImageSharp.Metadata.Profiles.Exif; using SixLabors.ImageSharp.Metadata.Profiles.Icc; using SixLabors.ImageSharp.PixelFormats; +using SixLabors.ImageSharp.Tests.TestDataIcc; using SixLabors.ImageSharp.Tests.TestUtilities; // ReSharper disable InconsistentNaming @@ -286,8 +287,9 @@ public void Decode_WithOversizedMetadataChunk_ThrowsInvalidImageContentException [InlineData("XMP ")] public void Decode_WithMetadataChunkLargerThanRemainingData_ThrowsInStrictMode(string chunkType) { + // The RIFF size covers the whole payload, so the metadata chunk is read rather than ignored as trailing data. byte[] payload = Convert.FromHexString( - "524946460000000057454250565038580A00000020000000010000010000494343500000004000000000"); + "524946462200000057454250565038580A00000020000000010000010000494343500000004000000000"); Encoding.ASCII.GetBytes(chunkType, payload.AsSpan(30, 4)); DecoderOptions options = new() { SegmentIntegrityHandling = SegmentIntegrityHandling.Strict }; @@ -366,6 +368,62 @@ public void Decode_TruncatedIccFraming_RemainsFatal(SegmentIntegrityHandling int Assert.Throws(() => Image.Identify(options, payload)); } + [Theory] + [InlineData(false)] + [InlineData(true)] + public void Decode_OddLengthMetadataChunks_ReadsPayloadsWithoutPaddingByte(bool skipMetadata) + { + // An ICC profile may have any length its header declares, so one extra byte makes it odd. + byte[] icc = [.. IccTestDataProfiles.ProfileRandomArray, 0x2A]; + BinaryPrimitives.WriteUInt32BigEndian(icc, (uint)icc.Length); + + ExifProfile exifProfile = new(); + exifProfile.SetValue(ExifTag.Software, "ImageSharp"); + byte[] exif = exifProfile.ToByteArray(); + if (exif.Length % 2 == 0) + { + // The EXIF reader ignores bytes after the last directory. + exif = [.. exif, 0x2A]; + } + + byte[] xmp = " "u8.ToArray(); + Assert.True(icc.Length % 2 == 1 && exif.Length % 2 == 1 && xmp.Length % 2 == 1); + + using Image source = new(1, 1, new Rgba32(255, 0, 0)); + using MemoryStream encoded = new(); + source.Save(encoded, new WebpEncoder { FileFormat = WebpFileFormatType.Lossless }); + byte[] simple = encoded.ToArray(); + + // RiffHelper writes the declared odd payload length followed by a zero padding byte. Placing chunks after + // each padded payload also checks that the padding is still skipped before the next chunk header. + using MemoryStream output = new(); + output.Write("RIFF\0\0\0\0WEBP"u8); + RiffHelper.WriteChunk(output, (uint)WebpChunkType.Vp8X, [0x2C, 0, 0, 0, 0, 0, 0, 0, 0, 0]); + RiffHelper.WriteChunk(output, (uint)WebpChunkType.Iccp, icc); + output.Write(simple.AsSpan(12)); + RiffHelper.WriteChunk(output, (uint)WebpChunkType.Exif, exif); + RiffHelper.WriteChunk(output, (uint)WebpChunkType.Xmp, xmp); + byte[] payload = output.ToArray(); + BinaryPrimitives.WriteUInt32LittleEndian(payload.AsSpan(4), (uint)payload.Length - 8); + + DecoderOptions options = new() { SkipMetadata = skipMetadata }; + using Image image = Image.Load(options, payload); + + Assert.Equal(new Rgba32(255, 0, 0), image[0, 0]); + if (skipMetadata) + { + Assert.Null(image.Metadata.IccProfile); + Assert.Null(image.Metadata.ExifProfile); + Assert.Null(image.Metadata.XmpProfile); + } + else + { + Assert.Equal(icc, image.Metadata.IccProfile.ToByteArray()); + Assert.Equal("ImageSharp", image.Metadata.ExifProfile.GetValue(ExifTag.Software).Value); + Assert.Equal(xmp, image.Metadata.XmpProfile.ToByteArray()); + } + } + /// /// Places a metadata declaration around a complete lossless image to test recovery independently of pixel truncation. ///