Fix memory corruption in the GIF decoder if a GIF specified a frame with no p...

webkit/port/platform/image-decoders/gif/GIFImageDecoder.cpp

 /*
  * Copyright (C) 2006 Apple Computer, Inc.  All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 327 matching lines @@
 
 void GIFImageDecoder::haveDecodedRow(unsigned frameIndex,
                                      unsigned char* rowBuffer,   // Pointer to single scanline temporary buffer
                                      unsigned char* rowEnd,
                                      unsigned rowNumber,  // The row index
                                      unsigned repeatCount,  // How many times to repeat the row
                                      bool writeTransparentPixels)
 {
     // Initialize the frame if necessary.
     RGBA32Buffer& buffer = m_frameBufferCache[frameIndex];
-    if (buffer.status() == RGBA32Buffer::FrameEmpty) {
-        if (!initFrameBuffer(frameIndex))
-          return;
-    }
+    if ((buffer.status() == RGBA32Buffer::FrameEmpty) && !initFrameBuffer(frameIndex))
+        return;
 
     // Do nothing for bogus data.
     if (rowBuffer == 0 || static_cast<int>(m_reader->frameYOffset() + rowNumber) >= size().height())
       return;
 
     unsigned colorMapSize;
     unsigned char* colorMap;
     m_reader->getColorMap(colorMap, colorMapSize);
     if (!colorMap)
         return;
@@ skipping 45 matching lines @@
             if (currDst + num > end) // Protect against a buffer overrun from a bogus repeatCount.
                 break;
             memcpy(currDst, dst, data_size);
             currDst += width;
         }
     }
 }
 
 void GIFImageDecoder::frameComplete(unsigned frameIndex, unsigned frameDuration, RGBA32Buffer::FrameDisposalMethod disposalMethod)
 {
+    // Initialize the frame if necessary.  Some GIFs insert do-nothing frames,
+    // so we never reach haveDecodedRow() before getting here.

[brettw, 2008/12/16 21:21:56]

"so we never" -> "in which case we never" (seems like you might be implying you
never reach here because some gifs insert do nothing frames).

     RGBA32Buffer& buffer = m_frameBufferCache[frameIndex];
+    if ((buffer.status() == RGBA32Buffer::FrameEmpty) && !initFrameBuffer(frameIndex))

[Mohamed Mansour (USE mhm), 2008/12/16 21:39:23]

my turn :)
nit: let second parameter go in the next line since it is > 80 chars

[Peter Kasting, 2008/12/16 21:42:17]

No, this is WebKit code and WebKit style.  Google style is 80-column limit,
WebKit style is to have arbitrary-length lines.

+        return;
+
     buffer.setStatus(RGBA32Buffer::FrameComplete);
     buffer.setDuration(frameDuration);
     buffer.setDisposalMethod(disposalMethod);
 
     if (!m_currentBufferSawAlpha) {
         // The whole frame was non-transparent, so it's possible that the entire
         // resulting buffer was non-transparent, and we can setHasAlpha(false).
         if (buffer.rect().contains(IntRect(IntPoint(0, 0), size()))) {
             buffer.setHasAlpha(false);
         } else if (frameIndex > 0) {
@@ skipping 30 matching lines @@
 
 void GIFImageDecoder::gifComplete()
 {
     if (m_reader)
         m_repetitionCount = m_reader->repetitionCount();
     delete m_reader;
     m_reader = 0;
 }
 
 }