cxt / webp

Automatically exported from code.google.com/p/webp
0 stars 0 forks source link

libwebp disallows acceptable buffer sizes #258

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
In CheckDecBuffer() in buffer.c, there is a check that the decode buffer is an 
acceptable size (in RGB mode):

const int stride = abs(buf->stride);
const uint64_t size = (uint64_t)stride * height;
ok &= (size <= buf->size);

In the case that the stride is larger than the width (meaning there is padding 
on the end of the rows), the buffer size may be less than stride * height and 
still be safe (since we don't need the padding on the last row).

I'm not sure if it was intentional to disallow buffers where the last row isn't 
padded.

Also, it's worth noting that there are similar checks in YUVA mode.

Original issue reported on code.google.com by msar...@google.com on 26 Aug 2015 at 7:24

GoogleCodeExporter commented 8 years ago
Seems like a valid remark, thanks!

But i'm not sure we want to encourage not padding the last row: some SSE2 
implementation might want to be able to stomp-write 16 bytes at a time, even on 
the last pixels of the last row without having to special-case this row. But 
that's a borderline concern, maybe. Not sure.

(note one subtlety: what about the case where buf->stride is negative? (flipped 
buffer). The supplied output pointer points at the beginning of the last row.)

Original comment by pascal.m...@gmail.com on 26 Aug 2015 at 7:52

GoogleCodeExporter commented 8 years ago
After some thoughts, it appears it's harmless after all to have a tighter 
check. The internal codec shouldn't access the padding area anyway, this is 
mandatory.

-> patch https://chromium-review.googlesource.com/#/c/298280/ was submitted 
accordingly.

thanks!

Original comment by pascal.m...@gmail.com on 9 Sep 2015 at 8:48

GoogleCodeExporter commented 8 years ago
Thanks for the fix!

Original comment by msar...@google.com on 10 Sep 2015 at 8:46

GoogleCodeExporter commented 8 years ago
note: https://chromium-review.googlesource.com/#/c/299190/ took care of YUV/(A) 
cases too.

Original comment by pascal.m...@gmail.com on 13 Sep 2015 at 8:42