From 306335198d2419988223cac98f7c99c6975204c2 Mon Sep 17 00:00:00 2001 From: James Zern Date: Thu, 21 Nov 2024 18:18:10 -0800 Subject: [PATCH] muxread: fix reading of buffers > riff size After: 2c70ad76 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 --- src/mux/muxread.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/mux/muxread.c b/src/mux/muxread.c index afd3542e..ddb0cd78 100644 --- a/src/mux/muxread.c +++ b/src/mux/muxread.c @@ -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;