[FFmpeg-devel] [PATCH 1/3] avformat/matroskadec: Reset state also on failure in matroska_reset_status()
Michael Niedermayer
michael at niedermayer.cc
Tue Sep 14 19:13:43 EEST 2021
On Mon, Sep 13, 2021 at 10:57:41PM +0200, Andreas Rheinhardt wrote:
> Michael Niedermayer:
> > On Mon, Jul 19, 2021 at 11:12:11PM +0200, Andreas Rheinhardt wrote:
> >> Michael Niedermayer:
> >>> On Fri, Jul 02, 2021 at 06:17:58PM +0200, Andreas Rheinhardt wrote:
> >>>> Michael Niedermayer:
> >>>>> The calling code does not handle failures and will fail with assertion failures later.
> >>>>> Seeking can always fail even when the position was previously read.
> >>>>>
> >>>>> Fixes: Assertion failure
> >>>>> Fixes: 35253/clusterfuzz-testcase-minimized-ffmpeg_dem_MATROSKA_fuzzer-4693059982983168
> >>>>>
> >>>>> 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/matroskadec.c | 19 ++++++++++++-------
> >>>>> 1 file changed, 12 insertions(+), 7 deletions(-)
> >>>>>
> >>>>> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
> >>>>> index 356a02339c..a0e6e0cf8b 100644
> >>>>> --- a/libavformat/matroskadec.c
> >>>>> +++ b/libavformat/matroskadec.c
> >>>>> @@ -804,20 +804,22 @@ static int matroska_read_close(AVFormatContext *s);
> >>>>> static int matroska_reset_status(MatroskaDemuxContext *matroska,
> >>>>> uint32_t id, int64_t position)
> >>>>> {
> >>>>> + int64_t err = 0;
> >>>>> if (position >= 0) {
> >>>>> - int64_t err = avio_seek(matroska->ctx->pb, position, SEEK_SET);
> >>>>> - if (err < 0)
> >>>>> - return err;
> >>>>> - }
> >>>>> + err = avio_seek(matroska->ctx->pb, position, SEEK_SET);
> >>>>> + if (err > 0)
> >>>>> + err = 0;
> >>>>> + } else
> >>>>> + position = avio_tell(matroska->ctx->pb);
> >>>>>
> >>>>> matroska->current_id = id;
> >>>>> matroska->num_levels = 1;
> >>>>> matroska->unknown_count = 0;
> >>>>> - matroska->resync_pos = avio_tell(matroska->ctx->pb);
> >>>>> + matroska->resync_pos = position;
> >>>>> if (id)
> >>>>> matroska->resync_pos -= (av_log2(id) + 7) / 8;
> >>>>>
> >>>>> - return 0;
> >>>>> + return err;
> >>>>
> >>>> The changes here will make the demuxer update its internal state as if
> >>>> it had seeked to its target level-1-element, even though it didn't. Is
> >>>> this really good?
> >>>
> >>> I dont know.
> >>> Ive not seen this issue happen in reality just in a fuzzer
> >>> environment.
> >>>
> >>
> >> Can you send me this sample (with instructions how to reproduce it if
> >> necessary)?
> >
> > This seems still "crashing" according to the tracker
> > has this been fixed ?
> >
>
> No, I have not found time to look at it. Proceed as you wish.
well, i will look at other issues first then, maybe someone else
will fix this ...
thx
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Republics decline into democracies and democracies degenerate into
despotisms. -- Aristotle
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20210914/cbf88971/attachment.sig>
More information about the ffmpeg-devel
mailing list