From 9d79d4bc420b040d6f9a49f4e109cab65fcd5dca Mon Sep 17 00:00:00 2001 From: Jason Finch Date: Sun, 18 Oct 2026 09:33:09 +1000 Subject: [PATCH] Make the WebP decoder tolerate reserved VP8X bits, trailing data and odd-length metadata chunks Readers must ignore the reserved VP8X flag bits, so they are no longer validated. Chunks are now read only up to the end of the RIFF payload, so bytes after it are ignored rather than parsed as more chunks, and unknown chunks larger than the payload end the parsing instead of becoming a negative skip. A stream that ends inside a chunk header, a 24 bit field or the VP8L header now throws InvalidImageContentException. Metadata chunks with an odd payload length are read without the RIFF padding byte, which is skipped before the next chunk. --- .../Formats/Webp/WebpChunkParsingUtils.cs | 81 +++--- .../Formats/Webp/WebpDecoderCore.cs | 38 ++- .../Formats/WebP/WebpDecoderTests.cs | 262 ++++++++++++++++++ .../Formats/WebP/WebpMetaDataTests.cs | 60 +++- 4 files changed, 394 insertions(+), 47 deletions(-) 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. ///