[FFmpeg-devel] [PATCH 2/5] avcodec/atrac9dec: Check conditions before apply_band_extension() to avoid out of array read in initialization of unused variables

[Lynne]

Jun 15, 2019, 11:00 PM by michael at niedermayer.cc:

> Fixes: global-buffer-overflow
> Fixes: 15247/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_ATRAC9_fuzzer-5671602181636096
>
> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
> ---
>  libavcodec/atrac9dec.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/libavcodec/atrac9dec.c b/libavcodec/atrac9dec.c
> index 805d46f3b8..5401d6e19e 100644
> --- a/libavcodec/atrac9dec.c
> +++ b/libavcodec/atrac9dec.c
> @@ -535,9 +535,6 @@ static inline void apply_band_extension(ATRAC9Context *s, ATRAC9BlockData *b,
>  at9_q_unit_to_coeff_idx[g_units[3]],
>  };
>  
> -    if (!b->has_band_ext || !b->has_band_ext_data)
> -        return;
> -
>  for (int ch = 0; ch <= stereo; ch++) {
>  ATRAC9ChannelData *c = &b->channel[ch];
>  
> @@ -741,7 +738,9 @@ static int atrac9_decode_block(ATRAC9Context *s, GetBitContext *gb,
>  
>  apply_intensity_stereo(s, b, stereo);
>  apply_scalefactors    (s, b, stereo);
> -    apply_band_extension  (s, b, stereo);
> +
> +    if (b->has_band_ext && b->has_band_ext_data)
> +        apply_band_extension  (s, b, stereo); 
>

False positive as usual, q_unit_cnt can't be anything out of array since its looked up from
at9_tab_band_q_unit_map.
I'd really appreciate it if you stopped fixing complaint messages from automated tools.
Especially from overflows and fuzzing timeouts. The latter are completely useless and
often make the code look worse and weird, and the former are all useless except when
outside of DSP code (e.g. malloc). And most of our code is DSP.