REGRESSION(277476@main): [GTK] Crash in WebCore::GIFImageDecoder::hav…

…eDecodedRow

https://bugs.webkit.org/show_bug.cgi?id=274027

Reviewed by Carlos Garcia Campos.

Confusingly, the "size" of the color maps is defined in 3-byte units, so
size in bytes is actually 3x the "size" of the color map. Chris
understandably missed this when converting the code to use std::span.
Now we're reading off the end of the span. This triggers libstdc++
runtime assertions, but the assertions are disabled by default, so our
EWS bots did not notice. Distros do (or should) enable the assertions
using something like -DCMAKE_CXX_FLAGS="-Wp,-D_GLIBCXX_ASSERTIONS".

* Source/WebCore/platform/image-decoders/gif/GIFImageDecoder.cpp:
(WebCore::GIFImageDecoder::haveDecodedRow):
* Source/WebCore/platform/image-decoders/gif/GIFImageReader.h:
(GIFImageReader::globalColormap const):
(GIFImageReader::localColormap const):

Canonical link: https://commits.webkit.org/278739@main

Source/WebCore/platform/image-decoders/gif/GIFImageDecoder.cpp

@@ -234,8 +234,8 @@ bool GIFImageDecoder::haveDecodedRow(unsigned frameIndex, const Vector<unsigned
     // Write one row's worth of data into the frame.  
     for (int x = xBegin; x < xEnd; ++x) {
         const unsigned char sourceValue = rowBuffer[x - frameContext->xOffset];
-        if ((!frameContext->isTransparent || (sourceValue != frameContext->tpixel)) && (sourceValue < colorMap.size())) {
-            const size_t colorIndex = static_cast<size_t>(sourceValue) * 3;
+        const size_t colorIndex = static_cast<size_t>(sourceValue) * 3;
+        if ((!frameContext->isTransparent || (sourceValue != frameContext->tpixel)) && (colorIndex + 2 < colorMap.size())) {
             buffer.backingStore()->setPixel(currentAddress, colorMap[colorIndex], colorMap[colorIndex + 1], colorMap[colorIndex + 2], 255);
         } else {
             m_currentBufferSawAlpha = true;

Source/WebCore/platform/image-decoders/gif/GIFImageReader.h

@@ -156,7 +156,7 @@ struct GIFFrameContext {
     int tpixel; // Index of transparent pixel.
     WebCore::ScalableImageDecoderFrame::DisposalMethod disposalMethod; // Restore to background, leave in place, etc.
     size_t localColormapPosition; // Per-image colormap.
-    int localColormapSize; // Size of local colormap array.
+    int localColormapSize; // Size of local colormap array (in 3-byte units)
     int datasize;
 
     bool isLocalColormapDefined : 1;
@@ -255,12 +255,12 @@ class GIFImageReader {
 
     std::span<const uint8_t> globalColormap() const
     {
-        return m_isGlobalColormapDefined ? data(m_globalColormapPosition, m_globalColormapSize) : std::span<const uint8_t> { };
+        return m_isGlobalColormapDefined ? data(m_globalColormapPosition, m_globalColormapSize * 3) : std::span<const uint8_t> { };
     }
 
     std::span<const uint8_t> localColormap(const GIFFrameContext* frame) const
     {
-        return frame->isLocalColormapDefined ? data(frame->localColormapPosition, frame->localColormapSize) : std::span<const uint8_t> { };
+        return frame->isLocalColormapDefined ? data(frame->localColormapPosition, frame->localColormapSize * 3) : std::span<const uint8_t> { };
     }
 
     const GIFFrameContext* frameContext() const
@@ -302,7 +302,7 @@ class GIFImageReader {
     unsigned m_screenHeight;
     bool m_isGlobalColormapDefined;
     size_t m_globalColormapPosition; // (3* MAX_COLORS in size) Default colormap if local not supplied, 3 bytes for each color.
-    int m_globalColormapSize; // Size of global colormap array.
+    int m_globalColormapSize; // Size of global colormap array (in 3-byte units)
     int m_loopCount; // Netscape specific extension block to control the number of animation loops a GIF renders.
 
     Vector<std::unique_ptr<GIFFrameContext> > m_frames;