James Almer: > On 10/2/2022 1:13 PM, Rémi Denis-Courmont wrote: >> 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 <mich...@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)) > > That's __builtin_add_overflow() from gcc/clang. > > There's av_sat_add32(), which will clip the result to fit in a 32-bit > variable. So i guess it could be used and then just check for step == > INT32_MAX and error out, but that's slower than what this patch is doing. >
The result of av_sat_add32() being INT32_MAX/MIN does not imply that the result has been saturated. Apart from that, the documentation of av_sat_add32() pretty much presumes that INT_MIN/MAX == INT32_MIN/MAX. >> 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. > > What 64-bit arch has sizeof(int) == 8? IIRC on Cray computers all integer types that don't have a size of 1 (character types) have size 8. Notice that there are several places in our codebase where we pretty much presume int/unsigned to be 32bits (and to have no padding). - Andreas _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
