[FFmpeg-devel] [PATCH 2/3] avformat: reject FFmpeg-style merged side data in raw packets

[Nicolas George]

Le nonidi 19 ventôse, an CCXXV, Michael Niedermayer a écrit :
> This is very basic really but lets elaborate
> for each side data type T
> possiblity A
> nothing uses side data type T
> 
> possiblity B
> something uses side data type T
> 
> Its the same with a codec, either a codec is used in some case or
> its used in no case.
> 
> If something is used in no case then it has been eliminated as you
> describe.
> If somehing is still used in a case it has not been eliminated
> 
> If as you describe side data has been eliminated then you could
> remove side data as a whole from the source code.
> 
> If you cannot remove side data or a specific side data type from
> the source code then it has not been eliminated
> 
> your change removes one way for an attacker to set side data but
> by the fact that you dont remove any of the side data types its
> clear you are aware of that every is still in use in some code path.
> 
> a attacker may need to use a specific container format to set a
> specific side data type or may depend on a specific demuxer lib or
> application that allows him to set a side data type.
> 
> now if you remove every way to set side data for an attacker then
> you can remove that side data type as a whole from the code.
> Of course that removes whatever the side data is for.
> 
> Let me provide a specific example
> If a container suports changing extradata mid stream it will either
> be support or not.
> if any demuxer supports it then you have not eliminated the possiblity
> for an attacker
> 
> I hope writing a elaborate reply will not lead to this discussion
> to shift onto some unrelated detail

You are rehashing a lot of obvious facts, but you do not address the
important questions.

Side data is useful. It is a badly designed API, because it adds a lot
of complexity for a hypothetical benefit but fails to reap that benefit.
As such, it should be kept but enhanced, either by removing the
complexity or by actually reaping the benefit.

But it is not the topic of this discussion.

MERGED side data is a completely brainded design that should never have
been written.

I am purposefully not looking at the archive to find out who is
responsible for this mess. Now is not the time to point fingers but to
fix the code.

Now, please answer this very specific question:

If someone were to REMOVE ALL AND EVERY use of
av_packet_merge_side_data() and av_packet_split_side_data(), what would
be the actual bad consequences?

But before you start with fuzzing or anything similar, let me stop you:
fuzzing exposes bugs that can be triggered by crafted inputs. If fuzzing
can not trigger it, that means the bug does not exist, period.

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20170309/4e114dbf/attachment.sig>