[FFmpeg-devel] [PATCH 1/3] ffmdec: reset packet_end in case of failure

[Andreas Cadhalpun]

On 02.01.2016 19:14, Michael Niedermayer wrote:
> On Sat, Jan 02, 2016 at 04:51:17PM +0100, Andreas Cadhalpun wrote:
>> This fixes segmentation faults caused by passing a packet_ptr of NULL to
>> memcpy.
>>
>> Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun at googlemail.com>
>> ---
>>  libavformat/ffmdec.c | 8 ++++++--
>>  1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/libavformat/ffmdec.c b/libavformat/ffmdec.c
>> index 9fe4155..7b2d0d7 100644
>> --- a/libavformat/ffmdec.c
>> +++ b/libavformat/ffmdec.c
>> @@ -123,8 +123,10 @@ static int ffm_read_data(AVFormatContext *s,
>>              frame_offset = avio_rb16(pb);
>>              avio_read(pb, ffm->packet, ffm->packet_size - FFM_HEADER_SIZE);
>>              ffm->packet_end = ffm->packet + (ffm->packet_size - FFM_HEADER_SIZE - fill_size);
>> -            if (ffm->packet_end < ffm->packet || frame_offset < 0)
>> +            if (ffm->packet_end < ffm->packet || frame_offset < 0) {
>> +                ffm->packet_end = ffm->packet_ptr;
> 
> doesnt this imply that packet_end was set to a invalid pointer?

Yes, if you use a strict definition of a valid pointer.
(It could still point to a valid memory address, but from a different
memory allocation than packet_ptr.)

By the way, the check for frame_offset < 0 is pointless, because
avio_rb16 returns an unsigned int.

> (that is undefined behavior)

Yes, but ubsan didn't catch it. ;)

Anyway, attached is an updated patch avoiding this problem.

Best regards,
Andreas
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-ffmdec-reset-packet_end-in-case-of-failure.patch
Type: text/x-diff
Size: 1958 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20160102/b588a8a7/attachment.patch>