| Message ID | 20210419182346.4445-7-michael@niedermayer.cc |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [FFmpeg-devel,1/9] avformat/utils: check dts/duration to be representable before using them | expand |
| Context | Check | Description |
|---|---|---|
| andriy/x86_make | success | Make finished |
| andriy/x86_make_fate | success | Make fate finished |
| andriy/PPC64_make | success | Make finished |
| andriy/PPC64_make_fate | success | Make fate finished |
On 4/19/2021 3:23 PM, Michael Niedermayer wrote: > Fixes: signed integer overflow: 9223372036840103978 + 67637280 cannot be represented in type 'long' > Fixes: 33341/clusterfuzz-testcase-minimized-ffmpeg_dem_DSF_fuzzer-6408154041679872 > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavformat/id3v2.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/libavformat/id3v2.c b/libavformat/id3v2.c > index e0fef08789..0f7035d4c5 100644 > --- a/libavformat/id3v2.c > +++ b/libavformat/id3v2.c > @@ -824,7 +824,7 @@ static void id3v2_parse(AVIOContext *pb, AVDictionary **metadata, > int isv34, unsync; > unsigned tlen; > char tag[5]; > - int64_t next, end = avio_tell(pb) + len; > + int64_t next, end = avio_tell(pb); > int taghdrlen; > const char *reason = NULL; > AVIOContext pb_local; > @@ -836,6 +836,10 @@ static void id3v2_parse(AVIOContext *pb, AVDictionary **metadata, > av_unused int uncompressed_buffer_size = 0; > const char *comm_frame; > > + if (av_sat_add64(end, len) != end + (uint64_t)len) Wouldn't a check like end > INT64_MAX - len be simpler? > + return; > + end += len; > + > av_log(s, AV_LOG_DEBUG, "id3v2 ver:%d flags:%02X len:%d\n", version, flags, len); > > switch (version) { > @@ -1057,7 +1061,7 @@ seek: > > /* Footer preset, always 10 bytes, skip over it */ > if (version == 4 && flags & 0x10) > - end += 10; > + end = av_sat_add64(end, 10); > > error: > if (reason) >
On Thu, Apr 22, 2021 at 07:51:53PM -0300, James Almer wrote: > On 4/19/2021 3:23 PM, Michael Niedermayer wrote: > > Fixes: signed integer overflow: 9223372036840103978 + 67637280 cannot be represented in type 'long' > > Fixes: 33341/clusterfuzz-testcase-minimized-ffmpeg_dem_DSF_fuzzer-6408154041679872 > > > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > --- > > libavformat/id3v2.c | 8 ++++++-- > > 1 file changed, 6 insertions(+), 2 deletions(-) > > > > diff --git a/libavformat/id3v2.c b/libavformat/id3v2.c > > index e0fef08789..0f7035d4c5 100644 > > --- a/libavformat/id3v2.c > > +++ b/libavformat/id3v2.c > > @@ -824,7 +824,7 @@ static void id3v2_parse(AVIOContext *pb, AVDictionary **metadata, > > int isv34, unsync; > > unsigned tlen; > > char tag[5]; > > - int64_t next, end = avio_tell(pb) + len; > > + int64_t next, end = avio_tell(pb); > > int taghdrlen; > > const char *reason = NULL; > > AVIOContext pb_local; > > @@ -836,6 +836,10 @@ static void id3v2_parse(AVIOContext *pb, AVDictionary **metadata, > > av_unused int uncompressed_buffer_size = 0; > > const char *comm_frame; > > + if (av_sat_add64(end, len) != end + (uint64_t)len) > > Wouldn't a check like end > INT64_MAX - len be simpler? will change it so it does not need av_sat_add64() and will apply thx [...]
diff --git a/libavformat/id3v2.c b/libavformat/id3v2.c index e0fef08789..0f7035d4c5 100644 --- a/libavformat/id3v2.c +++ b/libavformat/id3v2.c @@ -824,7 +824,7 @@ static void id3v2_parse(AVIOContext *pb, AVDictionary **metadata, int isv34, unsync; unsigned tlen; char tag[5]; - int64_t next, end = avio_tell(pb) + len; + int64_t next, end = avio_tell(pb); int taghdrlen; const char *reason = NULL; AVIOContext pb_local; @@ -836,6 +836,10 @@ static void id3v2_parse(AVIOContext *pb, AVDictionary **metadata, av_unused int uncompressed_buffer_size = 0; const char *comm_frame; + if (av_sat_add64(end, len) != end + (uint64_t)len) + return; + end += len; + av_log(s, AV_LOG_DEBUG, "id3v2 ver:%d flags:%02X len:%d\n", version, flags, len); switch (version) { @@ -1057,7 +1061,7 @@ seek: /* Footer preset, always 10 bytes, skip over it */ if (version == 4 && flags & 0x10) - end += 10; + end = av_sat_add64(end, 10); error: if (reason)
Fixes: signed integer overflow: 9223372036840103978 + 67637280 cannot be represented in type 'long' Fixes: 33341/clusterfuzz-testcase-minimized-ffmpeg_dem_DSF_fuzzer-6408154041679872 Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavformat/id3v2.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)