libwebp: validate chunk size in ParseOptionalChunks

libwebp: validate chunk size in ParseOptionalChunks

the max wasn't checked leading to a rollover case, possibly exploitable.
additionally check the RIFF size early, to avoid similar issues.

BUG=157079
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=163677

[fgalligan1, 2012-10-23 02:06:21]

lgtm

[commit-bot: I haz the power, 2012-10-23 04:55:18]

No LGTM from a valid reviewer yet. Only full committers are accepted.
Even if an LGTM may have been provided, it was from a non-committer or
a lowly provisional committer, _not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.

[jzern, 2012-10-23 05:10:25]

This is ready to land, just need a committer lgtm...

[pascal.massimino, 2012-10-23 08:24:33]

lgtm

lgtm

[fbarchard, 2012-10-23 19:05:04]

lgtm

http://codereview.chromium.org/11229048/diff/1/third_party/libwebp/dec/webp.c
File third_party/libwebp/dec/webp.c (right):

http://codereview.chromium.org/11229048/diff/1/third_party/libwebp/dec/webp.c...
third_party/libwebp/dec/webp.c:184: return VP8_STATUS_BITSTREAM_ERROR;         
// Not a valid chunk size.
nit Should be 2 spaces before comment.
return VP8_STATUS_BITSTREAM_ERROR;  // Not a valid chunk size.

[jzern, 2012-10-23 19:23:22]

http://codereview.chromium.org/11229048/diff/1/third_party/libwebp/dec/webp.c
File third_party/libwebp/dec/webp.c (right):

http://codereview.chromium.org/11229048/diff/1/third_party/libwebp/dec/webp.c...
third_party/libwebp/dec/webp.c:184: return VP8_STATUS_BITSTREAM_ERROR;         
// Not a valid chunk size.
On 2012/10/23 19:05:05, fbarchard wrote:
> nit Should be 2 spaces before comment.
> return VP8_STATUS_BITSTREAM_ERROR;  // Not a valid chunk size.

This is consistent to the remainder of the function. any cosmetics can be done
upstream...

[commit-bot: I haz the power, 2012-10-23 19:23:46]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jzern@chromium.org/11229048/1

[kenrb, 2012-10-23 20:25:25]

On 2012/10/23 05:10:25, jzern wrote:
> This is ready to land, just need a committer lgtm...

No objection to what is there, but should ParseVP8Header() have a similar check
on the size value it reads?

[jzern, 2012-10-23 20:39:19]

On 2012/10/23 20:25:25, kenrb wrote:
> On 2012/10/23 05:10:25, jzern wrote:
> > This is ready to land, just need a committer lgtm...
> 
> No objection to what is there, but should ParseVP8Header() have a similar
check
> on the size value it reads?

It's size check uses the riff size, so will pick up this limit.

[jzern, 2012-10-23 20:39:30]

On 2012/10/23 20:39:19, jzern wrote:
> On 2012/10/23 20:25:25, kenrb wrote:
> > On 2012/10/23 05:10:25, jzern wrote:
> > > This is ready to land, just need a committer lgtm...
> > 
> > No objection to what is there, but should ParseVP8Header() have a similar
> check
> > on the size value it reads?
> 
> It's size check uses the riff size, so will pick up this limit.

*Its

[commit-bot: I haz the power, 2012-10-23 21:17:28]

Change committed as 163677