| Message ID | 20201030215206.1629-1-michael@niedermayer.cc |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,1/5] avformat/argo_brp: Check block align before use | expand |
| Context | Check | Description |
|---|---|---|
| andriy/x86_make | success | Make finished |
| andriy/x86_make_fate | success | Make fate finished |
Quoting Michael Niedermayer (2020-10-30 22:52:02) > Fixes: division by 0 > Fixes: 26667/clusterfuzz-testcase-minimized-ffmpeg_dem_ARGO_BRP_fuzzer-5645146928185344.fuzz > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavformat/argo_brp.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavformat/argo_brp.c b/libavformat/argo_brp.c > index 48e0cd6aa4..7c679e944c 100644 > --- a/libavformat/argo_brp.c > +++ b/libavformat/argo_brp.c > @@ -390,7 +390,7 @@ static int argo_brp_read_packet(AVFormatContext *s, AVPacket *pkt) > > blk.size -= ASF_CHUNK_HEADER_SIZE; > > - if (blk.size % st->codecpar->block_align != 0) > + if (st->codecpar->block_align && blk.size % st->codecpar->block_align != 0) Shouldn't block_align==0 also trigger an error?
On 2/11/20 9:40 pm, Anton Khirnov wrote: > > Quoting Michael Niedermayer (2020-10-30 22:52:02) >> Fixes: division by 0 >> Fixes: 26667/clusterfuzz-testcase-minimized-ffmpeg_dem_ARGO_BRP_fuzzer-5645146928185344.fuzz >> >> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg >> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> >> --- >> libavformat/argo_brp.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/libavformat/argo_brp.c b/libavformat/argo_brp.c >> index 48e0cd6aa4..7c679e944c 100644 >> --- a/libavformat/argo_brp.c >> +++ b/libavformat/argo_brp.c >> @@ -390,7 +390,7 @@ static int argo_brp_read_packet(AVFormatContext *s, AVPacket *pkt) >> >> blk.size -= ASF_CHUNK_HEADER_SIZE; >> >> - if (blk.size % st->codecpar->block_align != 0) >> + if (st->codecpar->block_align && blk.size % st->codecpar->block_align != 0) > > Shouldn't block_align==0 also trigger an error? > block_align should never be zero for adocm_argo (is always 17 or 34), so this looks like a validation error in ff_argo_asf_fill_stream(). I'll investigate.
On 2/11/20 9:51 pm, Zane van Iperen wrote: > > > > On 2/11/20 9:40 pm, Anton Khirnov wrote: >> >> Quoting Michael Niedermayer (2020-10-30 22:52:02) >>> Fixes: division by 0 >>> Fixes: 26667/clusterfuzz-testcase-minimized-ffmpeg_dem_ARGO_BRP_fuzzer-5645146928185344.fuzz >>> >>> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg >>> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> >>> --- >>> libavformat/argo_brp.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/libavformat/argo_brp.c b/libavformat/argo_brp.c >>> index 48e0cd6aa4..7c679e944c 100644 >>> --- a/libavformat/argo_brp.c >>> +++ b/libavformat/argo_brp.c >>> @@ -390,7 +390,7 @@ static int argo_brp_read_packet(AVFormatContext *s, AVPacket *pkt) >>> >>> blk.size -= ASF_CHUNK_HEADER_SIZE; >>> >>> - if (blk.size % st->codecpar->block_align != 0) >>> + if (st->codecpar->block_align && blk.size % st->codecpar->block_align != 0) >> >> Shouldn't block_align==0 also trigger an error? >> > > block_align should never be zero for adocm_argo (is always 17 or 34), so > this looks like a validation error in ff_argo_asf_fill_stream(). I'll > investigate. Okay, I'm not sure how this happened. The only place block_align is set is in ff_argo_asf_fill_stream(): st->codecpar->block_align = st->codecpar->channels + (ckhdr->num_samples / 2) * st->codecpar->channels; Earlier in the function, num_samples is forced to be 32, and channels is set like so: if (ckhdr->flags & ASF_CF_STEREO) { st->codecpar->channel_layout = AV_CH_LAYOUT_STEREO; st->codecpar->channels = 2; } else { st->codecpar->channel_layout = AV_CH_LAYOUT_MONO; st->codecpar->channels = 1; } The only possible values for this are 17 and 34...
diff --git a/libavformat/argo_brp.c b/libavformat/argo_brp.c index 48e0cd6aa4..7c679e944c 100644 --- a/libavformat/argo_brp.c +++ b/libavformat/argo_brp.c @@ -390,7 +390,7 @@ static int argo_brp_read_packet(AVFormatContext *s, AVPacket *pkt) blk.size -= ASF_CHUNK_HEADER_SIZE; - if (blk.size % st->codecpar->block_align != 0) + if (st->codecpar->block_align && blk.size % st->codecpar->block_align != 0) return AVERROR_INVALIDDATA; }
Fixes: division by 0 Fixes: 26667/clusterfuzz-testcase-minimized-ffmpeg_dem_ARGO_BRP_fuzzer-5645146928185344.fuzz Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavformat/argo_brp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)