| Message ID | 20240718221606.3710874-1-michael@niedermayer.cc |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,1/2] avformat/mov: Check extradata in mov_read_iacb() | expand |
On 7/18/2024 7:16 PM, Michael Niedermayer wrote: > Fixes: MemLeak > Fixes: 69853/clusterfuzz-testcase-minimized-ffmpeg_dem_MOV_fuzzer-4660448545275904 > > 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 | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/libavformat/mov.c b/libavformat/mov.c > index ce95842ce58..82fce7ef5c1 100644 > --- a/libavformat/mov.c > +++ b/libavformat/mov.c > @@ -897,6 +897,8 @@ static int mov_read_iacb(MOVContext *c, AVIOContext *pb, MOVAtom atom) > > st = c->fc->streams[c->fc->nb_streams - 1]; > sc = st->priv_data; > + if (st->codecpar->extradata) > + return AVERROR_INVALIDDATA; Maybe it's better to do like other atoms where we ignore duplicate entries (See mov_read_glbl(), used for h264/hevc/etc). > > sc->iamf = av_mallocz(sizeof(*sc->iamf)); > if (!sc->iamf)
On Thu, Jul 18, 2024 at 07:19:17PM -0300, James Almer wrote: > On 7/18/2024 7:16 PM, Michael Niedermayer wrote: > > Fixes: MemLeak > > Fixes: 69853/clusterfuzz-testcase-minimized-ffmpeg_dem_MOV_fuzzer-4660448545275904 > > > > 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 | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/libavformat/mov.c b/libavformat/mov.c > > index ce95842ce58..82fce7ef5c1 100644 > > --- a/libavformat/mov.c > > +++ b/libavformat/mov.c > > @@ -897,6 +897,8 @@ static int mov_read_iacb(MOVContext *c, AVIOContext *pb, MOVAtom atom) > > st = c->fc->streams[c->fc->nb_streams - 1]; > > sc = st->priv_data; > > + if (st->codecpar->extradata) > > + return AVERROR_INVALIDDATA; > > Maybe it's better to do like other atoms where we ignore duplicate entries > (See mov_read_glbl(), used for h264/hevc/etc). IIRC its a mix of mov_read_iacb() and mov_read_stsd() both setting extradata i can certainly do a "return 0" with some warning if you prefer that thx [...]
On 7/19/2024 5:10 AM, Michael Niedermayer wrote: > On Thu, Jul 18, 2024 at 07:19:17PM -0300, James Almer wrote: >> On 7/18/2024 7:16 PM, Michael Niedermayer wrote: >>> Fixes: MemLeak >>> Fixes: 69853/clusterfuzz-testcase-minimized-ffmpeg_dem_MOV_fuzzer-4660448545275904 >>> >>> 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 | 2 ++ >>> 1 file changed, 2 insertions(+) >>> >>> diff --git a/libavformat/mov.c b/libavformat/mov.c >>> index ce95842ce58..82fce7ef5c1 100644 >>> --- a/libavformat/mov.c >>> +++ b/libavformat/mov.c >>> @@ -897,6 +897,8 @@ static int mov_read_iacb(MOVContext *c, AVIOContext *pb, MOVAtom atom) >>> st = c->fc->streams[c->fc->nb_streams - 1]; >>> sc = st->priv_data; >>> + if (st->codecpar->extradata) >>> + return AVERROR_INVALIDDATA; >> >> Maybe it's better to do like other atoms where we ignore duplicate entries >> (See mov_read_glbl(), used for h264/hevc/etc). > > IIRC its a mix of mov_read_iacb() and mov_read_stsd() both setting extradata iacb is a child box of stsd. In a sane file only one box will set extradata, so lets copy the behavior of glbl. > > i can certainly do a "return 0" with some warning if you prefer that Yes. > > thx > > [...] > > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
diff --git a/libavformat/mov.c b/libavformat/mov.c index ce95842ce58..82fce7ef5c1 100644 --- a/libavformat/mov.c +++ b/libavformat/mov.c @@ -897,6 +897,8 @@ static int mov_read_iacb(MOVContext *c, AVIOContext *pb, MOVAtom atom) st = c->fc->streams[c->fc->nb_streams - 1]; sc = st->priv_data; + if (st->codecpar->extradata) + return AVERROR_INVALIDDATA; sc->iamf = av_mallocz(sizeof(*sc->iamf)); if (!sc->iamf)
Fixes: MemLeak Fixes: 69853/clusterfuzz-testcase-minimized-ffmpeg_dem_MOV_fuzzer-4660448545275904 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 | 2 ++ 1 file changed, 2 insertions(+)