[FFmpeg-devel] [PATCH 3/3] avcodec/cfhd: More strictly check tag order and multiplicity

[Michael Niedermayer]

On Wed, Dec 23, 2020 at 10:52:03AM +0100, Paul B Mahol wrote:
> On Tue, Dec 22, 2020 at 10:27 PM Michael Niedermayer <michael at niedermayer.cc>
> wrote:
> 
> > On Sun, Dec 20, 2020 at 10:18:40PM +0100, Paul B Mahol wrote:
> > > Unacceptable, please share privately sample that allows to reproduce
> > this.
> >
> > shared the ones which reproduce.
> >
> > Please explain why this patch is unacceptable to you.
> >
> > the CFHD decoder decodes header elements in the order in which they are
> > stored. The problem is that many have interdependancies yet there are
> > no checks for these. And where there are checks theres no protection
> > against changing dependancies after they have been used.
> > Basically CFHD allows an attacker to do absolutely anything
> >
> > To pick a random example:
> > the code reading the SubbandNumber adjusts the level and then
> > checks its range based on transform_type. Yet transform_type
> > may be not set yet or may be subsequently changed.
> > That is issue 27872
> >
> > One surely can try to add specific checks for all this but i doubt that
> > will
> > result in secure code anytime soon. Its IMO better to fundamentally
> > fix this and not allow anything to occur in any multiplicity and order.
> > My posted patch is one way of many possible alternatives to move in that
> > direction
> >
> >
> Well, you forgot that when you check for order of tags, you basically still
> allow
> crash to happen, just this time crashing code path needs to follow correct
> parsing order.

I dont see how that would be possible

with the correct order of tags
transform_type is set and checked and can after that only be 0 or 2
the crash required it to be -1. As transform_type is marked as a mandatory
element in the table and -1 is not a possible value after it i dont see
how that could work.
But maybe iam missing something

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Breaking DRM is a little like attempting to break through a door even
though the window is wide open and the only thing in the house is a bunch
of things you dont want and which you would get tomorrow for free anyway
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20201223/326cecd6/attachment.sig>