Make the WebP decoder tolerate reserved VP8X bits, trailing data and odd-length metadata chunks

src/ImageSharp/Formats/Webp/Chunks/WebpFrameData.cs

@@ -121,13 +121,14 @@ public long WriteHeaderTo(Stream stream)
     /// Reads the animation frame header.
     /// </summary>
     /// <param name="stream">The stream to read from.</param>
+    /// <param name="containerEnd">The end of the RIFF payload containing this frame.</param>
     /// <returns>Animation frame data.</returns>
-    public static WebpFrameData Parse(BufferedReadStream stream)
+    public static WebpFrameData Parse(BufferedReadStream stream, long containerEnd)
     {
         Span<byte> buffer = stackalloc byte[4];
 
         return new WebpFrameData(
-            dataSize: WebpChunkParsingUtils.ReadChunkSize(stream, buffer),
+            dataSize: WebpChunkParsingUtils.ReadChunkSize(stream, buffer, containerEnd, 16, false),
             x: WebpChunkParsingUtils.ReadUInt24LittleEndian(stream, buffer) * 2,
             y: WebpChunkParsingUtils.ReadUInt24LittleEndian(stream, buffer) * 2,
             width: WebpChunkParsingUtils.ReadUInt24LittleEndian(stream, buffer) + 1,

tests/ImageSharp.Tests/Formats/WebP/WebpDecoderTests.cs

@@ -406,8 +406,15 @@ public void Decode_AnimatedLossy_AlphaBlending_Works<TPixel>(TestImageProvider<T
     public void WebpDecoder_CanDecode_Lossless_WithIssues<TPixel>(TestImageProvider<TPixel> provider)
         where TPixel : unmanaged, IPixel<TPixel>
     {
-        // Just make sure no exception is thrown. The reference decoder fails to load the image.
-        using Image<TPixel> image = provider.GetImage(WebpDecoder.Instance);
+        // These stored files end before their declared VP8L payload ends. Local libwebp reports NOT_ENOUGH_DATA.
+        // Recovery belongs to IgnoreImageData; ignoring ancillary errors must not permit missing image bytes.
+        DecoderOptions strict = new() { SegmentIntegrityHandling = SegmentIntegrityHandling.Strict };
+        DecoderOptions ancillary = new() { SegmentIntegrityHandling = SegmentIntegrityHandling.IgnoreAncillary };
+        Assert.Throws<InvalidImageContentException>(() => provider.GetImage(WebpDecoder.Instance, strict));
+        Assert.Throws<InvalidImageContentException>(() => provider.GetImage(WebpDecoder.Instance, ancillary));
+
+        DecoderOptions options = new() { SegmentIntegrityHandling = SegmentIntegrityHandling.IgnoreImageData };
+        using Image<TPixel> image = provider.GetImage(WebpDecoder.Instance, options);
         image.DebugSave(provider);
     }
 
@@ -501,10 +508,11 @@ public void WebpDecoder_CanDecode_Issue2866<TPixel>(TestImageProvider<TPixel> pr
     [WithFile(Lossless.LossLessCorruptImage3, PixelTypes.Rgba32)]
     public void WebpDecoder_ThrowImageFormatException_OnInvalidImages<TPixel>(TestImageProvider<TPixel> provider)
         where TPixel : unmanaged, IPixel<TPixel> =>
+        // Ignore the missing tail so this assertion still tests the invalid compressed color transform.
         Assert.Throws<ImageFormatException>(
             () =>
             {
-                using (provider.GetImage(WebpDecoder.Instance))
+                using (provider.GetImage(WebpDecoder.Instance, new DecoderOptions { SegmentIntegrityHandling = SegmentIntegrityHandling.IgnoreImageData }))
                 {
                 }
             });
@@ -621,4 +629,81 @@ public void Decode_WhenColorProfileHandlingIsConvert_ApplyIccProfile<TPixel>(Tes
         image.CompareToReferenceOutput(provider);
         Assert.Null(image.Metadata.IccProfile);
     }
+
+    /// <summary>
+    /// Checks that ignored container fields and data do not change pixels or metadata.
+    /// </summary>
+    [Theory]
+    [InlineData(ContainerDirectory + "odd-metadata.webp", ContainerDirectory + "reserved-fields.webp")]
+    [InlineData(Lossy.Small02, ContainerDirectory + "lossy-trailing.webp")]
+    [InlineData(Lossless.NoTransform1, ContainerDirectory + "lossless-trailing.webp")]
+    [InlineData(Lossy.AlphaCompressedHorizontalFilter, ContainerDirectory + "alpha-trailing.webp")]
+    [InlineData(ContainerDirectory + "odd-metadata.webp", ContainerDirectory + "metadata-trailing.webp")]
+    [InlineData(ContainerDirectory + "odd-metadata.webp", ContainerDirectory + "unknown-signed.webp")]
+    [InlineData(ContainerDirectory + "odd-metadata.webp", ContainerDirectory + "unknown-unsigned.webp")]
+    [InlineData(ContainerDirectory + "odd-metadata.webp", ContainerDirectory + "unknown-maximum.webp")]
+    [InlineData(ContainerDirectory + "animation.webp", ContainerDirectory + "animation-trailing.webp")]
+    [InlineData(ContainerDirectory + "animation.webp", ContainerDirectory + "animation-unknown.webp")]
+    public void Decode_ContainerVariations_PreservePixelsAndMetadata(string originalFile, string modifiedFile)
+    {
+        // Local libwebp confirms identical still pixels and two frames for each animation variant.
+        // The stored files alter reserved fields, append bytes outside RIFF, or add an unknown chunk.
+        byte[] original = TestFile.Create(originalFile).Bytes;
+        byte[] modified = TestFile.Create(modifiedFile).Bytes;
+        DecoderOptions options = new() { MaxFrames = 2 };
+        using Image<Rgba32> expected = Image.Load<Rgba32>(options, original);
+        using Image<Rgba32> actual = Image.Load<Rgba32>(options, modified);
+
+        ImageComparer.Exact.VerifySimilarity(expected, actual);
+        Assert.Equal(expected.Frames.Count, actual.Frames.Count);
+        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(options, original);
+        ImageInfo actualInfo = Image.Identify(options, modified);
+        Assert.Equal(expectedInfo.Size, actualInfo.Size);
+        Assert.Equal(expectedInfo.FrameMetadataCollection.Count, actualInfo.FrameMetadataCollection.Count);
+    }
+
+    /// <summary>
+    /// Rejects headers and image payloads whose declared extent exceeds RIFF.
+    /// </summary>
+    [Theory]
+    [InlineData(ContainerDirectory + "vp8-crosses-riff.webp")]
+    [InlineData(ContainerDirectory + "vp8l-crosses-riff.webp")]
+    [InlineData(ContainerDirectory + "vp8x-crosses-riff.webp")]
+    [InlineData(ContainerDirectory + "alpha-crosses-riff.webp")]
+    [InlineData(ContainerDirectory + "anim-crosses-riff.webp")]
+    [InlineData(ContainerDirectory + "frame-crosses-riff.webp")]
+    [InlineData(ContainerDirectory + "vp8l-short-header.webp")]
+    [InlineData(ContainerDirectory + "vp8x-short-header.webp")]
+    public void Decode_ChunkOutsideRiff_RejectsInvalidFraming(string fileName)
+    {
+        byte[] data = TestFile.Create(fileName).Bytes;
+
+        // These files retain bytes after RIFF. Those bytes must not complete a header or image payload.
+        // Framing errors cannot be recovered by ignoring image or ancillary segment contents.
+        foreach (SegmentIntegrityHandling integrity in Enum.GetValues<SegmentIntegrityHandling>())
+        {
+            DecoderOptions options = new() { SegmentIntegrityHandling = integrity };
+            Assert.Throws<InvalidImageContentException>(() => Image.Load(options, data));
+            Assert.Throws<InvalidImageContentException>(() => Image.Identify(options, data));
+        }
+    }
+
+    /// <summary>
+    /// Rejects an image payload that borrows bytes from the next animation frame.
+    /// </summary>
+    [Fact]
+    public void Decode_FrameImageOutsideAnmf_RejectsInvalidFraming()
+    {
+        // The VP8L payload extends into the next ANMF chunk. Local libwebp rejects this file.
+        byte[] data = TestFile.Create(ContainerDirectory + "frame-crosses-anmf.webp").Bytes;
+        foreach (SegmentIntegrityHandling integrity in Enum.GetValues<SegmentIntegrityHandling>())
+        {
+            DecoderOptions options = new() { SegmentIntegrityHandling = integrity };
+            Assert.Throws<InvalidImageContentException>(() => Image.Load(options, data));
+        }
+    }
 }

tests/ImageSharp.Tests/Formats/WebP/WebpMetaDataTests.cs

@@ -1,13 +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.TestDataIcc;
 using SixLabors.ImageSharp.Tests.TestUtilities;
 
 // ReSharper disable InconsistentNaming
@@ -38,8 +37,9 @@ public static IEnumerable<object[]> TruncatedMetadataOptions()
             {
                 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 };
+                    string fileName = $"{TestImages.Webp.ContainerDirectory}{chunkType.Trim().ToLowerInvariant()}-trailing-{length:x8}.webp";
+                    yield return new object[] { fileName, integrity, false };
+                    yield return new object[] { fileName, integrity, true };
                 }
             }
         }
@@ -262,33 +262,19 @@ public void Decode_InvalidExifChunk_ThrowsWithStrict()
     }
 
     [Theory]
-    [InlineData("ICCP", 0xFFFFFFFEU)]
-    [InlineData("EXIF", 0xFFFFFFFEU)]
-    [InlineData("XMP ", 0xFFFFFFFEU)]
-    [InlineData("ICCP", uint.MaxValue)]
-    [InlineData("EXIF", uint.MaxValue)]
-    [InlineData("XMP ", uint.MaxValue)]
+    [InlineData("iccp", 0x40000000U)]
+    [InlineData("exif", 0x40000000U)]
+    [InlineData("xmp", 0x40000000U)]
+    [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<InvalidImageContentException>(() => Image.Load(options, payload));
-        Assert.Throws<InvalidImageContentException>(() => Image.Identify(options, payload));
-    }
-
-    [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));
+        // The stored RIFF size covers the file. The metadata declaration itself exceeds that boundary.
+        byte[] payload = TestFile.Create($"{TestImages.Webp.ContainerDirectory}{chunkType}-oversized-{length:x8}.webp").Bytes;
         DecoderOptions options = new() { SegmentIntegrityHandling = SegmentIntegrityHandling.Strict };
 
         Assert.Throws<InvalidImageContentException>(() => Image.Load(options, payload));
@@ -297,9 +283,9 @@ public void Decode_WithMetadataChunkLargerThanRemainingData_ThrowsInStrictMode(s
 
     [Theory]
     [MemberData(nameof(TruncatedMetadataOptions))]
-    public void Decode_TruncatedTrailingMetadata_RespectsOptions(string chunkType, uint length, SegmentIntegrityHandling integrity, bool skipMetadata)
+    public void Decode_TruncatedTrailingMetadata_RespectsOptions(string fileName, SegmentIntegrityHandling integrity, bool skipMetadata)
     {
-        byte[] payload = CreateWebpWithMetadata(chunkType, length, false, false);
+        byte[] payload = TestFile.Create(fileName).Bytes;
         DecoderOptions options = new() { SegmentIntegrityHandling = integrity, SkipMetadata = skipMetadata };
 
         if (integrity is SegmentIntegrityHandling.Strict && !skipMetadata)
@@ -333,7 +319,7 @@ public void Decode_TruncatedTrailingMetadata_RespectsOptions(string chunkType, u
     [MemberData(nameof(IccMetadataOptions))]
     public void Decode_InvalidIccPayload_RespectsOptionsAndReadsFollowingImage(SegmentIntegrityHandling integrity, bool skipMetadata, bool animated)
     {
-        byte[] payload = CreateWebpWithMetadata("ICCP", 4, true, animated);
+        byte[] payload = TestFile.Create(TestImages.Webp.ContainerDirectory + (animated ? "invalid-icc-animated.webp" : "invalid-icc.webp")).Bytes;
         DecoderOptions options = new() { SegmentIntegrityHandling = integrity, SkipMetadata = skipMetadata };
 
         if (integrity is SegmentIntegrityHandling.Strict && !skipMetadata)
@@ -359,53 +345,74 @@ public void Decode_InvalidIccPayload_RespectsOptionsAndReadsFollowingImage(Segme
     [MemberData(nameof(IccMetadataOptions))]
     public void Decode_TruncatedIccFraming_RemainsFatal(SegmentIntegrityHandling integrity, bool skipMetadata, bool animated)
     {
-        byte[] payload = CreateWebpWithMetadata("ICCP", 0x40000000, true, animated);
+        byte[] payload = TestFile.Create(TestImages.Webp.ContainerDirectory + (animated ? "truncated-icc-animated.webp" : "truncated-icc.webp")).Bytes;
         DecoderOptions options = new() { SegmentIntegrityHandling = integrity, SkipMetadata = skipMetadata };
 
         Assert.Throws<InvalidImageContentException>(() => Image.Load(options, payload));
         Assert.Throws<InvalidImageContentException>(() => Image.Identify(options, payload));
     }
 
-    /// <summary>
-    /// Places a metadata declaration around a complete lossless image to test recovery independently of pixel truncation.
-    /// </summary>
-    private static byte[] CreateWebpWithMetadata(string chunkType, uint length, bool beforeImage, bool animated)
+    [Theory]
+    [InlineData(false, false)]
+    [InlineData(true, false)]
+    [InlineData(false, true)]
+    [InlineData(true, true)]
+    public void Decode_OddLengthMetadataChunks_ReadsPayloadsWithoutPaddingByte(bool skipMetadata, 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<Rgba32> source = new(2, 2, new Rgba32(17, 34, 51));
-        if (animated)
+        // Local libwebp extracts these exact odd-length payloads and decodes the same solid 2x2 image.
+        byte[] payload = TestFile.Create(TestImages.Webp.ContainerDirectory + (animated ? "odd-metadata-animated.webp" : "odd-metadata.webp")).Bytes;
+        byte[] icc = TestFile.Create(TestImages.Webp.ContainerDirectory + "odd.icc").Bytes;
+        byte[] xmp = TestFile.Create(TestImages.Webp.ContainerDirectory + "odd.xmp").Bytes;
+
+        DecoderOptions options = new() { SkipMetadata = skipMetadata };
+        using Image<Rgba32> image = Image.Load<Rgba32>(options, payload);
+
+        // ICCP precedes ANIM. Entering the frame decoder must retain that profile, not replace its metadata owner.
+        Assert.Equal(animated ? 2 : 1, image.Frames.Count);
+        Assert.Equal(new Rgba32(17, 34, 51), image[0, 0]);
+        if (skipMetadata)
         {
-            header[20] |= 0x02;
-            using Image<Rgba32> secondFrame = new(2, 2, new Rgba32(51, 34, 17));
-            source.Frames.AddFrame(secondFrame.Frames.RootFrame);
+            Assert.Null(image.Metadata.IccProfile);
+            Assert.Null(image.Metadata.ExifProfile);
+            Assert.Null(image.Metadata.XmpProfile);
         }
-
-        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)
+        else
         {
-            combined.Write(header.AsSpan(30));
+            Assert.Equal(icc, image.Metadata.IccProfile.ToByteArray());
+            Assert.Equal("ImageSharp", image.Metadata.ExifProfile.GetValue(ExifTag.Software).Value);
+            Assert.Equal(xmp, image.Metadata.XmpProfile.ToByteArray());
         }
+    }
 
-        // 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)
+    /// <summary>
+    /// Applies ancillary recovery only within RIFF and keeps incomplete ICC framing fatal.
+    /// </summary>
+    [Theory]
+    [InlineData(false)]
+    [InlineData(true)]
+    public void Decode_MetadataOutsideRiff_RespectsIntegrity(bool skipMetadata)
+    {
+        // Complete metadata bytes exist in the physical file, but the RIFF declaration excludes part of them.
+        byte[] exif = TestFile.Create(TestImages.Webp.ContainerDirectory + "exif-crosses-riff.webp").Bytes;
+        byte[] icc = TestFile.Create(TestImages.Webp.ContainerDirectory + "icc-crosses-riff.webp").Bytes;
+        foreach (SegmentIntegrityHandling integrity in Enum.GetValues<SegmentIntegrityHandling>())
         {
-            combined.Write(header.AsSpan(30));
-        }
+            DecoderOptions options = new() { SegmentIntegrityHandling = integrity, SkipMetadata = skipMetadata };
+            Assert.Throws<InvalidImageContentException>(() => Image.Load(options, icc));
+            Assert.Throws<InvalidImageContentException>(() => Image.Identify(options, icc));
 
-        byte[] payload = combined.ToArray();
-        BinaryPrimitives.WriteUInt32LittleEndian(payload.AsSpan(4), (uint)payload.Length - 8);
-        return payload;
+            if (integrity == SegmentIntegrityHandling.Strict && !skipMetadata)
+            {
+                Assert.Throws<InvalidImageContentException>(() => Image.Load(options, exif));
+                Assert.Throws<InvalidImageContentException>(() => Image.Identify(options, exif));
+            }
+            else
+            {
+                using Image<Rgba32> image = Image.Load<Rgba32>(options, exif);
+                Assert.Equal(new Rgba32(17, 34, 51), image[0, 0]);
+                Assert.Null(image.Metadata.ExifProfile);
+                Assert.Null(Image.Identify(options, exif).Metadata.ExifProfile);
+            }
+        }
     }
 }

tests/ImageSharp.Tests/TestImages.cs

@@ -787,6 +787,11 @@ public static class Tga
 
     public static class Webp
     {
+        /// <summary>
+        /// Stored WebP container boundary and metadata inputs.
+        /// </summary>
+        public const string ContainerDirectory = "Webp/container/";
+
         // Reference image as png
         public const string Peak = "Webp/peak.png";