diff mbox

[FFmpeg-devel,2/3] avcodec/agm: Check for too many too short codes in make_new_tree()

Message ID 20190421090519.27335-2-michael@niedermayer.cc
State Accepted
Commit 7ee7bb92e603c35b5467e4106583f5fe7ba9ba55
Headers show

Commit Message

Michael Niedermayer April 21, 2019, 9:05 a.m. UTC 
Fixes: SEGV on unknown address
Fixes: 14198/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_AGM_fuzzer-5723579234123776

untested due to lack of valid samples using this codepath

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

Comments

Paul B Mahol April 21, 2019, 9:36 a.m. UTC | #1 
On 4/21/19, Michael Niedermayer <michael@niedermayer.cc> wrote:
> Fixes: SEGV on unknown address
> Fixes:
> 14198/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_AGM_fuzzer-5723579234123776
>
> untested due to lack of valid samples using this codepath
>
> Found-by: continuous fuzzing process
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavcodec/agm.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/libavcodec/agm.c b/libavcodec/agm.c
> index f3d81bf163..c9d7be5521 100644
> --- a/libavcodec/agm.c
> +++ b/libavcodec/agm.c
> @@ -919,7 +919,7 @@ static void get_tree_codes(uint32_t *codes, Node *nodes,
> int idx, uint32_t pfx,
>      }
>  }
>
> -static void make_new_tree(const uint8_t *bitlens, uint32_t *codes)
> +static int make_new_tree(const uint8_t *bitlens, uint32_t *codes)
>  {
>      int zlcount = 0, curlen, idx, nindex, last, llast;
>      int blcounts[32] = { 0 };
> @@ -959,6 +959,9 @@ static void make_new_tree(const uint8_t *bitlens,
> uint32_t *codes)
>                  int p = node_idx[nindex - 1 + 512];
>                  int ch = syms[256 * curlen + i];
>
> +                if (nindex <= 0)
> +                    return AVERROR_INVALIDDATA;
> +
>                  if (nodes[p].child[0] == -1) {
>                      nodes[p].child[0] = ch;
>                  } else {
> @@ -998,6 +1001,7 @@ static void make_new_tree(const uint8_t *bitlens,
> uint32_t *codes)
>  next:
>
>      get_tree_codes(codes, nodes, 256, 0, 0);
> +    return 0;
>  }
>
>  static int build_huff(const uint8_t *bitlen, VLC *vlc)
> @@ -1008,7 +1012,9 @@ static int build_huff(const uint8_t *bitlen, VLC *vlc)
>      uint32_t codes[256];
>      int nb_codes = 0;
>
> -    make_new_tree(bitlen, new_codes);
> +    int ret = make_new_tree(bitlen, new_codes);
> +    if (ret < 0)
> +        return ret;
>
>      for (int i = 0; i < 256; i++) {
>          if (bitlen[i]) {
> --
> 2.21.0
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

LGTM
Michael Niedermayer April 21, 2019, 9:58 a.m. UTC | #2 
On Sun, Apr 21, 2019 at 11:36:00AM +0200, Paul B Mahol wrote:
> On 4/21/19, Michael Niedermayer <michael@niedermayer.cc> wrote:
> > Fixes: SEGV on unknown address
> > Fixes:
> > 14198/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_AGM_fuzzer-5723579234123776
> >
> > untested due to lack of valid samples using this codepath
> >
> > Found-by: continuous fuzzing process
> > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> >  libavcodec/agm.c | 10 ++++++++--
> >  1 file changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/libavcodec/agm.c b/libavcodec/agm.c
> > index f3d81bf163..c9d7be5521 100644
> > --- a/libavcodec/agm.c
> > +++ b/libavcodec/agm.c
> > @@ -919,7 +919,7 @@ static void get_tree_codes(uint32_t *codes, Node *nodes,
> > int idx, uint32_t pfx,
> >      }
> >  }
> >
> > -static void make_new_tree(const uint8_t *bitlens, uint32_t *codes)
> > +static int make_new_tree(const uint8_t *bitlens, uint32_t *codes)
> >  {
> >      int zlcount = 0, curlen, idx, nindex, last, llast;
> >      int blcounts[32] = { 0 };
> > @@ -959,6 +959,9 @@ static void make_new_tree(const uint8_t *bitlens,
> > uint32_t *codes)
> >                  int p = node_idx[nindex - 1 + 512];
> >                  int ch = syms[256 * curlen + i];
> >
> > +                if (nindex <= 0)
> > +                    return AVERROR_INVALIDDATA;
> > +
> >                  if (nodes[p].child[0] == -1) {
> >                      nodes[p].child[0] = ch;
> >                  } else {
> > @@ -998,6 +1001,7 @@ static void make_new_tree(const uint8_t *bitlens,
> > uint32_t *codes)
> >  next:
> >
> >      get_tree_codes(codes, nodes, 256, 0, 0);
> > +    return 0;
> >  }
> >
> >  static int build_huff(const uint8_t *bitlen, VLC *vlc)
> > @@ -1008,7 +1012,9 @@ static int build_huff(const uint8_t *bitlen, VLC *vlc)
> >      uint32_t codes[256];
> >      int nb_codes = 0;
> >
> > -    make_new_tree(bitlen, new_codes);
> > +    int ret = make_new_tree(bitlen, new_codes);
> > +    if (ret < 0)
> > +        return ret;
> >
> >      for (int i = 0; i < 256; i++) {
> >          if (bitlen[i]) {
> > --
> > 2.21.0
> >
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> > To unsubscribe, visit link above, or email
> > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> 
> LGTM

will apply

thx

[...]
diff mbox

Patch

diff --git a/libavcodec/agm.c b/libavcodec/agm.c
index f3d81bf163..c9d7be5521 100644
--- a/libavcodec/agm.c
+++ b/libavcodec/agm.c
@@ -919,7 +919,7 @@  static void get_tree_codes(uint32_t *codes, Node *nodes, int idx, uint32_t pfx,
     }
 }
 
-static void make_new_tree(const uint8_t *bitlens, uint32_t *codes)
+static int make_new_tree(const uint8_t *bitlens, uint32_t *codes)
 {
     int zlcount = 0, curlen, idx, nindex, last, llast;
     int blcounts[32] = { 0 };
@@ -959,6 +959,9 @@  static void make_new_tree(const uint8_t *bitlens, uint32_t *codes)
                 int p = node_idx[nindex - 1 + 512];
                 int ch = syms[256 * curlen + i];
 
+                if (nindex <= 0)
+                    return AVERROR_INVALIDDATA;
+
                 if (nodes[p].child[0] == -1) {
                     nodes[p].child[0] = ch;
                 } else {
@@ -998,6 +1001,7 @@  static void make_new_tree(const uint8_t *bitlens, uint32_t *codes)
 next:
 
     get_tree_codes(codes, nodes, 256, 0, 0);
+    return 0;
 }
 
 static int build_huff(const uint8_t *bitlen, VLC *vlc)
@@ -1008,7 +1012,9 @@  static int build_huff(const uint8_t *bitlen, VLC *vlc)
     uint32_t codes[256];
     int nb_codes = 0;
 
-    make_new_tree(bitlen, new_codes);
+    int ret = make_new_tree(bitlen, new_codes);
+    if (ret < 0)
+        return ret;
 
     for (int i = 0; i < 256; i++) {
         if (bitlen[i]) {