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

[Michael Niedermayer <michael@niedermayer.cc>]

[-- Attachment #1.1: Type: text/plain, Size: 2680 bytes --]

On Fri, Dec 24, 2021 at 03:29:50PM -0500, John-Paul Stewart wrote:
> 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@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.

ok will apply

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Breaking DRM is a little like attempting to break through a door even
though the window is wide open and the only thing in the house is a bunch
of things you dont want and which you would get tomorrow for free anyway

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".