Skip to content

Commit

Permalink
muxread: fix reading of buffers > riff size
Browse files Browse the repository at this point in the history
After:
  2c70ad7 muxread,CreateInternal: fix riff size checks (cl/200674839)

`SizeWithPadding()` adds `CHUNK_HEADER_SIZE` (plus additional 1 byte
padding if needed). A later check included `CHUNK_HEADER_SIZE` before
capping the value of the size passed to `WebPMuxCreateInternal()`,
missing cases with a small amount of extra data after the RIFF chunk
(like a newline when the file is opened and saved in a text editor) and
setting size to an incorrect value, so larger sizes would also fail.

Another check of `riff_size < CHUNK_HEADER_SIZE` after the call to
`SizeWithPadding()` is removed because 1) it could not fail given
`SizeWithPadding()` adds `CHUNK_HEADER_SIZE` to the value; and 2) it is
redundant as `size < RIFF_HEADER_SIZE + CHUNK_HEADER_SIZE` is checked
earlier in the function.

Bug: webp:42340561
Change-Id: I58dc4f071b27c2841001b4012aabdb1869f64f97
  • Loading branch information
jzern committed Nov 22, 2024
1 parent 4c85d86 commit 3063351
Showing 1 changed file with 4 additions and 4 deletions.
8 changes: 4 additions & 4 deletions src/mux/muxread.c
Original file line number Diff line number Diff line change
Expand Up @@ -223,11 +223,11 @@ WebPMux* WebPMuxCreateInternal(const WebPData* bitstream, int copy_data,
// Note this padding is historical and differs from demux.c which does not
// pad the file size.
riff_size = SizeWithPadding(riff_size);
if (riff_size < CHUNK_HEADER_SIZE) goto Err;
if (riff_size > size) goto Err;
// There's no point in reading past the end of the RIFF chunk.
if (size > riff_size + CHUNK_HEADER_SIZE) {
size = riff_size + CHUNK_HEADER_SIZE;
// There's no point in reading past the end of the RIFF chunk. Note riff_size
// includes CHUNK_HEADER_SIZE after SizeWithPadding().
if (size > riff_size) {
size = riff_size;
}

end = data + size;
Expand Down

0 comments on commit 3063351

Please sign in to comment.