On Wed, Mar 23, 2022 at 12:07:25PM +0100, Andreas Rheinhardt wrote: > Michael Niedermayer: > > Fixes: signed integer overflow: 2147483647 + 1 cannot be represented in type 'int' > > Fixes: 45891/clusterfuzz-testcase-minimized-ffmpeg_dem_AIFF_fuzzer-6159183893889024 > > > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > > Signed-off-by: Michael Niedermayer > > --- > > libavformat/aiffdec.c | 7 ++++++- > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > diff --git a/libavformat/aiffdec.c b/libavformat/aiffdec.c > > index 3634bb4960..c2b8e0dede 100644 > > --- a/libavformat/aiffdec.c > > +++ b/libavformat/aiffdec.c > > @@ -72,7 +72,12 @@ static int get_tag(AVIOContext *pb, uint32_t * tag) > > /* Metadata string read */ > > static void get_meta(AVFormatContext *s, const char *key, int size) > > { > > - uint8_t *str = av_malloc(size+1); > > + uint8_t *str; > > + > > + if (size == INT_MAX) > > + return; > > + > > + str = av_malloc(size+1); > > > > if (str) { > > int res = avio_read(s->pb, str, size); > > If a size of INT_MAX is legal, then you can just use "size + 1U" to > avoid the wraparound. (The allocation will then fail with the default > value of max_alloc_size and in this case the avio_skip() will be > executed, so that we don't lose sync (your patch does that).) > Looking at get_tag() shows that the size actually comes from a uint32_t > which gets truncated to INT_MAX if it is not representable in an int. I > don't know whether the specs mandate this behaviour (probably not, so > one loses sync). If one wanted to impose an arbitrary limit, I will use > something much smaller than INT_MAX. The goal of my patch was to fix the undefined behavior. You are very correct that aiffdec has other bugs. ill send a more complete set of fixes thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB There will always be a question for which you do not know the correct answer.