diff mbox

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

Message ID 20171029123906.GO6009@nb4
State Not Applicable
Headers show

Commit Message

Michael Niedermayer Oct. 29, 2017, 12:39 p.m. UTC
On Sat, Oct 28, 2017 at 08:26:16PM +0200, Michael Niedermayer wrote:
> On Thu, Oct 26, 2017 at 08:51:50PM -0700, Sasi Inguva wrote:
> > On Tue, Oct 24, 2017 at 2:48 PM, Michael Niedermayer <michael@niedermayer.cc
> > > wrote:
> > 
> > > On Mon, Oct 23, 2017 at 04:18:28PM -0700, Sasi Inguva wrote:
> > > > Signed-off-by: Sasi Inguva <isasi@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@ffmpeg.org
> > > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> > >
> > >
> 
> >  libavformat/mov.c                           |   21 +++++++++-
> >  tests/fate/mov.mak                          |    4 +
> >  tests/ref/fate/mov-invalid-elst-entry-count |   57 ++++++++++++++++++++++++++++
> >  3 files changed, 81 insertions(+), 1 deletion(-)
> > c553340f66797876d039f408f83574a40c54d17b  0001-lavf-mov.c-Fix-parsing-of-edit-list-atoms-with-inval.patch
> > From 188602e662868a79c7f85e9193e9aedc9ba1a170 Mon Sep 17 00:00:00 2001
> > From: Sasi Inguva <isasi@google.com>
> > Date: Wed, 18 Oct 2017 20:11:16 -0700
> > Subject: [PATCH] lavf/mov.c: Fix parsing of edit list atoms with invalid elst
> >  entry count.
> 
> applied

It seems this fails on ARM qemu

TEST    mov-invalid-elst-entry-count
Test mov-invalid-elst-entry-count failed. Look at tests/data/fate/mov-invalid-elst-entry-count.err for details.
make: *** [fate-mov-invalid-elst-entry-count] Error 1


[...]

Comments

Carl Eugen Hoyos Oct. 29, 2017, 2:06 p.m. UTC | #1
2017-10-29 13:39 GMT+01:00 Michael Niedermayer <michael@niedermayer.cc>:
> On Sat, Oct 28, 2017 at 08:26:16PM +0200, Michael Niedermayer wrote:
>> On Thu, Oct 26, 2017 at 08:51:50PM -0700, Sasi Inguva wrote:

>> >  libavformat/mov.c                           |   21 +++++++++-
>> >  tests/fate/mov.mak                          |    4 +
>> >  tests/ref/fate/mov-invalid-elst-entry-count |   57 ++++++++++++++++++++++++++++
>> >  3 files changed, 81 insertions(+), 1 deletion(-)
>> > c553340f66797876d039f408f83574a40c54d17b  0001-lavf-mov.c-Fix-parsing-of-edit-list-atoms-with-inval.patch
>> > From 188602e662868a79c7f85e9193e9aedc9ba1a170 Mon Sep 17 00:00:00 2001
>> > From: Sasi Inguva <isasi@google.com>
>> > Date: Wed, 18 Oct 2017 20:11:16 -0700
>> > Subject: [PATCH] lavf/mov.c: Fix parsing of edit list atoms with invalid elst
>> >  entry count.
>>
>> applied
>
> It seems this fails on ARM qemu

Fails differently here on ppc64be:

--- ./tests/ref/fate/mov-invalid-elst-entry-count       2017-10-28
22:32:57.766024470 +0000
+++ tests/data/fate/mov-invalid-elst-entry-count        2017-10-29
14:04:35.227325635 +0000
@@ -7,51 +7,51 @@
 #dimensions 0: 640x480
 #sar 0: 1/1
 #stream#, dts,        pts, duration,     size, hash
-0,          0,          0,        1,   460800, 549730883a0b56e6accaf021903daecf
-0,          1,          1,        1,   460800, 783389b4342d4be925fc5244791e760a
-0,          2,          2,        1,   460800, 8384af6426d94a2077930c93013e09ad
-0,          3,          3,        1,   460800, 9380a1d9ecacf5b3105383c1c8083188
-0,          4,          4,        1,   460800, eb28174acfceb868b9058757bed049c5
-0,          5,          5,        1,   460800, 9732bd4a58884dbf2be48d819433130f
-0,          6,          6,        1,   460800, 0c553fb530cf042eb84f5b13817a96a6
-0,          7,          7,        1,   460800, 621f02aded5e35fa1f373afd3ed283bd
-0,          8,          8,        1,   460800, c76167c6bda91f657708c88252ea315d
-0,          9,          9,        1,   460800, 872df2d8c522e2440ddd04bca7dce497
-0,         10,         10,        1,   460800, 6ee9154e48c5132ad4ba122b255bd2bb
-0,         11,         11,        1,   460800, 362e61629795702ebe9183ce3786d7f2
-0,         12,         12,        1,   460800, f3ec59e6fc4e3c2e75f42bef34ca73b5
-0,         13,         13,        1,   460800, 68d9caea8697736dd716cba43b614919
-0,         14,         14,        1,   460800, 4a4efb0201a64236db4330725758c139
-0,         15,         15,        1,   460800, f32f8997dcdd87ad910dea886a0de17d
-0,         16,         16,        1,   460800, 51a8549d7b4aaacaf6050bc07a82b440
-0,         17,         17,        1,   460800, 5145aa05bbb0c3faab40fc8fa233af1d
-0,         18,         18,        1,   460800, bbfcbe3c9600b2a0f413057d7959e9e7
-0,         19,         19,        1,   460800, 02cfd4a350fa274e12fce8352001bf21
-0,         20,         20,        1,   460800, 20dd372da9e656add433f31e3e9c1fb8
-0,         21,         21,        1,   460800, 3b885593f8b42676ce40c329a63f62bf
-0,         22,         22,        1,   460800, c38b453b56c3ea14f7d8691d83752486
-0,         23,         23,        1,   460800, 79643132988dabc9dc1ba3af0aeaebc5
-0,         24,         24,        1,   460800, 60a099be31244b2f69ca6107cdbd7e06
-0,         25,         25,        1,   460800, 1de6ff4e0aa81216e4b7b1c8e74fb143
-0,         26,         26,        1,   460800, 5223a81e6964c28cf42593f259397aa1
-0,         27,         27,        1,   460800, 2dfcf01c86aa712cd6f1c7656eeb17db
-0,         28,         28,        1,   460800, 8c86ee0f02fabccaed8d8fc8babd031e
-0,         29,         29,        1,   460800, b3ea1983f7efeec11306445d9ae5d477
-0,         30,         30,        1,   460800, 86a4cc9fa7db5ff5ca2be69ad191179f
-0,         31,         31,        1,   460800, 8194715afe23ae34a019797a53a6ee2c
-0,         32,         32,        1,   460800, 447a153f1c6bb703eff62edfd14e08e0
-0,         33,         33,        1,   460800, 092257082789b898dbb14d1f19e79347
-0,         34,         34,        1,   460800, d6320d204a119cfeef5645a4118bc600
-0,         35,         35,        1,   460800, 2ee710deae4bb0d156528797ad1c4981
-0,         36,         36,        1,   460800, 1256eac030985c04c4501ad5a72e9d66
-0,         37,         37,        1,   460800, f16ad8c1aa572be7666c7907ce4aebbc
-0,         38,         38,        1,   460800, 865088cbd47d0151b62a45d5426c8216
-0,         39,         39,        1,   460800, 26c78ca43d93c6da153f3dea5d945e0e
-0,         40,         40,        1,   460800, d775d6705c965401ccc143d5ae432938
-0,         41,         41,        1,   460800, f9837d514753c59e6776452d9043524f
-0,         42,         42,        1,   460800, 8463f5172914828abcc770f888bfd183
-0,         43,         43,        1,   460800, 3108557748cfb7965b33b16b35359de0
-0,         44,         44,        1,   460800, 477d596944e028dd72c207bb6e6b22de
-0,         45,         45,        1,   460800, 69e4ffbd600c8d8bc070d7d7324ee2b1
-0,         46,         46,        1,   460800, 2211c57bc9ec1788217002900f9102b1
-0,         47,         47,        1,   460800, bcfb5f0a7f88da3ec8c6b447ed4b67eb
+0,          0,          0,        1,   460800, 4a08c4fa53e02025ca698aba8cfe85df
+0,          1,          1,        1,   460800, 020e94a472e51a3cdeeee829aa52aaac
+0,          2,          2,        1,   460800, 5cab0035d7bc0c669fc220c213bca91a
+0,          3,          3,        1,   460800, 99f4da82dd19e91ebe41f6ae7cd55678
+0,          4,          4,        1,   460800, e67609abacb82cbe71661e309b94dfad
+0,          5,          5,        1,   460800, 2572c7691a795ffa6fc3739d975ab6ce
+0,          6,          6,        1,   460800, 74dcad292ea6e4fd997d35c0c89f32ba
+0,          7,          7,        1,   460800, 14b5f3ee154e6cf963e1d0238e39f935
+0,          8,          8,        1,   460800, 852e622431b2df7aee3a3b6be416fe11
+0,          9,          9,        1,   460800, 1c92d30597340196d85e48eb69ae3731
+0,         10,         10,        1,   460800, fed59cce13933cf3fc77e52d1cdbab3b
+0,         11,         11,        1,   460800, c7ff57fe043494bf589b7ed9af662093
+0,         12,         12,        1,   460800, 7ff9e3d4170b2bd53c7a82d704f61dfd
+0,         13,         13,        1,   460800, 70a48c539b07f41491191bd5bf3cb641
+0,         14,         14,        1,   460800, bf2c89c862cc255d2ce16776f6e18d48
+0,         15,         15,        1,   460800, 9410ee9a03a67bef668b2f95110fb21e
+0,         16,         16,        1,   460800, f8ea9b36f2f448c10d095b69d887eee7
+0,         17,         17,        1,   460800, 3959a273d8f1e830edd5fdb3bf02eafa
+0,         18,         18,        1,   460800, b4596024b3512be4f82492da22859be9
+0,         19,         19,        1,   460800, 555742238e273f7f2b81ae27bc3fc7dd
+0,         20,         20,        1,   460800, 0ddba2488002dc0d9fc4bcfe760c24ba
+0,         21,         21,        1,   460800, 3364bf8fc34508efd25e7b37516406fd
+0,         22,         22,        1,   460800, 3fb01bdeda657bb2673b70df7eb2c70d
+0,         23,         23,        1,   460800, bef8d3e2b722e79311bab50e38a48872
+0,         24,         24,        1,   460800, 8c072a41d1c0bd15e3dc69f2a826a08d
+0,         25,         25,        1,   460800, 26bd55391ff97f3b4e775d6327083420
+0,         26,         26,        1,   460800, 0a8a0f2f128d9698b12ac15d2487e4e9
+0,         27,         27,        1,   460800, 1477fd4e6452c1cf5a5e21868ae42086
+0,         28,         28,        1,   460800, 378641a8564e83164291d5754b1a0d17
+0,         29,         29,        1,   460800, 9b5ccf98982dc8b9f4297ac1f9d26a0b
+0,         30,         30,        1,   460800, 347e6ceb6792e374ed0c5f22b46ec29e
+0,         31,         31,        1,   460800, 707febefaec545d5c7587b9b8020c803
+0,         32,         32,        1,   460800, e3887ac397c73bf48f206009575b57c6
+0,         33,         33,        1,   460800, 327e0a9a1c7ead76b19f898cceb878d0
+0,         34,         34,        1,   460800, 44206e9bd73586377994c470fe7ba122
+0,         35,         35,        1,   460800, f81d054b8e0cc971c8691cdeed2ce08d
+0,         36,         36,        1,   460800, aa9d013200b8249d3be9af8b98fe723d
+0,         37,         37,        1,   460800, 063e387111509734e00fb60127e97eb1
+0,         38,         38,        1,   460800, 541a5efc9cb13bde0e0edccb7dceb2c7
+0,         39,         39,        1,   460800, 877ffed61eff3afd9377f7ae46d9c4f4
+0,         40,         40,        1,   460800, 8c755688e9784b67e37c2405fd46ead8
+0,         41,         41,        1,   460800, c151973719cf57337074efc1a1e6a04b
+0,         42,         42,        1,   460800, ebefe4c78f5dc0777c5b899acaea47ba
+0,         43,         43,        1,   460800, 9fbc39f4907064414d9cc6d585082100
+0,         44,         44,        1,   460800, 1f029a3c51f3a6293486763fe43c447c
+0,         45,         45,        1,   460800, bf92711a080b63264938f69775af9799
+0,         46,         46,        1,   460800, 34116e65a099e5eaf62e487331b5c700
+0,         47,         47,        1,   460800, 435e7e9ebc2c5773149da64a681e40bb
Test mov-invalid-elst-entry-count failed. Look at
tests/data/fate/mov-invalid-elst-entry-count.err for details.

Carl Eugen
Michael Niedermayer Dec. 4, 2017, 12:22 a.m. UTC | #2
On Sun, Oct 29, 2017 at 01:39:06PM +0100, Michael Niedermayer wrote:
> On Sat, Oct 28, 2017 at 08:26:16PM +0200, Michael Niedermayer wrote:
> > On Thu, Oct 26, 2017 at 08:51:50PM -0700, Sasi Inguva wrote:
> > > On Tue, Oct 24, 2017 at 2:48 PM, Michael Niedermayer <michael@niedermayer.cc
> > > > wrote:
> > > 
> > > > On Mon, Oct 23, 2017 at 04:18:28PM -0700, Sasi Inguva wrote:
> > > > > Signed-off-by: Sasi Inguva <isasi@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@ffmpeg.org
> > > > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> > > >
> > > >
> > 
> > >  libavformat/mov.c                           |   21 +++++++++-
> > >  tests/fate/mov.mak                          |    4 +
> > >  tests/ref/fate/mov-invalid-elst-entry-count |   57 ++++++++++++++++++++++++++++
> > >  3 files changed, 81 insertions(+), 1 deletion(-)
> > > c553340f66797876d039f408f83574a40c54d17b  0001-lavf-mov.c-Fix-parsing-of-edit-list-atoms-with-inval.patch
> > > From 188602e662868a79c7f85e9193e9aedc9ba1a170 Mon Sep 17 00:00:00 2001
> > > From: Sasi Inguva <isasi@google.com>
> > > Date: Wed, 18 Oct 2017 20:11:16 -0700
> > > Subject: [PATCH] lavf/mov.c: Fix parsing of edit list atoms with invalid elst
> > >  entry count.
> > 
> > applied
> 
> It seems this fails on ARM qemu
> 
> TEST    mov-invalid-elst-entry-count
> --- tests/ref/fate/mov-invalid-elst-entry-count 2017-10-29 13:26:29.180224127 +0100
> +++ tests/data/fate/mov-invalid-elst-entry-count        2017-10-29 13:38:18.536239072 +0100
> @@ -8,50 +8,50 @@
>  #sar 0: 1/1
>  #stream#, dts,        pts, duration,     size, hash
>  0,          0,          0,        1,   460800, 549730883a0b56e6accaf021903daecf
> -0,          1,          1,        1,   460800, 783389b4342d4be925fc5244791e760a
> -0,          2,          2,        1,   460800, 8384af6426d94a2077930c93013e09ad
> -0,          3,          3,        1,   460800, 9380a1d9ecacf5b3105383c1c8083188

should i disable this test or is someone working on fixing this ?
i guess its some rounding difference but thats just guessing

fate tests must not fail on any supported platform

[...]
Carl Eugen Hoyos Dec. 4, 2017, 2:24 a.m. UTC | #3
2017-12-04 1:22 GMT+01:00 Michael Niedermayer <michael@niedermayer.cc>:
> On Sun, Oct 29, 2017 at 01:39:06PM +0100, Michael Niedermayer wrote:
>> On Sat, Oct 28, 2017 at 08:26:16PM +0200, Michael Niedermayer wrote:
>> > On Thu, Oct 26, 2017 at 08:51:50PM -0700, Sasi Inguva wrote:
>> > > On Tue, Oct 24, 2017 at 2:48 PM, Michael Niedermayer <michael@niedermayer.cc
>> > > > wrote:
>> > >
>> > > > On Mon, Oct 23, 2017 at 04:18:28PM -0700, Sasi Inguva wrote:
>> > > > > Signed-off-by: Sasi Inguva <isasi@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@ffmpeg.org
>> > > > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>> > > >
>> > > >
>> >
>> > >  libavformat/mov.c                           |   21 +++++++++-
>> > >  tests/fate/mov.mak                          |    4 +
>> > >  tests/ref/fate/mov-invalid-elst-entry-count |   57 ++++++++++++++++++++++++++++
>> > >  3 files changed, 81 insertions(+), 1 deletion(-)
>> > > c553340f66797876d039f408f83574a40c54d17b  0001-lavf-mov.c-Fix-parsing-of-edit-list-atoms-with-inval.patch
>> > > From 188602e662868a79c7f85e9193e9aedc9ba1a170 Mon Sep 17 00:00:00 2001
>> > > From: Sasi Inguva <isasi@google.com>
>> > > Date: Wed, 18 Oct 2017 20:11:16 -0700
>> > > Subject: [PATCH] lavf/mov.c: Fix parsing of edit list atoms with invalid elst
>> > >  entry count.
>> >
>> > applied
>>
>> It seems this fails on ARM qemu
>>
>> TEST    mov-invalid-elst-entry-count
>> --- tests/ref/fate/mov-invalid-elst-entry-count 2017-10-29 13:26:29.180224127 +0100
>> +++ tests/data/fate/mov-invalid-elst-entry-count        2017-10-29 13:38:18.536239072 +0100
>> @@ -8,50 +8,50 @@
>>  #sar 0: 1/1
>>  #stream#, dts,        pts, duration,     size, hash
>>  0,          0,          0,        1,   460800, 549730883a0b56e6accaf021903daecf
>> -0,          1,          1,        1,   460800, 783389b4342d4be925fc5244791e760a
>> -0,          2,          2,        1,   460800, 8384af6426d94a2077930c93013e09ad
>> -0,          3,          3,        1,   460800, 9380a1d9ecacf5b3105383c1c8083188
>
> should i disable this test or is someone working on fixing this ?

I don't know what the issue is and I am not working on a fix.

Carl Eugen
Michael Niedermayer Dec. 4, 2017, 6:26 p.m. UTC | #4
On Mon, Dec 04, 2017 at 03:24:18AM +0100, Carl Eugen Hoyos wrote:
> 2017-12-04 1:22 GMT+01:00 Michael Niedermayer <michael@niedermayer.cc>:
> > On Sun, Oct 29, 2017 at 01:39:06PM +0100, Michael Niedermayer wrote:
> >> On Sat, Oct 28, 2017 at 08:26:16PM +0200, Michael Niedermayer wrote:
> >> > On Thu, Oct 26, 2017 at 08:51:50PM -0700, Sasi Inguva wrote:
> >> > > On Tue, Oct 24, 2017 at 2:48 PM, Michael Niedermayer <michael@niedermayer.cc
> >> > > > wrote:
> >> > >
> >> > > > On Mon, Oct 23, 2017 at 04:18:28PM -0700, Sasi Inguva wrote:
> >> > > > > Signed-off-by: Sasi Inguva <isasi@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@ffmpeg.org
> >> > > > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >> > > >
> >> > > >
> >> >
> >> > >  libavformat/mov.c                           |   21 +++++++++-
> >> > >  tests/fate/mov.mak                          |    4 +
> >> > >  tests/ref/fate/mov-invalid-elst-entry-count |   57 ++++++++++++++++++++++++++++
> >> > >  3 files changed, 81 insertions(+), 1 deletion(-)
> >> > > c553340f66797876d039f408f83574a40c54d17b  0001-lavf-mov.c-Fix-parsing-of-edit-list-atoms-with-inval.patch
> >> > > From 188602e662868a79c7f85e9193e9aedc9ba1a170 Mon Sep 17 00:00:00 2001
> >> > > From: Sasi Inguva <isasi@google.com>
> >> > > Date: Wed, 18 Oct 2017 20:11:16 -0700
> >> > > Subject: [PATCH] lavf/mov.c: Fix parsing of edit list atoms with invalid elst
> >> > >  entry count.
> >> >
> >> > applied
> >>
> >> It seems this fails on ARM qemu
> >>
> >> TEST    mov-invalid-elst-entry-count
> >> --- tests/ref/fate/mov-invalid-elst-entry-count 2017-10-29 13:26:29.180224127 +0100
> >> +++ tests/data/fate/mov-invalid-elst-entry-count        2017-10-29 13:38:18.536239072 +0100
> >> @@ -8,50 +8,50 @@
> >>  #sar 0: 1/1
> >>  #stream#, dts,        pts, duration,     size, hash
> >>  0,          0,          0,        1,   460800, 549730883a0b56e6accaf021903daecf
> >> -0,          1,          1,        1,   460800, 783389b4342d4be925fc5244791e760a
> >> -0,          2,          2,        1,   460800, 8384af6426d94a2077930c93013e09ad
> >> -0,          3,          3,        1,   460800, 9380a1d9ecacf5b3105383c1c8083188
> >
> > should i disable this test or is someone working on fixing this ?
> 
> I don't know what the issue is and I am not working on a fix.

i was more thinking of sasi than you but i guess its better
to disable this either way until its fixed
diff mbox

Patch

--- tests/ref/fate/mov-invalid-elst-entry-count 2017-10-29 13:26:29.180224127 +0100
+++ tests/data/fate/mov-invalid-elst-entry-count        2017-10-29 13:38:18.536239072 +0100
@@ -8,50 +8,50 @@ 
 #sar 0: 1/1
 #stream#, dts,        pts, duration,     size, hash
 0,          0,          0,        1,   460800, 549730883a0b56e6accaf021903daecf
-0,          1,          1,        1,   460800, 783389b4342d4be925fc5244791e760a
-0,          2,          2,        1,   460800, 8384af6426d94a2077930c93013e09ad
-0,          3,          3,        1,   460800, 9380a1d9ecacf5b3105383c1c8083188
-0,          4,          4,        1,   460800, eb28174acfceb868b9058757bed049c5
-0,          5,          5,        1,   460800, 9732bd4a58884dbf2be48d819433130f
-0,          6,          6,        1,   460800, 0c553fb530cf042eb84f5b13817a96a6
-0,          7,          7,        1,   460800, 621f02aded5e35fa1f373afd3ed283bd
-0,          8,          8,        1,   460800, c76167c6bda91f657708c88252ea315d
-0,          9,          9,        1,   460800, 872df2d8c522e2440ddd04bca7dce497
-0,         10,         10,        1,   460800, 6ee9154e48c5132ad4ba122b255bd2bb
-0,         11,         11,        1,   460800, 362e61629795702ebe9183ce3786d7f2
-0,         12,         12,        1,   460800, f3ec59e6fc4e3c2e75f42bef34ca73b5
-0,         13,         13,        1,   460800, 68d9caea8697736dd716cba43b614919
-0,         14,         14,        1,   460800, 4a4efb0201a64236db4330725758c139
-0,         15,         15,        1,   460800, f32f8997dcdd87ad910dea886a0de17d
-0,         16,         16,        1,   460800, 51a8549d7b4aaacaf6050bc07a82b440
-0,         17,         17,        1,   460800, 5145aa05bbb0c3faab40fc8fa233af1d
-0,         18,         18,        1,   460800, bbfcbe3c9600b2a0f413057d7959e9e7
-0,         19,         19,        1,   460800, 02cfd4a350fa274e12fce8352001bf21
-0,         20,         20,        1,   460800, 20dd372da9e656add433f31e3e9c1fb8
-0,         21,         21,        1,   460800, 3b885593f8b42676ce40c329a63f62bf
-0,         22,         22,        1,   460800, c38b453b56c3ea14f7d8691d83752486
-0,         23,         23,        1,   460800, 79643132988dabc9dc1ba3af0aeaebc5
-0,         24,         24,        1,   460800, 60a099be31244b2f69ca6107cdbd7e06
-0,         25,         25,        1,   460800, 1de6ff4e0aa81216e4b7b1c8e74fb143
-0,         26,         26,        1,   460800, 5223a81e6964c28cf42593f259397aa1
-0,         27,         27,        1,   460800, 2dfcf01c86aa712cd6f1c7656eeb17db
-0,         28,         28,        1,   460800, 8c86ee0f02fabccaed8d8fc8babd031e
-0,         29,         29,        1,   460800, b3ea1983f7efeec11306445d9ae5d477
-0,         30,         30,        1,   460800, 86a4cc9fa7db5ff5ca2be69ad191179f
-0,         31,         31,        1,   460800, 8194715afe23ae34a019797a53a6ee2c
-0,         32,         32,        1,   460800, 447a153f1c6bb703eff62edfd14e08e0
-0,         33,         33,        1,   460800, 092257082789b898dbb14d1f19e79347
-0,         34,         34,        1,   460800, d6320d204a119cfeef5645a4118bc600
-0,         35,         35,        1,   460800, 2ee710deae4bb0d156528797ad1c4981
-0,         36,         36,        1,   460800, 1256eac030985c04c4501ad5a72e9d66
-0,         37,         37,        1,   460800, f16ad8c1aa572be7666c7907ce4aebbc
-0,         38,         38,        1,   460800, 865088cbd47d0151b62a45d5426c8216
-0,         39,         39,        1,   460800, 26c78ca43d93c6da153f3dea5d945e0e
-0,         40,         40,        1,   460800, d775d6705c965401ccc143d5ae432938
-0,         41,         41,        1,   460800, f9837d514753c59e6776452d9043524f
-0,         42,         42,        1,   460800, 8463f5172914828abcc770f888bfd183
-0,         43,         43,        1,   460800, 3108557748cfb7965b33b16b35359de0
-0,         44,         44,        1,   460800, 477d596944e028dd72c207bb6e6b22de
-0,         45,         45,        1,   460800, 69e4ffbd600c8d8bc070d7d7324ee2b1
-0,         46,         46,        1,   460800, 2211c57bc9ec1788217002900f9102b1
-0,         47,         47,        1,   460800, bcfb5f0a7f88da3ec8c6b447ed4b67eb
+0,          1,          1,        1,   460800, 775562e27486a2c36c0349df75f6d4a0
+0,          2,          2,        1,   460800, 3636927be803c7d268a48fb2052b90a1
+0,          3,          3,        1,   460800, 015bcde069a5c4cf3739400a5929980e
+0,          4,          4,        1,   460800, bc3583f954eae618828cfe8b030943d6
+0,          5,          5,        1,   460800, d2716fdc5eed8cde9126170caad647f3
+0,          6,          6,        1,   460800, 33ba34bede74e292ac3d8983215a2fb9
+0,          7,          7,        1,   460800, 86d9a78c874b583abaab13904418be05
+0,          8,          8,        1,   460800, ac343233031e7f6210983e6a8d3ff0f0
+0,          9,          9,        1,   460800, d0c8b10539808d816a0629d50115da51
+0,         10,         10,        1,   460800, f2e89405cf1a977ed92593b790863ba3
+0,         11,         11,        1,   460800, dfcb845ade78744be3f09c3f6e39c343
+0,         12,         12,        1,   460800, ae32bb4886a7c32bd6d9166854dd2996
+0,         13,         13,        1,   460800, 38e556eca3301f4493a80814907cab72
+0,         14,         14,        1,   460800, a604c22a62faa29da2f15d1657f2b601
+0,         15,         15,        1,   460800, 256c65a300c5d0fd55294461a94a951d
+0,         16,         16,        1,   460800, 12fa29377baf8ec06636a12658067e59
+0,         17,         17,        1,   460800, 90e197533dca39d92d761912300d8da4
+0,         18,         18,        1,   460800, d28a6429a157deb55737bda95bc155f9
+0,         19,         19,        1,   460800, e57be4b099a80c7aaebc24f1532a674c
+0,         20,         20,        1,   460800, 9188eac3bb43f8862f476c3a6940d17f
+0,         21,         21,        1,   460800, ee7956492057442f86fe582577640b98
+0,         22,         22,        1,   460800, 029a8cdc7c6c86fdb96a4b8714e70530
+0,         23,         23,        1,   460800, bee2eeadc2d31a655c3e27a089da98f0
+0,         24,         24,        1,   460800, 93e124d391bb707b6d393b9b27391186
+0,         25,         25,        1,   460800, b678115be54d0d339b499ef43cde7aa2
+0,         26,         26,        1,   460800, 2ecd64473e956cdeb589dbc4bedf7aca
+0,         27,         27,        1,   460800, f3691b67f915a0675f4ca096d83a90fa
+0,         28,         28,        1,   460800, 576dadb29b63c99e9a2dd61920336da3
+0,         29,         29,        1,   460800, 364301bd29a2bdb46a0aac0fdc543ca3
+0,         30,         30,        1,   460800, 32ad47e9909abf79bc3c357f10d6ffdd
+0,         31,         31,        1,   460800, 0eb19cdd1e9029aa5d974267efcab083
+0,         32,         32,        1,   460800, 9ca0c86309fb33ae98c76e5736c9101c
+0,         33,         33,        1,   460800, c4adb0d6c303969cb4dd721bcb626f89
+0,         34,         34,        1,   460800, 1eaf768d6b4b9fd67ff32da1e4a204af
+0,         35,         35,        1,   460800, 2b5112b639711ae561e43c17f20df51c
+0,         36,         36,        1,   460800, f90526f5eecb3458fa7cb171d50207ff
+0,         37,         37,        1,   460800, 7d51cee34cd6273b5a319d0bfe4a749b
+0,         38,         38,        1,   460800, 63e55f48bd5de98be1302aafd8210c18
+0,         39,         39,        1,   460800, 176e8c8497d21f30d5a9c74e12503ddd
+0,         40,         40,        1,   460800, eb7f75e02200a6d5e4a815762e621699
+0,         41,         41,        1,   460800, a7768ce2fe79f3c8209a8eedcc9f9bea
+0,         42,         42,        1,   460800, 7279e5439354cac6b6742e9b5ff019b5
+0,         43,         43,        1,   460800, a1e8807e2c831c3005eee641b4766b89
+0,         44,         44,        1,   460800, e5b377763bd638d3104775b32f8dcd1c
+0,         45,         45,        1,   460800, d37fc113aedb5c7d0e12b2cfdebc344f
+0,         46,         46,        1,   460800, ae595183aeafa259568d352b61106d7f
+0,         47,         47,        1,   460800, e5710c9af7febb4861fd139d74513c87