| Message ID | 20211223211527.20408-5-michael@niedermayer.cc |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,1/5] avcodec/targa: Check input size for uncompressed TGA before allocation | expand |
| Context | Check | Description |
|---|---|---|
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
| andriy/make_ppc | success | Make finished |
| andriy/make_fate_ppc | fail | Make fate failed |
On Thu, Dec 23, 2021 at 10:15:27PM +0100, 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; > + > if (set_channels(avctx, ast, avio_rb32(pb)) < 0) > return AVERROR_INVALIDDATA; please apply. -- Peter (A907 E02F A6E5 0CD2 34CD 20D2 6760 79C5 AC40 DD6B)
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 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.
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 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 thx [...]
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. And again, my apologies for the confusion.
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 [...]
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; + if (set_channels(avctx, ast, avio_rb32(pb)) < 0) return AVERROR_INVALIDDATA;
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(+)