[FFmpeg-devel] [PATCH 3/3] avcodec/aacpsdsp_template: Fixes integer overflow in ps_add_squares_c()

[Rostislav Pehlivanov]

On 2 July 2017 at 03:28, Michael Niedermayer <michael at niedermayer.cc> wrote:

> Fixes: runtime error: signed integer overflow: 1965219850 + 995792909
> cannot be represented in type 'int'
> Fixes: part of 2096/clusterfuzz-testcase-minimized-4901566068817920
>
> Found-by: continuous fuzzing process https://github.com/google/oss-
> fuzz/tree/master/projects/ffmpeg
> Signed-off-by
> <https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg%0ASigned-off-by>:
> Michael Niedermayer <michael at niedermayer.cc>
> ---
>  libavcodec/aacpsdsp_template.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/libavcodec/aacpsdsp_template.c b/libavcodec/aacpsdsp_
> template.c
> index 9e1a95c1a1..2c0afd4512 100644
> --- a/libavcodec/aacpsdsp_template.c
> +++ b/libavcodec/aacpsdsp_template.c
> @@ -26,9 +26,10 @@
>  #include "libavutil/attributes.h"
>  #include "aacpsdsp.h"
>
> -static void ps_add_squares_c(INTFLOAT *dst, const INTFLOAT (*src)[2], int
> n)
> +static void ps_add_squares_c(INTFLOAT *dst_param, const INTFLOAT
> (*src)[2], int n)
>  {
>      int i;
> +    SUINTFLOAT *dst = dst_param;
>      for (i = 0; i < n; i++)
>          dst[i] += AAC_MADD28(src[i][0], src[i][0], src[i][1], src[i][1]);
>  }
>
>
What's the issue with just _not_ fixing it here? It only occurs on fuzzed
inputs, doesn't crash on any known platform ever, makes the code uglier and
why? Because some fuzzer is super pedantic.
Why not fix the fuzzer? Why not just mark this as a false positive, since
fixing it is pointless from the standpoint of security (you can't exploit
overflows in transforms or functions like this), and all developers hate it.