On Tue, Jun 28, 2022 at 08:26:54AM -0300, James Almer wrote:
> 
> 
> On 6/28/2022 2:21 AM, Anton Khirnov wrote:
> > Quoting Michael Niedermayer (2022-06-27 10:43:47)
> > > Fixes: Timeout
> > > Fixes: 48154/clusterfuzz-testcase-minimized-ffmpeg_dem_AAX_fuzzer-5149094353436672
> > > 
> > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > > ---
> > >   libavformat/aaxdec.c | 2 ++
> > >   1 file changed, 2 insertions(+)
> > > 
> > > diff --git a/libavformat/aaxdec.c b/libavformat/aaxdec.c
> > > index dd1fbde736..bcbff216db 100644
> > > --- a/libavformat/aaxdec.c
> > > +++ b/libavformat/aaxdec.c
> > > @@ -252,6 +252,8 @@ static int aax_read_header(AVFormatContext *s)
> > >                   size  = avio_rb32(pb);
> > >                   a->segments[r].start = start + a->data_offset;
> > >                   a->segments[r].end   = a->segments[r].start + size;
> > > +                if (!size)
> > > +                    return AVERROR_INVALIDDATA;
> > 
> > Why check for invalid size only after some things are set based on it
> > and not before?

moved it up


> 
> Also, if the problem is that a->segments[r].start == a->segments[r].end,
> then maybe it'd be better, or at least more clear to the reader, to ensure
> that as part of the checks immediately after this line.

its easy to add it into that check but the check becomes more complex and
harder to understand.
Unless you have some specific suggestion i would tend to keep the if()s seperate

thx

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

it is not once nor twice but times without number that the same ideas make
their appearance in the world. -- Aristotle