[FFmpeg-devel] [PATCH] lavf/mov.c: Fix parsing of edit list atoms with invalid elst entry count.

Sasi Inguva isasi at google.com
Fri Oct 27 06:51:50 EEST 2017


On Tue, Oct 24, 2017 at 2:48 PM, Michael Niedermayer <michael at niedermayer.cc
> wrote:

> On Mon, Oct 23, 2017 at 04:18:28PM -0700, Sasi Inguva wrote:
> > Signed-off-by: Sasi Inguva <isasi at google.com>
> > ---
> >  libavformat/mov.c                           | 15 +++++++-
> >  tests/fate/mov.mak                          |  4 ++
> >  tests/ref/fate/mov-invalid-elst-entry-count | 57
> +++++++++++++++++++++++++++++
> >  3 files changed, 75 insertions(+), 1 deletion(-)
> >  create mode 100644 tests/ref/fate/mov-invalid-elst-entry-count
> >
> > diff --git a/libavformat/mov.c b/libavformat/mov.c
> > index b22a116140..424293ad93 100644
> > --- a/libavformat/mov.c
> > +++ b/libavformat/mov.c
> > @@ -4597,6 +4597,7 @@ static int mov_read_elst(MOVContext *c,
> AVIOContext *pb, MOVAtom atom)
> >  {
> >      MOVStreamContext *sc;
> >      int i, edit_count, version;
> > +    int64_t elst_entry_size;
> >
> >      if (c->fc->nb_streams < 1 || c->ignore_editlist)
> >          return 0;
> > @@ -4605,6 +4606,15 @@ static int mov_read_elst(MOVContext *c,
> AVIOContext *pb, MOVAtom atom)
> >      version = avio_r8(pb); /* version */
> >      avio_rb24(pb); /* flags */
> >      edit_count = avio_rb32(pb); /* entries */
> > +    atom.size -= 8;
> > +
> > +    elst_entry_size = version == 1 ? 20 : 12;
> > +    if (atom.size != edit_count * elst_entry_size &&
> > +        c->fc->strict_std_compliance >= FF_COMPLIANCE_STRICT) {
> > +        av_log(c->fc, AV_LOG_ERROR, "Invalid edit list entry_count: %d
> for elst atom of size: %"PRId64" bytes.\n",
> > +               edit_count, atom.size + 8);
> > +        return AVERROR_INVALIDDATA;
> > +    }
> >
> >      if (!edit_count)
> >          return 0;
> > @@ -4617,17 +4627,20 @@ static int mov_read_elst(MOVContext *c,
> AVIOContext *pb, MOVAtom atom)
> >          return AVERROR(ENOMEM);
> >
> >      av_log(c->fc, AV_LOG_TRACE, "track[%u].edit_count = %i\n",
> c->fc->nb_streams - 1, edit_count);
> > -    for (i = 0; i < edit_count && !pb->eof_reached; i++) {
> > +    for (i = 0; i < edit_count && atom.size > 0 && !pb->eof_reached;
> i++) {
> >          MOVElst *e = &sc->elst_data[i];
> >
> >          if (version == 1) {
> >              e->duration = avio_rb64(pb);
> >              e->time     = avio_rb64(pb);
> > +            atom.size -= 16;
> >          } else {
> >              e->duration = avio_rb32(pb); /* segment duration */
> >              e->time     = (int32_t)avio_rb32(pb); /* media time */
> > +            atom.size -= 8;
> >          }
> >          e->rate = avio_rb32(pb) / 65536.0;
> > +        atom.size -= 4;
> >          av_log(c->fc, AV_LOG_TRACE, "duration=%"PRId64" time=%"PRId64"
> rate=%f\n",
> >                 e->duration, e->time, e->rate);
>
> it would be simpler to adjust edit_count in case of ineqality.
> you already compute the elst_entry_size
> this would also avoid allocating a larger than needed array
>
> Done. Attaching the corrected patch.

>
> [...]
>
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Many things microsoft did are stupid, but not doing something just because
> microsoft did it is even more stupid. If everything ms did were stupid they
> would be bankrupt already.
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-lavf-mov.c-Fix-parsing-of-edit-list-atoms-with-inval.patch
Type: text/x-patch
Size: 8555 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20171026/263a4eb2/attachment.bin>


More information about the ffmpeg-devel mailing list