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

James Almer jamrial at gmail.com
Sat Oct 21 02:28:50 EEST 2017


On 10/19/2017 12:12 AM, Sasi Inguva wrote:
> Signed-off-by: Sasi Inguva <isasi at google.com>
> ---
>  libavformat/mov.c                           | 16 +++++++-
>  tests/fate/mov.mak                          |  6 ++-
>  tests/ref/fate/mov-invalid-elst-entry-count | 57 +++++++++++++++++++++++++++++
>  3 files changed, 76 insertions(+), 3 deletions(-)
>  create mode 100644 tests/ref/fate/mov-invalid-elst-entry-count
> 
> diff --git a/libavformat/mov.c b/libavformat/mov.c
> index b22a116140..8d2602c739 100644
> --- a/libavformat/mov.c
> +++ b/libavformat/mov.c
> @@ -4596,7 +4596,7 @@ free_and_return:
>  static int mov_read_elst(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>  {
>      MOVStreamContext *sc;
> -    int i, edit_count, version;
> +    int i, edit_count, version, elst_entry_size;
>  
>      if (c->fc->nb_streams < 1 || c->ignore_editlist)
>          return 0;
> @@ -4605,6 +4605,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 +4626,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);
>  
> diff --git a/tests/fate/mov.mak b/tests/fate/mov.mak
> index cfdada7a2e..012e6f61b7 100644
> --- a/tests/fate/mov.mak
> +++ b/tests/fate/mov.mak
> @@ -6,7 +6,8 @@ FATE_MOV = fate-mov-3elist \
>             fate-mov-1elist-ends-last-bframe \
>             fate-mov-2elist-elist1-ends-bframe \
>             fate-mov-3elist-encrypted \
> -           fate-mov-gpmf-remux \
> +	   fate-mov-invalid-elst-entry-count \
> +	   fate-mov-gpmf-remux \

Don't use tabs.

Could you look at ticket #6714 while at it? (And others also potentially
related to edit lists).


More information about the ffmpeg-devel mailing list