diff mbox series

[FFmpeg-devel,1/5] avformat/argo_brp: Check block align before use

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
Related show

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished

Commit Message

Michael Niedermayer Oct. 30, 2020, 9:52 p.m. UTC
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(-)

Comments

Anton Khirnov Nov. 2, 2020, 11:40 a.m. UTC | #1
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?
Zane van Iperen Nov. 2, 2020, 11:51 a.m. UTC | #2
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.
Zane van Iperen Nov. 2, 2020, 12:22 p.m. UTC | #3
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 mbox series

Patch

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;
     }