diff mbox

[FFmpeg-devel] avcodec/truemotion2: Check huffman code max bits

Message ID 20181119224713.8482-1-michael@niedermayer.cc
State Accepted
Commit 77bf85515e59f7b17685fbbec943ff46f6217719
Headers show

Commit Message

Michael Niedermayer Nov. 19, 2018, 10:47 p.m. UTC 
Fixes: Timeout
Fixes: 10984/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_TRUEMOTION2_fuzzer-6643310750859264

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

Comments

Tomas Härdin Nov. 20, 2018, 11:09 a.m. UTC | #1 
mån 2018-11-19 klockan 23:47 +0100 skrev Michael Niedermayer:
> Fixes: Timeout
> Fixes: 10984/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_TRUEMOTION2_fuzzer-6643310750859264
> 
> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavcodec/truemotion2.c | 19 ++++++++++++++-----
>  1 file changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/libavcodec/truemotion2.c b/libavcodec/truemotion2.c
> index 58a577f53c..6d58483a77 100644
> --- a/libavcodec/truemotion2.c
> +++ b/libavcodec/truemotion2.c
> @@ -112,9 +112,13 @@ typedef struct TM2Huff {
>      int *lens; ///< codelengths
>  } TM2Huff;
>  
> +/**
> + *
> + * @returns the length of the longest code or an AVERROR code
> + */
>  static int tm2_read_tree(TM2Context *ctx, uint32_t prefix, int length, TM2Huff *huff)
>  {
> -    int ret;
> +    int ret, ret2;
>      if (length > huff->max_bits) {
>          av_log(ctx->avctx, AV_LOG_ERROR, "Tree exceeded its given depth (%i)\n",
>                 huff->max_bits);
> @@ -133,14 +137,14 @@ static int tm2_read_tree(TM2Context *ctx, uint32_t prefix, int length, TM2Huff *
>          huff->bits[huff->num] = prefix;
>          huff->lens[huff->num] = length;
>          huff->num++;
> -        return 0;
> +        return length;
>      } else { /* non-terminal node */
> -        if ((ret = tm2_read_tree(ctx, prefix << 1, length + 1, huff)) < 0)
> -            return ret;
> +        if ((ret2 = tm2_read_tree(ctx, prefix << 1, length + 1, huff)) < 0)
> +            return ret2;
>          if ((ret = tm2_read_tree(ctx, (prefix << 1) | 1, length + 1, huff)) < 0)
>              return ret;
>      }
> -    return 0;
> +    return FFMAX(ret, ret2);
>  }
>  
>  static int tm2_build_huff_table(TM2Context *ctx, TM2Codes *code)
> @@ -183,6 +187,11 @@ static int tm2_build_huff_table(TM2Context *ctx, TM2Codes *code)
>  
>      res = tm2_read_tree(ctx, 0, 0, &huff);
>  
> +    if (res >= 0 && res != huff.max_bits) {
> +        av_log(ctx->avctx, AV_LOG_ERROR, "Got less bits than expected: %i of %i\n",
> +               res, huff.max_bits);
> +        res = AVERROR_INVALIDDATA;
> +    }

Looks OK

/Tomas
Michael Niedermayer Nov. 20, 2018, 10:27 p.m. UTC | #2 
On Tue, Nov 20, 2018 at 12:09:50PM +0100, Tomas Härdin wrote:
> mån 2018-11-19 klockan 23:47 +0100 skrev Michael Niedermayer:
> > Fixes: Timeout
> > Fixes: 10984/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_TRUEMOTION2_fuzzer-6643310750859264
> > 
> > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> >  libavcodec/truemotion2.c | 19 ++++++++++++++-----
> >  1 file changed, 14 insertions(+), 5 deletions(-)
> > 
> > diff --git a/libavcodec/truemotion2.c b/libavcodec/truemotion2.c
> > index 58a577f53c..6d58483a77 100644
> > --- a/libavcodec/truemotion2.c
> > +++ b/libavcodec/truemotion2.c
> > @@ -112,9 +112,13 @@ typedef struct TM2Huff {
> >      int *lens; ///< codelengths
> >  } TM2Huff;
> >  
> > +/**
> > + *
> > + * @returns the length of the longest code or an AVERROR code
> > + */
> >  static int tm2_read_tree(TM2Context *ctx, uint32_t prefix, int length, TM2Huff *huff)
> >  {
> > -    int ret;
> > +    int ret, ret2;
> >      if (length > huff->max_bits) {
> >          av_log(ctx->avctx, AV_LOG_ERROR, "Tree exceeded its given depth (%i)\n",
> >                 huff->max_bits);
> > @@ -133,14 +137,14 @@ static int tm2_read_tree(TM2Context *ctx, uint32_t prefix, int length, TM2Huff *
> >          huff->bits[huff->num] = prefix;
> >          huff->lens[huff->num] = length;
> >          huff->num++;
> > -        return 0;
> > +        return length;
> >      } else { /* non-terminal node */
> > -        if ((ret = tm2_read_tree(ctx, prefix << 1, length + 1, huff)) < 0)
> > -            return ret;
> > +        if ((ret2 = tm2_read_tree(ctx, prefix << 1, length + 1, huff)) < 0)
> > +            return ret2;
> >          if ((ret = tm2_read_tree(ctx, (prefix << 1) | 1, length + 1, huff)) < 0)
> >              return ret;
> >      }
> > -    return 0;
> > +    return FFMAX(ret, ret2);
> >  }
> >  
> >  static int tm2_build_huff_table(TM2Context *ctx, TM2Codes *code)
> > @@ -183,6 +187,11 @@ static int tm2_build_huff_table(TM2Context *ctx, TM2Codes *code)
> >  
> >      res = tm2_read_tree(ctx, 0, 0, &huff);
> >  
> > +    if (res >= 0 && res != huff.max_bits) {
> > +        av_log(ctx->avctx, AV_LOG_ERROR, "Got less bits than expected: %i of %i\n",
> > +               res, huff.max_bits);
> > +        res = AVERROR_INVALIDDATA;
> > +    }
> 
> Looks OK

will apply

thx

[...]
diff mbox

Patch

diff --git a/libavcodec/truemotion2.c b/libavcodec/truemotion2.c
index 58a577f53c..6d58483a77 100644
--- a/libavcodec/truemotion2.c
+++ b/libavcodec/truemotion2.c
@@ -112,9 +112,13 @@  typedef struct TM2Huff {
     int *lens; ///< codelengths
 } TM2Huff;
 
+/**
+ *
+ * @returns the length of the longest code or an AVERROR code
+ */
 static int tm2_read_tree(TM2Context *ctx, uint32_t prefix, int length, TM2Huff *huff)
 {
-    int ret;
+    int ret, ret2;
     if (length > huff->max_bits) {
         av_log(ctx->avctx, AV_LOG_ERROR, "Tree exceeded its given depth (%i)\n",
                huff->max_bits);
@@ -133,14 +137,14 @@  static int tm2_read_tree(TM2Context *ctx, uint32_t prefix, int length, TM2Huff *
         huff->bits[huff->num] = prefix;
         huff->lens[huff->num] = length;
         huff->num++;
-        return 0;
+        return length;
     } else { /* non-terminal node */
-        if ((ret = tm2_read_tree(ctx, prefix << 1, length + 1, huff)) < 0)
-            return ret;
+        if ((ret2 = tm2_read_tree(ctx, prefix << 1, length + 1, huff)) < 0)
+            return ret2;
         if ((ret = tm2_read_tree(ctx, (prefix << 1) | 1, length + 1, huff)) < 0)
             return ret;
     }
-    return 0;
+    return FFMAX(ret, ret2);
 }
 
 static int tm2_build_huff_table(TM2Context *ctx, TM2Codes *code)
@@ -183,6 +187,11 @@  static int tm2_build_huff_table(TM2Context *ctx, TM2Codes *code)
 
     res = tm2_read_tree(ctx, 0, 0, &huff);
 
+    if (res >= 0 && res != huff.max_bits) {
+        av_log(ctx->avctx, AV_LOG_ERROR, "Got less bits than expected: %i of %i\n",
+               res, huff.max_bits);
+        res = AVERROR_INVALIDDATA;
+    }
     if (huff.num != huff.max_num) {
         av_log(ctx->avctx, AV_LOG_ERROR, "Got less codes than expected: %i of %i\n",
                huff.num, huff.max_num);