From e47172875407736fda81ad66c89ea8530abafe7a Mon Sep 17 00:00:00 2001 From: James Zern Date: Mon, 4 Apr 2022 10:54:58 -0700 Subject: [PATCH 1/4] gif2webp: fix segfault on OOM previously calls to WebPPictureCopy() weren't checked, causing a crash when the canvas copies were accessed later in the loop Tested: for i in `seq 1 1125`; do export MALLOC_FAIL_AT=$i ./examples/gif2webp gif_file ./examples/gif2webp -mixed gif_file done Change-Id: I186d13be0ec8f3b72b7d247a8482590c1b1be541 --- examples/gif2webp.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/examples/gif2webp.c b/examples/gif2webp.c index 78d84c62..cc9b25d9 100644 --- a/examples/gif2webp.c +++ b/examples/gif2webp.c @@ -314,8 +314,11 @@ int main(int argc, const char* argv[]) { frame.use_argb = 1; if (!WebPPictureAlloc(&frame)) goto End; GIFClearPic(&frame, NULL); - WebPPictureCopy(&frame, &curr_canvas); - WebPPictureCopy(&frame, &prev_canvas); + if (!(WebPPictureCopy(&frame, &curr_canvas) && + WebPPictureCopy(&frame, &prev_canvas))) { + fprintf(stderr, "Error allocating canvas.\n"); + goto End; + } // Background color. GIFGetBackgroundColor(gif->SColorMap, gif->SBackGroundColor, From ec34fd70239e69326b5d8aa25fa33b761b0db66c Mon Sep 17 00:00:00 2001 From: James Zern Date: Mon, 4 Apr 2022 10:49:42 -0700 Subject: [PATCH 2/4] anim_util: fix leaks on error in ReadAnimatedWebP() & ReadAnimatedGIF() when AllocateFrames() fails due to OOM Tested: for i in `seq 1 278`; do export MALLOC_FAIL_AT=$i ./examples/anim_diff webp1 webp2 ./examples/anim_diff gif1 gif2 done Change-Id: I5d486c2c2982ae088e34a25d8e3e0188df1a9b51 --- examples/anim_util.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/examples/anim_util.c b/examples/anim_util.c index 97b32c07..cf7da4c0 100644 --- a/examples/anim_util.c +++ b/examples/anim_util.c @@ -241,7 +241,7 @@ static int ReadAnimatedWebP(const char filename[], image->bgcolor = anim_info.bgcolor; // Allocate frames. - if (!AllocateFrames(image, anim_info.frame_count)) return 0; + if (!AllocateFrames(image, anim_info.frame_count)) goto End; // Decode frames. while (WebPAnimDecoderHasMoreFrames(dec)) { @@ -558,7 +558,10 @@ static int ReadAnimatedGIF(const char filename[], AnimatedImage* const image, } } // Allocate frames. - AllocateFrames(image, frame_count); + if (!AllocateFrames(image, frame_count)) { + DGifCloseFile(gif, NULL); + return 0; + } canvas_width = image->canvas_width; canvas_height = image->canvas_height; From 2d3293ad7634a8e66725831a4a74b66118fc2d3d Mon Sep 17 00:00:00 2001 From: James Zern Date: Mon, 4 Apr 2022 10:51:13 -0700 Subject: [PATCH 3/4] ExUtilInitCommandLineArguments: fix leak on error argv_data would leak if the argv_ allocation failed or the MAX_ARGC cap was hit Tested: for i in `seq 1 171`; do export MALLOC_FAIL_AT=$i ./examples/img2webp jpeg_file -o /dev/null ./examples/img2webp -mixed jpeg_file -o /dev/null done Change-Id: I39864691e96d5456f324d95a3653bba0f6d6a7be --- examples/example_util.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/examples/example_util.c b/examples/example_util.c index b0436541..fa38d3c2 100644 --- a/examples/example_util.c +++ b/examples/example_util.c @@ -103,7 +103,10 @@ int ExUtilInitCommandLineArguments(int argc, const char* argv[], } args->own_argv_ = 1; args->argv_ = (const char**)WebPMalloc(MAX_ARGC * sizeof(*args->argv_)); - if (args->argv_ == NULL) return 0; + if (args->argv_ == NULL) { + ExUtilDeleteCommandLineArguments(args); + return 0; + } argc = 0; for (cur = strtok((char*)args->argv_data_.bytes, sep); @@ -111,6 +114,7 @@ int ExUtilInitCommandLineArguments(int argc, const char* argv[], cur = strtok(NULL, sep)) { if (argc == MAX_ARGC) { fprintf(stderr, "ERROR: Arguments limit %d reached\n", MAX_ARGC); + ExUtilDeleteCommandLineArguments(args); return 0; } assert(strlen(cur) != 0); From 89f774e61d464285c8d5b20c93002717aebda1ff Mon Sep 17 00:00:00 2001 From: James Zern Date: Mon, 4 Apr 2022 10:58:11 -0700 Subject: [PATCH 4/4] mux{edit,internal}: fix leaks on error several calls to ChunkSetHead() were unchecked, causing the chunk to leak should the call fail due to OOM Tested: for i in `seq 1 1125`; do export MALLOC_FAIL_AT=$i ./examples/gif2webp gif_file ./examples/gif2webp -mixed gif_file done for i in `seq 1 171`; do export MALLOC_FAIL_AT=$i ./examples/img2webp jpeg_file -o /dev/null ./examples/img2webp -mixed jpeg_file -o /dev/null done Change-Id: I479bc487f61b493e5ce033872d353007055c172a --- src/mux/muxedit.c | 1 + src/mux/muxinternal.c | 9 +++++---- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/mux/muxedit.c b/src/mux/muxedit.c index 02c3edec..63e71a0a 100644 --- a/src/mux/muxedit.c +++ b/src/mux/muxedit.c @@ -70,6 +70,7 @@ void WebPMuxDelete(WebPMux* mux) { err = ChunkAssignData(&chunk, data, copy_data, tag); \ if (err == WEBP_MUX_OK) { \ err = ChunkSetHead(&chunk, (LIST)); \ + if (err != WEBP_MUX_OK) ChunkRelease(&chunk); \ } \ return err; \ } diff --git a/src/mux/muxinternal.c b/src/mux/muxinternal.c index b9ee6717..75b6b416 100644 --- a/src/mux/muxinternal.c +++ b/src/mux/muxinternal.c @@ -155,17 +155,18 @@ WebPMuxError ChunkSetHead(WebPChunk* const chunk, WebPMuxError ChunkAppend(WebPChunk* const chunk, WebPChunk*** const chunk_list) { + WebPMuxError err; assert(chunk_list != NULL && *chunk_list != NULL); if (**chunk_list == NULL) { - ChunkSetHead(chunk, *chunk_list); + err = ChunkSetHead(chunk, *chunk_list); } else { WebPChunk* last_chunk = **chunk_list; while (last_chunk->next_ != NULL) last_chunk = last_chunk->next_; - ChunkSetHead(chunk, &last_chunk->next_); - *chunk_list = &last_chunk->next_; + err = ChunkSetHead(chunk, &last_chunk->next_); + if (err == WEBP_MUX_OK) *chunk_list = &last_chunk->next_; } - return WEBP_MUX_OK; + return err; } //------------------------------------------------------------------------------