[FFmpeg-devel] [PATCH] avcodec/aacenc: Check all coefficients for finiteness

[Claudio Freire]

On Wed, Jan 20, 2016 at 11:05 AM, Michael Niedermayer <michaelni at gmx.at> wrote:
> From: Michael Niedermayer <michael at niedermayer.cc>
>
> This is needed as near infinite values on the input side result in only some
> output to be non finite.
> Also it may still be insufficient if subsequent computations overflow
>
> Fixes null pointer dereference
> Fixes: ae66c0f6c12ac1cd5c2c237031240f57/signal_sigsegv_2618c99_9516_6007026f2185a26d7afea895fbed6e38.ogg
>
> Found-by: Mateusz "j00ru" Jurczyk and Gynvael Coldwind
> Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
> ---
>  libavcodec/aacenc.c |   17 ++++++-----------
>  1 file changed, 6 insertions(+), 11 deletions(-)
>
> diff --git a/libavcodec/aacenc.c b/libavcodec/aacenc.c
> index 688b131..0dcf404 100644
> --- a/libavcodec/aacenc.c
> +++ b/libavcodec/aacenc.c
> @@ -544,6 +544,7 @@ static int aac_encode_frame(AVCodecContext *avctx, AVPacket *avpkt,
>          chans    = tag == TYPE_CPE ? 2 : 1;
>          cpe      = &s->cpe[i];
>          for (ch = 0; ch < chans; ch++) {
> +            int k;
>              float clip_avoidance_factor;
>              sce = &cpe->ch[ch];
>              ics = &sce->ics;
> @@ -607,17 +608,11 @@ static int aac_encode_frame(AVCodecContext *avctx, AVPacket *avpkt,
>                  s->mdct1024.mdct_calc(&s->mdct1024, sce->lcoeffs, sce->ret_buf);
>              }
>
> -            if (!(isfinite(cpe->ch[ch].coeffs[    0]) &&
> -                  isfinite(cpe->ch[ch].coeffs[  128]) &&
> -                  isfinite(cpe->ch[ch].coeffs[2*128]) &&
> -                  isfinite(cpe->ch[ch].coeffs[3*128]) &&
> -                  isfinite(cpe->ch[ch].coeffs[4*128]) &&
> -                  isfinite(cpe->ch[ch].coeffs[5*128]) &&
> -                  isfinite(cpe->ch[ch].coeffs[6*128]) &&
> -                  isfinite(cpe->ch[ch].coeffs[7*128]))
> -            ) {
> -                av_log(avctx, AV_LOG_ERROR, "Input contains NaN/+-Inf\n");
> -                return AVERROR(EINVAL);
> +            for (k = 0; k < 1024; k++) {
> +                if (!isfinite(cpe->ch[ch].coeffs[k])) {
> +                    av_log(avctx, AV_LOG_ERROR, "Input contains NaN/+-Inf\n");
> +                    return AVERROR(EINVAL);
> +                }
>              }
>              avoid_clipping(s, sce);
>          }


LGTM.

I was actually wondering whether it would be needed to do exactly this.