[FFmpeg-devel] [PATCH] avcodec/jpeg2000dwt: Fix left shift of negative number

[Tomas Härdin]

tis 2022-09-27 klockan 01:11 +0200 skrev Andreas Rheinhardt:
> Fixes the j2k-dwt FATE-test; also fixes #9945.
> (I don't know whether the multiplication can overflow.)

The 5/3 transform is used in lossless mode and therefore shouldn't
overflow for normal use cases. But someone can of course craft a
malicious file

> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at outlook.com>
> ---
>  libavcodec/jpeg2000dwt.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavcodec/jpeg2000dwt.c b/libavcodec/jpeg2000dwt.c
> index f2da7307c4..34e33553f7 100644
> --- a/libavcodec/jpeg2000dwt.c
> +++ b/libavcodec/jpeg2000dwt.c
> @@ -81,7 +81,7 @@ static void sd_1d53(int *p, int i0, int i1)
>  
>      if (i1 <= i0 + 1) {
>          if (i0 == 1)
> -            p[1] <<= 1;
> +            p[1] *= 2;

To trigger an actual overflow here you need enough coefficient bits and
enough decomposition levels, meaning also huge resolution. Resolution
is capped at what 32k x 32k currently? That means you need 17-bit
coefficients at the lowest levels to get over INT_MAX. I'm not actually
sure what the limits for that in jpeg2000 is, but 12-bit lossless would
certaily hit these levels at 5 or more decomp levels. I have samples
that use 6, and it's easy to generate ones that have even more.

To be really safe we'd need to use something like
https://gcc.gnu.org/onlinedocs/gcc/Integer-Overflow-Builtins.html
and maybe define fallbacks for other compilers.

This is part of why I've been harping about formal verification.

/Tomas