Re: [FFmpeg-devel] [PATCH 1/2] avformat/vividas: Check packet size

[Michael Niedermayer <michael@niedermayer.cc>]

[-- Attachment #1.1: Type: text/plain, Size: 2203 bytes --]

On Wed, Sep 28, 2022 at 05:16:05PM +0200, Anton Khirnov wrote:
> Quoting Michael Niedermayer (2022-09-22 20:08:51)
> > Fixes: signed integer overflow: 119760682 - -2084600173 cannot be represented in type 'int'
> > Fixes: 50993/clusterfuzz-testcase-minimized-ffmpeg_dem_VIVIDAS_fuzzer-6745781167587328
> > 
> > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> >  libavformat/vividas.c | 13 +++++++++++--
> >  1 file changed, 11 insertions(+), 2 deletions(-)
> > 
> > diff --git a/libavformat/vividas.c b/libavformat/vividas.c
> > index e9954f73ed0..e8efe49a5c0 100644
> > --- a/libavformat/vividas.c
> > +++ b/libavformat/vividas.c
> > @@ -683,6 +683,7 @@ static int viv_read_packet(AVFormatContext *s,
> >  
> >      if (viv->sb_entries[viv->current_sb_entry].flag == 0) {
> >          uint64_t v_size = ffio_read_varlen(pb);
> > +        int last, last_start;
> >  
> >          if (!viv->num_audio)
> >              return AVERROR_INVALIDDATA;
> > @@ -704,14 +705,22 @@ static int viv_read_packet(AVFormatContext *s,
> >              start = ffio_read_varlen(pb);
> >              pcm_bytes = ffio_read_varlen(pb);
> >  
> > -            if (i > 0 && start == 0)
> > -                break;
> > +            if (i > 0) {
> > +                if (start == 0)
> > +                    break;
> > +                if (start < last || start - (unsigned)last > INT_MAX)
> 
> What is the second condition for?

start is signed int so are "copyies" of it
"start < last" would allow a negative last with a large start
the 2nd check handles that.
the difference of consequtive values are stored as int later

The patch tried to leave the storage types and check for it.
The types could be changed or some other checks could be used
I was undecided on this patch a bit too. I picked this mainly
to keep changes more minimal but maybe this was not the best
choice

Thx


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

No great genius has ever existed without some touch of madness. -- Aristotle

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".