[FFmpeg-devel,1/2] avcodec/aacdec_template: Fix running cleanup in decode_ics_info()

Submitted by Michael Niedermayer on Aug. 21, 2017, 12:15 a.m.

Details

Message ID 20170821001550.10803-1-michael@niedermayer.cc
State New
Headers show

Commit Message

Michael Niedermayer Aug. 21, 2017, 12:15 a.m.
Fixes: out of array read
Fixes: 2873/clusterfuzz-testcase-minimized-5924145713905664

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

Comments

Alex Converse Aug. 21, 2017, 10:23 p.m.
On Sun, Aug 20, 2017 at 5:15 PM, Michael Niedermayer
<michael@niedermayer.cc> wrote:
>
> Fixes: out of array read
> Fixes: 2873/clusterfuzz-testcase-minimized-5924145713905664
>
> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavcodec/aacdec_template.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/libavcodec/aacdec_template.c b/libavcodec/aacdec_template.c
> index a539f74e6f..e7fa27e8db 100644
> --- a/libavcodec/aacdec_template.c
> +++ b/libavcodec/aacdec_template.c
> @@ -1332,7 +1332,7 @@ static int decode_ics_info(AACContext *ac, IndividualChannelStream *ics,
>                  ics->tns_max_bands =  ff_tns_max_bands_512[sampling_index];
>              }
>              if (!ics->num_swb || !ics->swb_offset)
> -                return AVERROR_BUG;
> +                goto fail;
>          } else {
>              ics->swb_offset    =    ff_swb_offset_1024[sampling_index];
>              ics->num_swb       =   ff_aac_num_swb_1024[sampling_index];

okay

> @@ -1356,7 +1356,7 @@ static int decode_ics_info(AACContext *ac, IndividualChannelStream *ics,
>                  if (aot == AOT_ER_AAC_LD) {
>                      av_log(ac->avctx, AV_LOG_ERROR,
>                             "LTP in ER AAC LD not yet implemented.\n");
> -                    return AVERROR_PATCHWELCOME;
> +                    goto fail;
>                  }
>                  if ((ics->ltp.present = get_bits(gb, 1)))
>                      decode_ltp(&ics->ltp, gb, ics->max_sfb);

I'm not sure if it matters to anyone, but this is a missing decoder
feature and returning AVERROR_INVALIDDATA is semantically wrong.

> --
> 2.14.1
Michael Niedermayer Aug. 22, 2017, 6:53 p.m.
On Mon, Aug 21, 2017 at 03:23:09PM -0700, Alex Converse wrote:
> On Sun, Aug 20, 2017 at 5:15 PM, Michael Niedermayer
> <michael@niedermayer.cc> wrote:
> >
> > Fixes: out of array read
> > Fixes: 2873/clusterfuzz-testcase-minimized-5924145713905664
> >
> > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> >  libavcodec/aacdec_template.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/libavcodec/aacdec_template.c b/libavcodec/aacdec_template.c
> > index a539f74e6f..e7fa27e8db 100644
> > --- a/libavcodec/aacdec_template.c
> > +++ b/libavcodec/aacdec_template.c
> > @@ -1332,7 +1332,7 @@ static int decode_ics_info(AACContext *ac, IndividualChannelStream *ics,
> >                  ics->tns_max_bands =  ff_tns_max_bands_512[sampling_index];
> >              }
> >              if (!ics->num_swb || !ics->swb_offset)
> > -                return AVERROR_BUG;
> > +                goto fail;
> >          } else {
> >              ics->swb_offset    =    ff_swb_offset_1024[sampling_index];
> >              ics->num_swb       =   ff_aac_num_swb_1024[sampling_index];
> 
> okay
> 
> > @@ -1356,7 +1356,7 @@ static int decode_ics_info(AACContext *ac, IndividualChannelStream *ics,
> >                  if (aot == AOT_ER_AAC_LD) {
> >                      av_log(ac->avctx, AV_LOG_ERROR,
> >                             "LTP in ER AAC LD not yet implemented.\n");
> > -                    return AVERROR_PATCHWELCOME;
> > +                    goto fail;
> >                  }
> >                  if ((ics->ltp.present = get_bits(gb, 1)))
> >                      decode_ltp(&ics->ltp, gb, ics->max_sfb);
> 
> I'm not sure if it matters to anyone, but this is a missing decoder
> feature and returning AVERROR_INVALIDDATA is semantically wrong.

ill apply with the unchanged error codes

thx

[...]

Patch hide | download patch | download mbox

diff --git a/libavcodec/aacdec_template.c b/libavcodec/aacdec_template.c
index a539f74e6f..e7fa27e8db 100644
--- a/libavcodec/aacdec_template.c
+++ b/libavcodec/aacdec_template.c
@@ -1332,7 +1332,7 @@  static int decode_ics_info(AACContext *ac, IndividualChannelStream *ics,
                 ics->tns_max_bands =  ff_tns_max_bands_512[sampling_index];
             }
             if (!ics->num_swb || !ics->swb_offset)
-                return AVERROR_BUG;
+                goto fail;
         } else {
             ics->swb_offset    =    ff_swb_offset_1024[sampling_index];
             ics->num_swb       =   ff_aac_num_swb_1024[sampling_index];
@@ -1356,7 +1356,7 @@  static int decode_ics_info(AACContext *ac, IndividualChannelStream *ics,
                 if (aot == AOT_ER_AAC_LD) {
                     av_log(ac->avctx, AV_LOG_ERROR,
                            "LTP in ER AAC LD not yet implemented.\n");
-                    return AVERROR_PATCHWELCOME;
+                    goto fail;
                 }
                 if ((ics->ltp.present = get_bits(gb, 1)))
                     decode_ltp(&ics->ltp, gb, ics->max_sfb);