check limit of width * height is 32 bits

Update the RIFF-specs file too

Change-Id: I113a7e25da2f7a19c1344c1dc5d496127cfe2596

doc/webp-container-spec.txt

@@ -329,9 +329,9 @@ Extended WebP file header:
     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
     |  R  |L|M|I|A|T|                   Reserved                    |
     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
-    |                    Canvas Width               |             ...
+    |          Canvas Width Minus One               |             ...
     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
-    ...    Canvas Height            |
+    ...  Canvas Height Minus One    |
     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
 
 Tiling (T): 1 bit
@@ -387,15 +387,17 @@ Reserved: 24 bits
 
 : SHOULD be `0`.
 
-Canvas Width: 24 bits
+Canvas Width Minus One: 24 bits
 
 : _1-based_ width of the canvas in pixels.
+  The actual canvas width is '1 + Canvas Width Minus One'
 
-Canvas Height: 24 bits
+Canvas Height Minus One: 24 bits
 
 : _1-based_ height of the canvas in pixels.
+  The actual canvas height is '1 + Canvas Height Minus One'
 
-The product of _Canvas Width_ and _Canvas Height_ SHOULD be at most `2^32 - 1`.
+The product of _Canvas Width_ and _Canvas Height_ MUST be at most `2^32 - 1`.
 
 Future specifications MAY add more fields.
 
@@ -433,11 +435,11 @@ Frame chunk:
     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
     |                        Frame X                |             ...
     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
-    ...          Frame Y            |      Frame Width            ...
+    ...          Frame Y            |   Frame Width Minus One     ...
     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
-    ...             |              Frame Height                     |
+    ...             |           Frame Height Minus One              |
     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
-    |         Frame Duration        |
+    |            Frame Duration Minus One           |
     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
 
 Frame X: 24 bits (_uint24_)
@@ -448,18 +450,20 @@ Frame Y: 24 bits (_uint24_)
 
 : The Y coordinate of the upper left corner of the frame is `Frame Y * 2`
 
-Frame Width: 24 bits (_uint24_)
+Frame Width Minus One: 24 bits (_uint24_)
 
 : The _1-based_ width of the frame.
+  The Frame Width is '1 + Frame Width Minus One'
 
-Frame Height: 24 bits (_uint24_)
+Frame Height Minus One: 24 bits (_uint24_)
 
 : The _1-based_ height of the frame.
+  The Frame Height is '1 + Frame Height Minus One'
 
-Frame Duration: 24 bits (_uint24_)
+Frame Duration Minus One: 24 bits (_uint24_)
 
-: Time to wait before displaying the next frame, in 1 millisecond units
+: _1-based_ time to wait before displaying the next frame, in 1 millisecond
-(_1-based_).
+  units. The actual duration is '1 + Frame Duration Minus One' milliseconds.
 
 For images that are animations, this chunk contains information about a single
 frame, and describes the (optional) alpha chunk and the bitstream chunk that

src/dec/webp.c

@@ -58,7 +58,7 @@ static WEBP_INLINE uint32_t get_le32(const uint8_t* const data) {
 // Returns VP8_STATUS_BITSTREAM_ERROR for invalid header, and
 //         VP8_STATUS_OK otherwise.
 // In case there are not enough bytes (partial RIFF container), return 0 for
-// riff_size. Else return the riff_size extracted from the header.
+// *riff_size. Else return the RIFF size extracted from the header.
 static VP8StatusCode ParseRIFF(const uint8_t** data, size_t* data_size,
                                size_t* riff_size) {
   assert(data);
@@ -88,15 +88,18 @@ static VP8StatusCode ParseRIFF(const uint8_t** data, size_t* data_size,
 // Returns VP8_STATUS_BITSTREAM_ERROR for invalid VP8X header,
 //         VP8_STATUS_NOT_ENOUGH_DATA in case of insufficient data, and
 //         VP8_STATUS_OK otherwise.
-// If a VP8X chunk is found, found_vp8x is set to true and *width, *height and
+// If a VP8X chunk is found, found_vp8x is set to true and *width_ptr,
-// *flags are set to the corresponding values extracted from the VP8X chunk.
+// *height_ptr and *flags_ptr are set to the corresponding values extracted
-static VP8StatusCode ParseVP8X(const uint8_t** data, size_t* data_size,
+// from the VP8X chunk.
-                               int* found_vp8x,
+static VP8StatusCode ParseVP8X(const uint8_t** const data,
-                               int* width, int* height, uint32_t* flags) {
+                               size_t* const data_size,
+                               int* const found_vp8x,
+                               int* const width_ptr, int* const height_ptr,
+                               uint32_t* const flags_ptr) {
   const uint32_t vp8x_size = CHUNK_HEADER_SIZE + VP8X_CHUNK_SIZE;
-  assert(data);
+  assert(data != NULL);
-  assert(data_size);
+  assert(data_size != NULL);
-  assert(found_vp8x);
+  assert(found_vp8x != NULL);
 
   *found_vp8x = 0;
 
@@ -105,6 +108,8 @@ static VP8StatusCode ParseVP8X(const uint8_t** data, size_t* data_size,
   }
 
   if (!memcmp(*data, "VP8X", TAG_SIZE)) {
+    int width, height;
+    uint32_t flags;
     const uint32_t chunk_size = get_le32(*data + TAG_SIZE);
     if (chunk_size != VP8X_CHUNK_SIZE) {
       return VP8_STATUS_BITSTREAM_ERROR;  // Wrong chunk size.
@@ -114,16 +119,16 @@ static VP8StatusCode ParseVP8X(const uint8_t** data, size_t* data_size,
     if (*data_size < vp8x_size) {
       return VP8_STATUS_NOT_ENOUGH_DATA;  // Insufficient data.
     }
+    flags = get_le32(*data + 8);
+    width = 1 + get_le24(*data + 12);
+    height = 1 + get_le24(*data + 15);
+    if (width * (uint64_t)height >= MAX_IMAGE_AREA) {
+      return VP8_STATUS_BITSTREAM_ERROR;  // image is too large
+    }
 
-    if (flags != NULL) {
+    if (flags_ptr != NULL) *flags_ptr = flags;
-      *flags = get_le32(*data + 8);
+    if (width_ptr != NULL) *width_ptr = width;
-    }
+    if (height_ptr != NULL) *height_ptr = height;
-    if (width != NULL) {
-      *width = 1 + get_le24(*data + 12);
-    }
-    if (height != NULL) {
-      *height = 1 + get_le24(*data + 15);
-    }
     // Skip over VP8X header bytes.
     *data += vp8x_size;
     *data_size -= vp8x_size;
@@ -137,7 +142,8 @@ static VP8StatusCode ParseVP8X(const uint8_t** data, size_t* data_size,
 // Returns VP8_STATUS_BITSTREAM_ERROR if any invalid chunk size is encountered,
 //         VP8_STATUS_NOT_ENOUGH_DATA in case of insufficient data, and
 //         VP8_STATUS_OK otherwise.
-// If an alpha chunk is found, alpha_data and alpha_size are set appropriately.
+// If an alpha chunk is found, *alpha_data and *alpha_size are set
+// appropriately.
 static VP8StatusCode ParseOptionalChunks(const uint8_t** data,
                                          size_t* data_size,
                                          size_t riff_size,
@@ -202,9 +208,9 @@ static VP8StatusCode ParseOptionalChunks(const uint8_t** data,
 //         riff_size) VP8/VP8L header,
 //         VP8_STATUS_NOT_ENOUGH_DATA in case of insufficient data, and
 //         VP8_STATUS_OK otherwise.
-// If a VP8/VP8L chunk is found, chunk_size is set to the total number of bytes
+// If a VP8/VP8L chunk is found, *chunk_size is set to the total number of bytes
 // extracted from the VP8/VP8L chunk header.
-// The flag 'is_lossless' is set to 1 in case of VP8L chunk / raw VP8L data.
+// The flag '*is_lossless' is set to 1 in case of VP8L chunk / raw VP8L data.
 static VP8StatusCode ParseVP8Header(const uint8_t** data_ptr, size_t* data_size,
                                     size_t riff_size,
                                     size_t* chunk_size, int* is_lossless) {
@@ -245,9 +251,9 @@ static VP8StatusCode ParseVP8Header(const uint8_t** data_ptr, size_t* data_size,
 
 //------------------------------------------------------------------------------
 
-// Fetch 'width', 'height', 'has_alpha' and fill out 'headers' based on 'data'.
+// Fetch '*width', '*height', '*has_alpha' and fill out 'headers' based on
-// All the output parameters may be NULL. If 'headers' is NULL only the minimal
+// 'data'. All the output parameters may be NULL. If 'headers' is NULL only the
-// amount will be read to fetch the remaining parameters.
+// minimal amount will be read to fetch the remaining parameters.
 // If 'headers' is non-NULL this function will attempt to locate both alpha
 // data (with or without a VP8X chunk) and the bitstream chunk (VP8/VP8L).
 // Note: The following chunk sequences (before the raw VP8/VP8L data) are

src/mux/demux.c

@@ -444,6 +444,9 @@ static ParseStatus ParseVP8X(WebPDemuxer* const dmux) {
   Skip(mem, 3);  // Reserved.
   dmux->canvas_width_  = 1 + GetLE24s(mem);
   dmux->canvas_height_ = 1 + GetLE24s(mem);
+  if (dmux->canvas_width_ * (uint64_t)dmux->canvas_height_ >= MAX_IMAGE_AREA) {
+    return PARSE_ERROR;  // image final dimension is too large
+  }
   Skip(mem, vp8x_size - VP8X_CHUNK_SIZE);  // skip any trailing data.
   dmux->state_ = WEBP_DEMUX_PARSED_HEADER;
 

src/mux/muxread.c

@@ -218,6 +218,7 @@ static uint8_t* EmitVP8XChunk(uint8_t* const dst, uint32_t width,
   const size_t vp8x_size = CHUNK_HEADER_SIZE + VP8X_CHUNK_SIZE;
   assert(width >= 1 && height >= 1);
   assert(width <= MAX_CANVAS_SIZE && height <= MAX_CANVAS_SIZE);
+  assert(width * (uint64_t)height < MAX_IMAGE_AREA);
   PutLE32(dst, mktag('V', 'P', '8', 'X'));
   PutLE32(dst + TAG_SIZE, VP8X_CHUNK_SIZE);
   PutLE32(dst + CHUNK_HEADER_SIZE, flags);

src/webp/format_constants.h

@@ -74,7 +74,11 @@ typedef enum {
 #define ALPHA_FLAG_BIT     0x10  // Should be same as the ALPHA_FLAG in mux.h
 #define ROTATION_FLAG_BITS 0xe0  // all 3 bits for rotation + symmetry
 
-#define MAX_CANVAS_SIZE    (1 << 24)   // 24 bit max for VP8X width/height.
+#define MAX_CANVAS_SIZE     (1 << 24)    // 24-bit max for VP8X width/height.
+#define MAX_IMAGE_AREA      (1ULL << 32) // 32-bit max for width x height.
+#define MAX_LOOP_COUNT      (1  << 16)   // maximum value for loop-count
+#define MAX_DURATION        (1U << 24)   // maximum duration
+#define MAX_POSITION_OFFSET (1U << 24)   // maximum frame/tile x/y offset
 
 // Maximum chunk payload is such that adding the header and padding won't
 // overflow a uint32_t.