On Sat, May 16, 2020 at 07:12:28PM +0200, Paul B Mahol wrote: > 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 ?
will apply an adjusted patch Thanks! [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Concerning the gods, I have no means of knowing whether they exist or not or of what sort they may be, because of the obscurity of the subject, and the brevity of human life -- Protagoras
signature.asc
Description: PGP signature
_______________________________________________ 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".
