New issue
Advanced search Search tips
Starred by 1 user

Issue metadata

Status: Fixed
Owner: ----
Closed: Nov 2015

Sign in to add a comment

out-of-bounds pointers

Reported by, Nov 18 2015 Back to list

Issue description

What steps will reproduce the problem?

in two code locations, libwebp will compute an out of bounds pointer (but not dereference it). it is clear that the code does this intentionally, but please keep in mind that this is undefined behavior in C and could lead to problems in practice on a platform where the allocated block is bumped up against the end of the address space, in which case the invalid pointer would wrap around, preventing a safety check from firing.

the first problem is the computation of (br->buf_ + sizeof(lbit_t)) at src/util/bit_reader_inl.h:61, where the summed pointer is up to 8 past the end of the array. you can confirm by adding a printf and then running this command:

  examples/dwebp id:001880,src:001631,op:havoc,rep:16.webp

the other UB is the computation of part_end at src/dec/vp8.c:205. repro by adding a printf and running:

  examples/dwebp id:002329,src:001680,op:havoc,rep:4,+cov.webp

here part_end is up to 8331300 beyond buf_end (and could be worse since psize is 24 bits) which seems like it might be a real problem.

I believe the issue is analogous to the issue discussed here:

and there is some additional good discussion about how not to fix this kind of problem here:

The webp files in question can be found here:

What is the expected output? What do you see instead?

expect pointers to remain in bounds, or to point at most one element past the end of a valid storage block

What version of the product are you using? On what operating system?

libwebp's master branch as of Nov 17 on Ubuntu 14.04

Please provide any additional information below.

Project Member

Comment 1 by, Nov 18 2015

thanks for the very documented report John. Having a look.
Project Member

Comment 2 by, Nov 18 2015

proposed patch:

while the second case (vp8.c:205) wasn't speed-critical and rather straightforward to fix, the first one (bit_reader_inl.h:61) needed a check for potential speed regression.
-> Whereas x86 is ok, i see a 2% speed drop on ARM.

-> needs more investigation.

Since the overflow is not by much, is it possible to just make that buffer 8 bytes bigger than it needs to be? Then you don't need to touch the performance-sensitive code.
There's another file in this batch that makes the 2nd pointer OOB by just about the full 16 MB:


The "buf_end - part_start" fix requires (on 32-bit platforms) that the difference does not exceed 2^31. Is this guaranteed to be the case?
Project Member

Comment 6 by, Nov 19 2015

re: comment #3: unfortunately, the buffer is user-supplied in most cases, so we can't pad it at will

re: comment #5: the 2^31 bound is pretty much ensured by the format and early checks in the code, but i will cross-check. Patch update to follow.
Project Member

Comment 7 by, Nov 21 2015

Status: Fixed
The patch was submitted.
Please don't hesitate to re-open this bug if you spot other risky pointer calculations.


Sign in to add a comment