On Fri, Jun 27, 2025 at 12:20:11PM -0700, Pavel Roslyy wrote:
> On Thu, Jun 26, 2025 at 2:08 PM Michael Niedermayer
> <michael@niedermayer.cc> wrote:
> >
> > Hi Pavel
> >
> > On Thu, Jun 26, 2025 at 12:04:17AM -0700, Pavel Roslyy wrote:
> > > On Wed, Jun 25, 2025 at 3:40 PM Michael Niedermayer
> > > <michael@niedermayer.cc> wrote:
> > > >
> > > > [...]
> > > >
> > > > bug found, not applying yet
> > > >
> > > > ret = ff_alloc_extradata(par, pkt_size + key_buf);
> > > >
> > > > pkt_size + key_buf can overflow i think
> > >
> > > If I'm understanding right, I don't think it can.
> > > pkt_size = chunk_size - (ret - chunk_start) - padding_size;
> > >
> > > (ret - chunk_start) should be at least 24 at this point, and I don't think
> > > padding_size will be negative so pkt_size is at most UINT32_MAX - 24.
> >
> > chunk_size is arbitrary 32bit thus pkt_size is arbitrary 32bit
> >
> >
> > >
> > > key_buf adds at most 10, which is not enough to overflow.
> >
> > arbitrary uint32_t + 10 can overflow. Its a defined overflow
> > but the following allocation is then bad
> 
> I think what happens is arbitrary uint32_t - 24 + 10, which cannot overflow
> but it looks like I wasn't convincing.  I assume you want an overflow check?
> 

> if (key_buf > UINT32_MAX - pkt_size)
>     return AVERROR_INVALIDDATA;
> 
> Would this work or do you have a better suggestion?

ok

thx

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

Nations do behave wisely once they have exhausted all other alternatives. 
-- Abba Eban