[FFmpeg-devel] [PATCH 2/3] avformat/evcdec: Check that enough data has been read

[Andreas Rheinhardt]

James Almer:
> On 7/6/2023 10:14 PM, James Almer wrote:
>> On 7/6/2023 6:08 PM, Andreas Rheinhardt wrote:
>>> Fixes potential use of uninitialized values
>>> in evc_read_nal_unit_length().
>>>
>>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at outlook.com>
>>> ---
>>>   libavformat/evcdec.c | 2 ++
>>>   1 file changed, 2 insertions(+)
>>>
>>> diff --git a/libavformat/evcdec.c b/libavformat/evcdec.c
>>> index 9886542311..0f464930f7 100644
>>> --- a/libavformat/evcdec.c
>>> +++ b/libavformat/evcdec.c
>>> @@ -162,6 +162,8 @@ static int evc_read_packet(AVFormatContext *s,
>>> AVPacket *pkt)
>>>           ret = avio_read(s->pb, buf, EVC_NALU_LENGTH_PREFIX_SIZE);
>>>           if (ret < 0)
>>>               return ret;
>>> +        if (ret != EVC_NALU_LENGTH_PREFIX_SIZE)
>>> +            return AVERROR_INVALIDDATA;
>>
>> There's a ffio_ensure_seekback() for EVC_NALU_LENGTH_PREFIX_SIZE bytes
>> immediately before the avio_read() call. Shouldn't that be enough to
>> guarantee that much can be read?
>>
>> Also, you can just pass ret to evc_read_nal_unit_length() below
>> instead of adding this check here. It will return an error if it's <
>> EVC_NALU_LENGTH_PREFIX_SIZE.
> 
> Oh, my bad, i was looking at the function of the same name in
> libavcodec/evc_parse.h
> 
> The function in evc.h could be changed to also use the same check as the
> one the evc_parse.h version alongside the other change you're doing in
> patch 3/3.
> 

These functions already do the same (except for the log message); they
both return 0 if not enough data is available. The return value would
need to be int64_t if one wanted to return both error codes and lengths
via it. I don't really see the advantage of this.

- Andreas