[FFmpeg-devel] [PATCH] add av_shrink_packet

[Reimar Döffinger]

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".

> 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...
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.