Fix #2801 and secure allocation

SixLabors · Oct 15, 2024 · 4b3afc1 · 4b3afc1
1 parent 63cae5c
commit 4b3afc1
Show file tree

Hide file tree

Showing 4 changed files with 19 additions and 10 deletions.
diff --git a/src/ImageSharp/Formats/Webp/AlphaDecoder.cs b/src/ImageSharp/Formats/Webp/AlphaDecoder.cs
@@ -183,7 +183,7 @@ public void Decode()
         else
         {
             this.LosslessDecoder.DecodeImageData(this.Vp8LDec, this.Vp8LDec.Pixels.Memory.Span);
-            this.ExtractAlphaRows(this.Vp8LDec);
+            this.ExtractAlphaRows(this.Vp8LDec, this.Width);
         }
     }
 
@@ -257,14 +257,15 @@ public void ExtractPalettedAlphaRows(int lastRow)
     /// Once the image-stream is decoded into ARGB color values, the transparency information will be extracted from the green channel of the ARGB quadruplet.
     /// </summary>
     /// <param name="dec">The VP8L decoder.</param>
-    private void ExtractAlphaRows(Vp8LDecoder dec)
+    /// <param name="width">The image width.</param>
+    private void ExtractAlphaRows(Vp8LDecoder dec, int width)
     {
         int numRowsToProcess = dec.Height;
-        int width = dec.Width;
         Span<uint> input = dec.Pixels.Memory.Span;
         Span<byte> output = this.Alpha.Memory.Span;
 
         // Extract alpha (which is stored in the green plane).
+        // the final width (!= dec->width_)
         int pixelCount = width * numRowsToProcess;
         WebpLosslessDecoder.ApplyInverseTransforms(dec, input, this.memoryAllocator);
         ExtractGreen(input, output, pixelCount);

diff --git a/src/ImageSharp/Formats/Webp/Lossless/LosslessUtils.cs b/src/ImageSharp/Formats/Webp/Lossless/LosslessUtils.cs
@@ -269,7 +269,11 @@ private static void SubtractGreenFromBlueAndRedScalar(Span<uint> pixelData)
     /// </summary>
     /// <param name="transform">The transform data contains color table size and the entries in the color table.</param>
     /// <param name="pixelData">The pixel data to apply the reverse transform on.</param>
-    public static void ColorIndexInverseTransform(Vp8LTransform transform, Span<uint> pixelData)
+    /// <param name="outputSpan">The resulting pixel data with the reversed transformation data.</param>
+    public static void ColorIndexInverseTransform(
+        Vp8LTransform transform,
+        Span<uint> pixelData,
+        Span<uint> outputSpan)
     {
         int bitsPerPixel = 8 >> transform.Bits;
         int width = transform.XSize;
@@ -282,7 +286,6 @@ public static void ColorIndexInverseTransform(Vp8LTransform transform, Span<uint
             int countMask = pixelsPerByte - 1;
             int bitMask = (1 << bitsPerPixel) - 1;
 
-            uint[] decodedPixelData = new uint[width * height];
             int pixelDataPos = 0;
             for (int y = 0; y < height; y++)
             {
@@ -298,12 +301,12 @@ public static void ColorIndexInverseTransform(Vp8LTransform transform, Span<uint
                         packedPixels = GetArgbIndex(pixelData[pixelDataPos++]);
                     }
 
-                    decodedPixelData[decodedPixels++] = colorMap[(int)(packedPixels & bitMask)];
+                    outputSpan[decodedPixels++] = colorMap[(int)(packedPixels & bitMask)];
                     packedPixels >>= bitsPerPixel;
                 }
             }
 
-            decodedPixelData.AsSpan().CopyTo(pixelData);
+            outputSpan.CopyTo(pixelData);
         }
         else
         {

diff --git a/src/ImageSharp/Formats/Webp/Lossless/WebpLosslessDecoder.cs b/src/ImageSharp/Formats/Webp/Lossless/WebpLosslessDecoder.cs
@@ -684,6 +684,7 @@ public static void ApplyInverseTransforms(Vp8LDecoder decoder, Span<uint> pixelD
         List<Vp8LTransform> transforms = decoder.Transforms;
         for (int i = transforms.Count - 1; i >= 0; i--)
         {
+            // TODO: Review these 1D allocations. They could conceivably exceed limits.
             Vp8LTransform transform = transforms[i];
             switch (transform.TransformType)
             {
@@ -701,7 +702,11 @@ public static void ApplyInverseTransforms(Vp8LDecoder decoder, Span<uint> pixelD
                     LosslessUtils.ColorSpaceInverseTransform(transform, pixelData);
                     break;
                 case Vp8LTransformType.ColorIndexingTransform:
-                    LosslessUtils.ColorIndexInverseTransform(transform, pixelData);
+                    using (IMemoryOwner<uint> output = memoryAllocator.Allocate<uint>(transform.XSize * transform.YSize, AllocationOptions.Clean))
+                    {
+                        LosslessUtils.ColorIndexInverseTransform(transform, pixelData, output.GetSpan());
+                    }
+
                     break;
             }
         }

diff --git a/tests/ImageSharp.Tests/Formats/WebP/WebpEncoderTests.cs b/tests/ImageSharp.Tests/Formats/WebP/WebpEncoderTests.cs
@@ -524,11 +524,11 @@ public void WebpDecoder_CanDecode_Issue2801<TPixel>(TestImageProvider<TPixel> pr
     {
         WebpEncoder encoder = new()
         {
+            Quality = 100
         };
 
         using Image<TPixel> image = provider.GetImage();
-        image.DebugSave(provider);
-        image.VerifyEncoder(provider, "webp", string.Empty, encoder);
+        image.VerifyEncoder(provider, "webp", string.Empty, encoder, ImageComparer.TolerantPercentage(0.0994F));
     }
 
     public static void RunEncodeLossy_WithPeakImage()