[FFmpeg-devel] [PATCH 1/3] avformat/matroskadec: Reset state also on failure in matroska_reset_status()

[Andreas Rheinhardt]

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)?

- Andreas