| Message ID | 20240716131929.3708881-1-michael@niedermayer.cc |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] avformat/mov: sanity check count in IPRP | expand |
| Context | Check | Description |
|---|---|---|
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
On 7/16/2024 10:19 AM, Michael Niedermayer wrote: > Fixes: Timeout > Fixes: 69230/clusterfuzz-testcase-minimized-ffmpeg_IO_DEMUXER_fuzzer-6540512101203968 > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavformat/mov.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/libavformat/mov.c b/libavformat/mov.c > index ce95842ce58..9042753d221 100644 > --- a/libavformat/mov.c > +++ b/libavformat/mov.c > @@ -8925,6 +8925,11 @@ static int mov_read_iprp(MOVContext *c, AVIOContext *pb, MOVAtom atom) > flags = avio_rb24(pb); > count = avio_rb32(pb); > > + if (count * 5LL > a.size) { > + ret = AVERROR_INVALIDDATA; > + goto fail; > + } a.size is also read from the aviocontext, so i think it'd be better to add an avio_feof() check inside the for loop below, after assoc_count is read. > + > for (int i = 0; i < count; i++) { > int item_id = version ? avio_rb32(pb) : avio_rb16(pb); > int assoc_count = avio_r8(pb);
On Tue, Jul 16, 2024 at 10:31:54AM -0300, James Almer wrote: > On 7/16/2024 10:19 AM, Michael Niedermayer wrote: > > Fixes: Timeout > > Fixes: 69230/clusterfuzz-testcase-minimized-ffmpeg_IO_DEMUXER_fuzzer-6540512101203968 > > > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > --- > > libavformat/mov.c | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > diff --git a/libavformat/mov.c b/libavformat/mov.c > > index ce95842ce58..9042753d221 100644 > > --- a/libavformat/mov.c > > +++ b/libavformat/mov.c > > @@ -8925,6 +8925,11 @@ static int mov_read_iprp(MOVContext *c, AVIOContext *pb, MOVAtom atom) > > flags = avio_rb24(pb); > > count = avio_rb32(pb); > > + if (count * 5LL > a.size) { > > + ret = AVERROR_INVALIDDATA; > > + goto fail; > > + } > > a.size is also read from the aviocontext, so i think it'd be better to add > an avio_feof() check inside the for loop below, after assoc_count is read. ok will apply with that solution thx [...]
diff --git a/libavformat/mov.c b/libavformat/mov.c index ce95842ce58..9042753d221 100644 --- a/libavformat/mov.c +++ b/libavformat/mov.c @@ -8925,6 +8925,11 @@ static int mov_read_iprp(MOVContext *c, AVIOContext *pb, MOVAtom atom) flags = avio_rb24(pb); count = avio_rb32(pb); + if (count * 5LL > a.size) { + ret = AVERROR_INVALIDDATA; + goto fail; + } + for (int i = 0; i < count; i++) { int item_id = version ? avio_rb32(pb) : avio_rb16(pb); int assoc_count = avio_r8(pb);
Fixes: Timeout Fixes: 69230/clusterfuzz-testcase-minimized-ffmpeg_IO_DEMUXER_fuzzer-6540512101203968 Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavformat/mov.c | 5 +++++ 1 file changed, 5 insertions(+)