On 5/16/20, Michael Niedermayer <mich...@niedermayer.cc> wrote: > On Mon, Apr 20, 2020 at 01:03:34AM +0200, Michael Niedermayer wrote: >> On Sun, Apr 19, 2020 at 05:52:01PM +0200, Lynne wrote: >> > Apr 19, 2020, 16:05 by mich...@niedermayer.cc: >> > >> > > Fixes: signed integer overflow: 2147483647 + 1 cannot be represented >> > > in type 'int' >> > > Fixes: >> > > 19950/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_BINKAUDIO_DCT_fuzzer-5765514337189888 >> > > >> > > Found-by: continuous fuzzing process >> > > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg >> > > Signed-off-by: Michael Niedermayer <mich...@niedermayer.cc> >> > > --- >> > > libavcodec/binkaudio.c | 3 +++ >> > > 1 file changed, 3 insertions(+) >> > > >> > > diff --git a/libavcodec/binkaudio.c b/libavcodec/binkaudio.c >> > > index 64a08b8608..2df3dc645a 100644 >> > > --- a/libavcodec/binkaudio.c >> > > +++ b/libavcodec/binkaudio.c >> > > @@ -106,6 +106,9 @@ static av_cold int decode_init(AVCodecContext >> > > *avctx) >> > > avctx->sample_fmt = AV_SAMPLE_FMT_FLTP; >> > > } >> > > >> > > + if (sample_rate >= INT_MAX) >> > > + return AVERROR_INVALIDDATA; >> > > + >> > > s->frame_len = 1 << frame_len_bits; >> > > s->overlap_len = s->frame_len / 16; >> > > s->block_size = (s->frame_len - s->overlap_len) * s->channels; >> > > >> > >> > Did you even bother to look at the checks you added in this decoder >> > previously? >> > Specifically 11 lines above? >> > >> > > if (sample_rate > INT_MAX / avctx->channels) >> > > return AVERROR_INVALIDDATA; >> > > sample_rate *= avctx->channels; >> > >> > To start with the sample rate of the avctx is already checked in >> > utils.c, and you >> > still haven't cleaned up any decoders from the checks made unnecessary >> > by you, >> >> The new check is needed, it overflows without: >> libavcodec/binkaudio.c:116:37: runtime error: signed integer overflow: >> 2147483647 + 1 cannot be represented in type 'int' >> >> The other check is also needed, if i remove it it still overflows >> libavcodec/binkaudio.c:100:22: runtime error: signed integer overflow: >> 1092624416 * 2 cannot be represented in type 'int' >> >> >> > so am reminding you again to clean up the codebase by getting rid of >> > them. >> > At least you might get to clean the codebase for once rather than adding >> > crap like this. >> > >> > So there's only the branch which I quoted that's needed to be fixed, and >> > since there's a >> > check there already, there's no reason to have a check here as well. >> >> I posted exactly this same patch in january, there was a objection, and i >> asked >> what was preferred instead, and pinged it multiple times over the >> following >> months, in february and april: >> >> 189192 N F 0114 15:36 To FFmpeg devel (1,6K) [FFmpeg-devel] [PATCH] >> avcodec/binkaudio: Check sample_rate to avoid integer overflow >> 189193 r 0114 16:04 Paul B Mahol (2,1K) └─> >> 189194 sF 0201 16:14 To FFmpeg devel (1,7K) └─> >> 189195 r 0201 16:17 Paul B Mahol (1,3K) └─> >> 189196 rsF 0201 23:48 To FFmpeg devel (2,6K) └─> >> 189197 rs 0209 21:28 Michael Niederm (2,9K) └─> >> 189198 rsF 0404 23:38 To FFmpeg devel (2,8K) └─> >> 189199 sF 0407 23:17 To FFmpeg devel (3,1K) └─> >> >> but i failed to get a reply, so i tried reposting it >> >> So, if the patch is still not liked, can you explain what do you >> prefer ? >> >> i can put the sample_rate >= INT_MAX check in generic code but it >> is specific to this decoder it wont help anything else >> >> i can put a more aggressive check like sample_rate * channels > >> MAX_CHANNEL_SAMPLES >> in generic code, that will of course block some otherwise supported cases >> >> or we can have checks just for what is not supported >> >> or we can extend the code to support a wider range where it is possible >> > >> Just say what you prefer, i dont mind at all what it is, i just want the >> issue fixed its already so many months open ... > > ping > id like to fix ossfuzz issue 19950
Use (sample_rate + 1LL) / 2 ? > > thx > > [...] > > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > What does censorship reveal? It reveals fear. -- Julian Assange > _______________________________________________ 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".
