diff mbox series

[FFmpeg-devel,1/2] avcodec/hapdec: Check tex_size more strictly and before using it

Message ID 20200328211246.30591-1-michael@niedermayer.cc
State Superseded
Headers show
Series [FFmpeg-devel,1/2] avcodec/hapdec: Check tex_size more strictly and before using it
Related show

Checks

Context Check Description
andriy/ffmpeg-patchwork pending
andriy/ffmpeg-patchwork success Applied patch
andriy/ffmpeg-patchwork success Configure finished
andriy/ffmpeg-patchwork warning New warnings during build
andriy/ffmpeg-patchwork success Make finished
andriy/ffmpeg-patchwork success Make fate finished

Commit Message

Michael Niedermayer March 28, 2020, 9:12 p.m. UTC
Fixes: OOM
Fixes: 20774/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_HAP_fuzzer-5678608951803904
Fixes: 20956/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_HAP_fuzzer-5713643025203200

Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavcodec/hapdec.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

Comments

Moritz Barsnick March 31, 2020, 3:45 p.m. UTC | #1
On Sat, Mar 28, 2020 at 22:12:45 +0100, Michael Niedermayer wrote:
> +        if (ctx->tex_size != (avctx->coded_width  / TEXTURE_BLOCK_W)
> +            *(avctx->coded_height / TEXTURE_BLOCK_H)
> +            *tex_rat[t]) {

It would be easier to recognize that this is a multiplication and not a
dereference, if the '*' wasn't attached to the right hand term.

But I do realize you only moved the block of code.

Moritz
Michael Niedermayer March 31, 2020, 6:58 p.m. UTC | #2
On Tue, Mar 31, 2020 at 05:45:39PM +0200, Moritz Barsnick wrote:
> On Sat, Mar 28, 2020 at 22:12:45 +0100, Michael Niedermayer wrote:
> > +        if (ctx->tex_size != (avctx->coded_width  / TEXTURE_BLOCK_W)
> > +            *(avctx->coded_height / TEXTURE_BLOCK_H)
> > +            *tex_rat[t]) {
> 
> It would be easier to recognize that this is a multiplication and not a
> dereference, if the '*' wasn't attached to the right hand term.
> 
> But I do realize you only moved the block of code.

ill fix this together with another bug i found in this when i apply or
post a new version of this

thx

[...]
diff mbox series

Patch

diff --git a/libavcodec/hapdec.c b/libavcodec/hapdec.c
index d23ceb5cef..e4abd739c5 100644
--- a/libavcodec/hapdec.c
+++ b/libavcodec/hapdec.c
@@ -342,6 +342,13 @@  static int hap_decode(AVCodecContext *avctx, void *data,
         if (ret < 0)
             return ret;
 
+        if (ctx->tex_size != (avctx->coded_width  / TEXTURE_BLOCK_W)
+            *(avctx->coded_height / TEXTURE_BLOCK_H)
+            *tex_rat[t]) {
+            av_log(avctx, AV_LOG_ERROR, "uncompressed size mismatches\n");
+            return AVERROR_INVALIDDATA;
+        }
+
         start_texture_section += ctx->texture_section_size + 4;
 
         if (avctx->codec->update_thread_context)
@@ -370,13 +377,6 @@  static int hap_decode(AVCodecContext *avctx, void *data,
             tex_size = ctx->tex_size;
         }
 
-        if (tex_size < (avctx->coded_width  / TEXTURE_BLOCK_W)
-            *(avctx->coded_height / TEXTURE_BLOCK_H)
-            *tex_rat[t]) {
-            av_log(avctx, AV_LOG_ERROR, "Insufficient data\n");
-            return AVERROR_INVALIDDATA;
-        }
-
         /* Use the decompress function on the texture, one block per thread */
         if (t == 0){
             avctx->execute2(avctx, decompress_texture_thread, tframe.f, NULL, ctx->slice_count);