Fix bmp bug in ReadHeader()

src/codec/SkBmpCodec.cpp

 /*
  * Copyright 2015 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #include "SkBmpCodec.h"
 #include "SkBmpMaskCodec.h"
 #include "SkBmpRLECodec.h"
@@ skipping 439 matching lines @@
     // Check for a valid number of total bytes when in RLE mode
     if (totalBytes <= offset && kRLE_BmpInputFormat == inputFormat) {
         SkCodecPrintf("Error: RLE requires valid input size.\n");
         return false;
     }
     const size_t RLEBytes = totalBytes - offset;
 
     // Calculate the number of bytes read so far
     const uint32_t bytesRead = kBmpHeaderBytes + infoBytes + maskBytes;
     if (!inIco && offset < bytesRead) {
+        // TODO (msarett): Do we really want to fail if the offset in the header is invalid?
+        //                 Seems like we can just assume that the offset is zero and try to decode?
+        //                 Maybe we don't want to try to decode corrupt images?

[scroggo, 2015/09/01 21:31:25]

All good questions. I think we should support an implicit zero offset if this is
a common use case.

If the rest of our code is robust, getting some false positives here (i.e. we
assumed the offset was zero and the rest of the file was not valid) is probably
okay.

         SkCodecPrintf("Error: pixel data offset less than header size.\n");
         return false;
     }
 
+    // Skip to the start of the pixel array.
+    // We can do this here because there is no color table to read
+    // in bit mask mode.
+    if (!inIco && kBitMask_BmpInputFormat == inputFormat) {
+        if (stream->skip(offset - bytesRead) != offset - bytesRead) {
+            SkCodecPrintf("Error: unable to skip to image data.\n");
+            return false;
+        }
+    }
+
     if (codecOut) {
         // Set the image info
         const SkImageInfo& imageInfo = SkImageInfo::Make(width, height,
                 colorType, alphaType);
 
         // Return the codec
         switch (inputFormat) {
             case kStandard_BmpInputFormat:
-                *codecOut =
-                        new SkBmpStandardCodec(imageInfo, stream, bitsPerPixel, numColors,
-                                               bytesPerColor, offset - bytesRead, rowOrder, inIco);
+                *codecOut = new SkBmpStandardCodec(imageInfo, stream, bitsPerPixel, numColors,
+                        bytesPerColor, offset - bytesRead, rowOrder, inIco);
                 return true;
             case kBitMask_BmpInputFormat:
                 // Bmp-in-Ico must be standard mode
                 if (inIco) {
                     SkCodecPrintf("Error: Icos may not use bit mask format.\n");
                     return false;
                 }
-                // Skip to the start of the pixel array.
-                // We can do this here because there is no color table to read
-                // in bit mask mode.
-                if (stream->skip(offset - bytesRead) != offset - bytesRead) {
-                    SkCodecPrintf("Error: unable to skip to image data.\n");
-                    return false;
-                }
 
                 *codecOut = new SkBmpMaskCodec(imageInfo, stream, bitsPerPixel, masks.detach(),
-                                               rowOrder);
+                        rowOrder);
                 return true;
             case kRLE_BmpInputFormat:
                 // Bmp-in-Ico must be standard mode
                 // When inIco is true, this line cannot be reached, since we
                 // require that RLE Bmps have a valid number of totalBytes, and
                 // Icos skip the header that contains totalBytes.
                 SkASSERT(!inIco);
-                *codecOut =
-                        new SkBmpRLECodec(imageInfo, stream, bitsPerPixel, numColors, bytesPerColor,
-                                          offset - bytesRead, rowOrder, RLEBytes);
+                *codecOut = new SkBmpRLECodec(imageInfo, stream, bitsPerPixel, numColors,
+                        bytesPerColor, offset - bytesRead, rowOrder, RLEBytes);
                 return true;
             default:
                 SkASSERT(false);
                 return false;
         }
     }
 
     return true;
 }
 
@@ skipping 119 matching lines @@
 };
 
 SkScanlineDecoder* SkBmpCodec::NewSDFromStream(SkStream* stream) {
     SkAutoTDelete<SkBmpCodec> codec(static_cast<SkBmpCodec*>(SkBmpCodec::NewFromStream(stream)));
     if (!codec) {
         return NULL;
     }
 
     return SkNEW_ARGS(SkBmpScanlineDecoder, (codec.detach()));
 }