On Sun, May 31, 2020 at 07:07:11PM -0300, James Almer wrote: > On 5/31/2020 6:48 PM, Michael Niedermayer wrote: > > On Sun, May 31, 2020 at 10:58:16AM -0300, James Almer wrote: > >> On 5/31/2020 10:50 AM, Michael Niedermayer wrote: > >>> Fixes: NULL pointer dereference > >>> Fixes: > >>> 22604/clusterfuzz-testcase-minimized-ffmpeg_DEMUXER_fuzzer-5667739074297856.fuzz > >>> > >>> Found-by: continuous fuzzing process > >>> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > >>> Signed-off-by: Michael Niedermayer <mich...@niedermayer.cc> > >>> --- > >>> libavformat/mlvdec.c | 6 +++++- > >>> 1 file changed, 5 insertions(+), 1 deletion(-) > >>> > >>> diff --git a/libavformat/mlvdec.c b/libavformat/mlvdec.c > >>> index dae13cae53..03aed71024 100644 > >>> --- a/libavformat/mlvdec.c > >>> +++ b/libavformat/mlvdec.c > >>> @@ -393,10 +393,14 @@ static int read_packet(AVFormatContext *avctx, > >>> AVPacket *pkt) > >>> { > >>> MlvContext *mlv = avctx->priv_data; > >>> AVIOContext *pb; > >>> - AVStream *st = avctx->streams[mlv->stream_index]; > >>> + AVStream *st; > >>> int index, ret; > >>> unsigned int size, space; > >>> > >>> + if (!avctx->nb_streams) > >>> + return AVERROR_EOF; > >> > >> Shouldn't you abort during read_header() instead if no streams are ever > >> allocated? > > > > read_header() should fail if the haeder is invalid. > > is there something which says that "no streams" is invalid ? > > > > As it is the read_header() code contains a if() which only make > > a difference for a "no stream" case. Which gave me the feeling that > > it wasnt intended to fail in that case > > The checks i'm seeing are all considering video only, audio only, or > both, like the very last check in the function where there's no last > resort else that would cover a no streams case.
if (vst && ast)
A
else if (vst)
B
else if (ast)
C
if one didnt think of "no streams" this would be
if (vst && ast)
A
else if (vst)
B
else
C
if one wanted to reject "no streams" this would be
if (vst && ast)
A
else if (vst)
B
else if (ast)
C
else
FAIL
to me this looked like the intend was to support that
I have no preferrance between any of this, just wanted to not error
out on a path that seemed the author didnt want it to error ...
>
> > but i can add the check for no streams in there where theres already
> > a check for that in read_header ...
>
> Unless there's something in a file that could be signaled without
> streams (All the metadata strings in scan_file() look like they
> definitely apply to some video or audio stream), such a file seems odd
> to me, and if the end result will be just signaling EOF as soon as the
> first attempt at fetching a packet, then might as well just abort in
> read_header() and save the library caller further processing.
>
> Maybe Peter Ross can comment. But in any case, no strong feelings about
> it, so apply whatever you think is best.
ok, ill wait for peter to reply then or in absence of a reply, next time
i remember ill apply something
thanks
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Those who are best at talking, realize last or never when they are wrong.
signature.asc
Description: PGP signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
