[FFmpeg-devel] [PATCH] avformat/mov: check offset for overflow in mov_probe()

[Michael Niedermayer]

On Sun, Apr 04, 2021 at 07:18:22PM -0300, James Almer wrote:
> On 4/4/2021 6:44 PM, Michael Niedermayer wrote:
> > Fixes: Invalid read of size 4
> > Fixes: ASAN_Deadlysignal.zip
> > 
> > Found-by: Hardik Shah <hardik05 at gmail.com>
> > Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
> > ---
> >   libavformat/mov.c | 2 ++
> >   1 file changed, 2 insertions(+)
> > 
> > diff --git a/libavformat/mov.c b/libavformat/mov.c
> > index 7805330bf9..ef73f3199d 100644
> > --- a/libavformat/mov.c
> > +++ b/libavformat/mov.c
> > @@ -7161,6 +7161,8 @@ static int mov_probe(const AVProbeData *p)
> >               score  = FFMAX(score, AVPROBE_SCORE_EXTENSION);
> >               break;
> >           }
> > +        if ((uint64_t)size + 8 > INT64_MAX - offset)
> > +            break;
> >           offset += size;
> >       }
> >       if (score > AVPROBE_SCORE_MAX - 50 && moov_offset != -1) {
> 
> Would something like this also work?
> 
> > diff --git a/libavformat/mov.c b/libavformat/mov.c
> > index 1974498d1e..cd9d9996b3 100644
> > --- a/libavformat/mov.c
> > +++ b/libavformat/mov.c
> > @@ -7119,7 +7119,7 @@ static int mov_probe(const AVProbeData *p)
> >          int64_t size;
> >          int minsize = 8;
> >          /* ignore invalid offset */
> > -        if ((offset + 8) > (unsigned int)p->buf_size)
> > +        if ((offset + 8ULL) > (unsigned int)p->buf_size)
> >              break;
> >          size = AV_RB32(p->buf + offset);
> >          if (size == 1 && offset + 16 <= (unsigned int)p->buf_size) {
> > @@ -7166,6 +7166,8 @@ static int mov_probe(const AVProbeData *p)
> >              score  = FFMAX(score, AVPROBE_SCORE_EXTENSION);
> >              break;
> >          }
> > +        if (size > INT64_MAX - offset)
> > +            break;
> >          offset += size;
> >      }
> >      if (score > AVPROBE_SCORE_MAX - 50 && moov_offset != -1) {
> 
> I think it conveys what it's trying to do more clearly than your version,
> where the + 8 on top of size is not immediately clear where it comes from.

yes that works, i will apply this

thx

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

No snowflake in an avalanche ever feels responsible. -- Voltaire
-------------- 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/20210405/efb47170/attachment.sig>