[FFmpeg-devel] [PATCH 03/10] avcodec/cbs_av1: Fix potential undefined shift

[Mark Thompson]

On 18/09/2019 04:26, Andreas Rheinhardt wrote:
> 1 << w is undefined as soon as w is >= 31, as 1 has type int. In the
> case of cbs_av1_read_ns, w could potentially even be 32, so one has to
> use a 64bit type.
> 
> (None of the current callers ever use arguments that are so large that
> the above scenario can happen, but this might change in the future.)
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
> ---
>  libavcodec/cbs_av1.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavcodec/cbs_av1.c b/libavcodec/cbs_av1.c
> index 98cd37ef74..0ff6d60ae2 100644
> --- a/libavcodec/cbs_av1.c
> +++ b/libavcodec/cbs_av1.c
> @@ -220,7 +220,7 @@ static int cbs_av1_read_ns(CodedBitstreamContext *ctx, GetBitContext *gbc,
>          position = get_bits_count(gbc);
>  
>      w = av_log2(n) + 1;
> -    m = (1 << w) - n;
> +    m = (1LL << w) - n;
>  
>      if (get_bits_left(gbc) < w) {
>          av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid non-symmetric value at "
> 

Seems like it would be better to just assert that n < 2^25 (or something like that) above instead?  The 64-bit intermediate with 32-bit variables isn't very nice, and you're also going to run into constraints on get_bits() later as well.

Values like this can't be encountered in the current AV1 specification, so the problem is very theoretical in any case.

(Patches 1 and 4 from this set LGTM and applied.)

Thanks,

- Mark