[FFmpeg-devel] [PATCH] avcodec/bonk: Check step
Rémi Denis-Courmont
remi at remlab.net
Sun Oct 2 19:13:39 EEST 2022
Le sunnuntaina 2. lokakuuta 2022, 18.43.23 EEST Michael Niedermayer a écrit :
> Fixes: signed integer overflow: 2040812214 + 255101526 cannot be represented
> in type 'int' Fixes:
> 51323/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_BONK_fuzzer-4791481
> 067503616
>
> 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/bonk.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/libavcodec/bonk.c b/libavcodec/bonk.c
> index 409694f710d..32f7c9b9bdb 100644
> --- a/libavcodec/bonk.c
> +++ b/libavcodec/bonk.c
> @@ -187,6 +187,9 @@ static int intlist_read(BonkContext *s, int *buf, int
> entries, int base_2_part) if (!dominant)
> n_zeros += steplet;
>
> + if (step > INT_MAX/9*8)
> + return AVERROR_INVALIDDATA;
> +
> step += step / 8;
> } else if (steplet > 0) {
> int actual_run = read_uint_max(s, steplet - 1);
No problem with this patch *specifically* but wouldn't it be more effective to
fix that sort of issue with checked arithmetic, e.g. something like:
if (av_ckd_add(&step, step, step / 8))
return AVERROR_INVALIDDATA;
...especially on 64-bit systems whence this is really just an add. This also
avoids having to figure out what the exact boundary value is.
--
レミ・デニ-クールモン
http://www.remlab.net/
More information about the ffmpeg-devel
mailing list