[FFmpeg-devel] [PATCH] lavc/dovi_rpu: Fix UB for possible left shift of negative values

Andreas Rheinhardt andreas.rheinhardt at outlook.com
Thu Jun 16 13:13:37 EEST 2022


Thilo Borgmann:
> diff --git a/libavcodec/dovi_rpu.c b/libavcodec/dovi_rpu.c
> index a87562c8a3..833ce9e705 100644
> --- a/libavcodec/dovi_rpu.c
> +++ b/libavcodec/dovi_rpu.c
> @@ -153,7 +153,7 @@ static inline uint64_t get_ue_coef(GetBitContext *gb, const AVDOVIRpuDataHeader
>      case RPU_COEFF_FIXED:
>          ipart = get_ue_golomb_long(gb);
>          fpart.u32 = get_bits_long(gb, hdr->coef_log2_denom);
> -        return (ipart << hdr->coef_log2_denom) + fpart.u32;
> +        return ipart * (1 << hdr->coef_log2_denom) + fpart.u32;
>  
>      case RPU_COEFF_FLOAT:
>          fpart.u32 = get_bits_long(gb, 32);
> @@ -172,7 +172,7 @@ static inline int64_t get_se_coef(GetBitContext *gb, const AVDOVIRpuDataHeader *
>      case RPU_COEFF_FIXED:
>          ipart = get_se_golomb_long(gb);
>          fpart.u32 = get_bits_long(gb, hdr->coef_log2_denom);
> -        return (ipart << hdr->coef_log2_denom) + fpart.u32;
> +        return ipart * (1 << hdr->coef_log2_denom) + fpart.u32;
>  
>      case RPU_COEFF_FLOAT:
>          fpart.u32 = get_bits_long(gb, 32);

coef_log2_denom can be in the range 13..32. This means that 1 <<
hdr->coef_log2_denom can be UB (namely if coef_log2_denom is 31 or 32
for ordinary 32 bit ints); this time it is not UB that happens to work
as expected, because 1 << 32 will be 0 or 1 (depending upon the system)
and not 2^32. In case of get_ue_coef() this actually adds UB to
otherwise fine code.

- Andreas


More information about the ffmpeg-devel mailing list