[FFmpeg-devel] [PATCH] add av_shrink_packet

[Baptiste Coudurier]

On 4/8/2009 12:41 PM, Reimar D?ffinger wrote:
> On Wed, Apr 08, 2009 at 10:49:55AM -0700, Baptiste Coudurier wrote:
>> On 4/8/2009 2:29 AM, Reimar D?ffinger wrote:
>>> On Tue, Apr 07, 2009 at 06:02:39PM -0700, Baptiste Coudurier wrote:
>>>> On 4/7/2009 5:49 PM, Michael Niedermayer wrote:
>>>>> and decoders have to deal with nonsense input anyway, throwing such
>>>>> away at demuxer level doesnt feel correct to me
>>>> Well, I tend to agree but this is only good if someone actually fixes
>>>> crashes in decoder...
>>> [...]
>>>>> maybe we could set some flag in AVPacket to indicate that a packet is
>>>>> possibly damaged, iam not sure if this would be of any use, but a user
>>>>> application could at least drop such packets if its author thinks its
>>>>> better though i dont really think it is ...
>>>> Well when you know that decoder is not error proof regarding partial
>>>> packets, it certainly is IMHO.
>>> IMO that is not a valid argument, because such a decoder is a major
>>> security issue and needs to be fixed ASAP.
>> It is a perfectly valid argument.
>>
>> You just cannot predict bugs and considering complexity of some codecs
>> you may just don't exactly know if there are bugs in the code, you don't
>> know about weird cases.
>>
>> It would be _safer_ in any case to discard these packets.
> 
> I can't follow that reasoning, removing any chance that the bugs are triggered
> for "normal" files while it is still trivial to craft files that trigger them
> IMO does not even qualify as "security by obscurity".

Well, I understand your reasoning, and I tend to agree with it, but I
still believe that promoting this solution requires heavy maintaining
and devotion to fix every crash.

However, out there, in the real world, people don't like crashes and try
to avoid doing actions which increase chances of crashing.

>> A quick look at roundup will show you that ALAC has or had problems (and
>> the question to discard partial packets was considered) and I suspect
>> H.264 decoder has problems as well, considering how many crash are reported.
> 
> I was not aware of any serious crashes remaning, I admit I do not use ALAC but H.264
> seems quite stable to me. I have now seen a few in roundup for H.264
> (though those crashing in avpicture_layout look very suspicious and possible
> unrelated to the decoder), but there seem to be none for ALAC...

Issue 758 for ALAC, this is not crash but reading unitialized mem, this
could be considered potentially harmful. It seems the get_bits API does
not check end of buffer anyway (no use of size_in_bits field), I
understand it would slow down the code though.

> Though I have to point out that crashes would not be a big surprise, since
> libavcodec currently _requires_ the padding bytes to be 0, and libavformat
> currently _violates_ that requirement at least for partial packets.

That's possible, indeed.

-- 
Baptiste COUDURIER                              GnuPG Key Id: 0x5C1ABAAA
Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
checking for life_signs in -lkenny... no
FFmpeg maintainer                                  http://www.ffmpeg.org