[FFmpeg-devel] [PATCH 5/5] avformat/mvdec: Check bytes_per_sample

[John-Paul Stewart]

On 2021-12-24 11:58, Michael Niedermayer wrote:
> On Thu, Dec 23, 2021 at 10:32:12PM -0500, John-Paul Stewart wrote:
>> On 2021-12-23 16:15, Michael Niedermayer wrote:
>>> Fixes: division by zero
>>> Fixes: 42814/clusterfuzz-testcase-minimized-ffmpeg_dem_MV_fuzzer-4787014237552640
>>>
>>> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
>>> Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
>>> ---
>>>  libavformat/mvdec.c | 3 +++
>>>  1 file changed, 3 insertions(+)
>>>
>>> diff --git a/libavformat/mvdec.c b/libavformat/mvdec.c
>>> index 1a5012e5076..390f6ba4de8 100644
>>> --- a/libavformat/mvdec.c
>>> +++ b/libavformat/mvdec.c
>>> @@ -366,6 +366,9 @@ static int mv_read_header(AVFormatContext *avctx)
>>>              avpriv_request_sample(avctx, "Audio compression (format %i)", v);
>>>          }
>>>  
>>> +        if (bytes_per_sample <= 0)
>>> +            return AVERROR_INVALIDDATA;
>>> +
>>
>> bytes_per_sample is uint32_t so it can never be less than zero.
>>
>> bytes_per_sample will be zero for movie files with no audio, so that is
>> not necessarily invalid data.

I have to retract that comment.  Sorry for the confusion.

Now that I've had time to delve into it further, the Silicon Graphics
format will fill in a placeholder audio track (16 bit stereo, 22050 Hz)
even when there is no actual audio.  So even movies with no sound will
have bytes_per_sample > 0.

> i can change it to AVERROR_PATCHWELCOME but this codepath has already
> created a audio stream so the code at least belives at this point that
> there is audio and it will crash a few lines later
> 
>>
>> I can't offer a better suggestion at the moment, though.  I'll see if
>> can come up with something, unless one of you guys gets to it first.
> 
> ok, ill apply this unless i see another fix first as it fixes the crash
> and in fact in this path requests for samples where already printed also

The real issue is that the audio stream is always allocated even when
there is no audio.  Fixing that is a project for another day.  Don't let
me stop you from applying the above patch.

And again, my apologies for the confusion.