| Message ID | 20240726210832.288597-1-michael@niedermayer.cc |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,1/3] avformat/mov: Check sample_sizes before using it | expand |
| Context | Check | Description |
|---|---|---|
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
On 7/26/2024 6:08 PM, Michael Niedermayer wrote: > Fixes: NULL pointer dereference > Fixes: 70569/clusterfuzz-testcase-minimized-ffmpeg_dem_MOV_fuzzer-5247918563459072 > > 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 | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/libavformat/mov.c b/libavformat/mov.c > index b74e43e2140..63db7d59a58 100644 > --- a/libavformat/mov.c > +++ b/libavformat/mov.c > @@ -10060,6 +10060,10 @@ static int mov_read_header(AVFormatContext *s) > > st = item->st; > sc = st->priv_data; > + > + if (!sc->sample_sizes || !sc->sample_count) > + return AVERROR_INVALIDDATA; Deja vu. Didn't you send something like this before? Also, can i get the sample? As with other issues, we shouldn't reach this point if these were not allocated. > + > st->codecpar->width = item->width; > st->codecpar->height = item->height; >
On 7/26/2024 7:11 PM, James Almer wrote: > On 7/26/2024 6:08 PM, Michael Niedermayer wrote: >> Fixes: NULL pointer dereference >> Fixes: >> 70569/clusterfuzz-testcase-minimized-ffmpeg_dem_MOV_fuzzer-5247918563459072 >> >> 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 | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/libavformat/mov.c b/libavformat/mov.c >> index b74e43e2140..63db7d59a58 100644 >> --- a/libavformat/mov.c >> +++ b/libavformat/mov.c >> @@ -10060,6 +10060,10 @@ static int mov_read_header(AVFormatContext *s) >> st = item->st; >> sc = st->priv_data; >> + >> + if (!sc->sample_sizes || !sc->sample_count) >> + return AVERROR_INVALIDDATA; > > Deja vu. Didn't you send something like this before? > > Also, can i get the sample? As with other issues, we shouldn't reach No, it was me: https://ffmpeg.org//pipermail/ffmpeg-devel/2024-June/330391.html Still, i want to check the sample because i'm not sure how this code is reached like this. > this point if these were not allocated. > >> + >> st->codecpar->width = item->width; >> st->codecpar->height = item->height;
Hi On Fri, Jul 26, 2024 at 07:24:38PM -0300, James Almer wrote: [...] > > Deja vu. Didn't you send something like this before? > > > > Also, can i get the sample? As with other issues, we shouldn't reach > > No, it was me: > https://ffmpeg.org//pipermail/ffmpeg-devel/2024-June/330391.html Iam surprised we dont have more collisions either way i will drop this on my side > > Still, i want to check the sample because i'm not sure how this code is > reached like this. sure, sent privatly thx [...]
On 7/27/2024 7:06 PM, Michael Niedermayer wrote: > Hi > > On Fri, Jul 26, 2024 at 07:24:38PM -0300, James Almer wrote: > [...] >>> Deja vu. Didn't you send something like this before? >>> >>> Also, can i get the sample? As with other issues, we shouldn't reach >> >> No, it was me: >> https://ffmpeg.org//pipermail/ffmpeg-devel/2024-June/330391.html > > Iam surprised we dont have more collisions There's a stsz atom after the iinf atom that tries to replace sc->sample_sizes. It's inside the same meta box structure as the items instead of inside an stsd structure, which is not spec compliant, so ideally we should stop parsing it if that's the case. I'll push my fix for now, but if such an stsz atom ends up allocating an array with a single entry, it will be accepted, so not exactly ideal. > either way i will drop this on my side > > >> >> Still, i want to check the sample because i'm not sure how this code is >> reached like this. > > sure, sent privatly > > 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 b74e43e2140..63db7d59a58 100644 --- a/libavformat/mov.c +++ b/libavformat/mov.c @@ -10060,6 +10060,10 @@ static int mov_read_header(AVFormatContext *s) st = item->st; sc = st->priv_data; + + if (!sc->sample_sizes || !sc->sample_count) + return AVERROR_INVALIDDATA; + st->codecpar->width = item->width; st->codecpar->height = item->height;
Fixes: NULL pointer dereference Fixes: 70569/clusterfuzz-testcase-minimized-ffmpeg_dem_MOV_fuzzer-5247918563459072 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 | 4 ++++ 1 file changed, 4 insertions(+)