On Sun, Oct 02, 2022 at 07:13:39PM +0300, 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 <michael@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)
if you want the exact limit for any INT_MAX its: (INT_MAX + 1ULL) / 9 * 8 + (INT_MAX + 1ULL) % 9 - 1; though probably a fixed bit value would be better anyway for reproducability
+ 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.
If someone has a set of testfiles for this decoder then I can look at this and cleanup the code more completely. i dont know if a step > C vs overflow check is better but i have the suspicion there are more problems as multiple statments around this seem unreachable. And iam also missing some check that i would expect to be in there I found links to datafilehost.com to test files but they are all expired so i have no test files, so iam a bit cautious with changes ... CCing piotr who linked to the files on datafilehost, maybe he still has them. thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB In fact, the RIAA has been known to suggest that students drop out of college or go to community college in order to be able to afford settlements. -- The RIAA