From patchwork Fri Oct 27 03:51:50 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sasi Inguva X-Patchwork-Id: 5712 Delivered-To: ffmpegpatchwork@gmail.com Received: by 10.2.161.90 with SMTP id m26csp250547jah; Thu, 26 Oct 2017 20:57:21 -0700 (PDT) X-Google-Smtp-Source: ABhQp+SihAp/iaxz0uQRR4iS+VgQ90hpryCsSurhd6t41gElJw5ZsJAXgI8m2LrolB2p0TTrFJdr X-Received: by 10.28.174.78 with SMTP id x75mr777497wme.27.1509076641208; Thu, 26 Oct 2017 20:57:21 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1509076641; cv=none; d=google.com; s=arc-20160816; b=FqWj3XgUJio7FzJBXZJFR4TgRLnUisKLMcvF2jKE8mKMFU7ZMfKatnnUFLovpBpQI/ dWEL/yXiy8eF62aNzZQW5n9HaZIfPgZxy2cZVd/64U0GQePQD/IZka6jFHfslWZe37VA /7hgBJgsDwS+PCAGBI3CMEz9tiqR1RKqh0yAgNblf7GWzupbj0onDFz3r58bQt/0dPiy F8SgzU9pm1WgsyLF3oZ4QNSDl3kScB4IuETkkK3P29lzjnvK60prqkbpIsYDcWJdtkUC e60rFxMhHbUYJLJ3oVVBh2iAijSOVG3SjikFvZVFimzuJ6xDvIcRqfbe3RX3LDG3B2UH OixA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:reply-to:list-subscribe:list-help:list-post :list-archive:list-unsubscribe:list-id:precedence:subject:to :message-id:date:from:references:in-reply-to:mime-version :dkim-signature:delivered-to:arc-authentication-results; bh=3TyDyorAfCnuhs5JRzrcPgi6ztDUM+SHBtXXBLnPgHg=; b=XlVUZJRmSuimENCcNqEiJZzzTStM1iuV+dVlWaxXUF9nFARKWIbxgAEBz8RIColkN0 rfbrEnPeEeziHIo86prlw6aWIiF4LoRXdI6ef4UH9URAhAyP/NyzdQ9qjTzRlSz3RbLW 4je/ZZTQZ0xu7aCk6WZRa7GBMqdToX9HZowwNK2DOAp9c8xYOMB7fPhwk5MD9IbrJaXF h/XcDQI8MKnXdMs93RjmJzij1RbfNlA25KsdvuDjsKGPd/lBiRrL/iygDnGo4mKFn6B9 3FbLoVmjDJnCRDMO5xp2AkEz5tOlXgwJnRyDrzu7QlwuWkKKES9Zi3BwKBqnsFyn3a8P ES0g== ARC-Authentication-Results: i=1; mx.google.com; dkim=neutral (body hash did not verify) header.i=@google.com header.s=20161025 header.b=FxilSrE5; spf=pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) smtp.mailfrom=ffmpeg-devel-bounces@ffmpeg.org Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org. [79.124.17.100]) by mx.google.com with ESMTP id k129si553722wmd.246.2017.10.26.20.57.20; Thu, 26 Oct 2017 20:57:21 -0700 (PDT) Received-SPF: pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) client-ip=79.124.17.100; Authentication-Results: mx.google.com; dkim=neutral (body hash did not verify) header.i=@google.com header.s=20161025 header.b=FxilSrE5; spf=pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) smtp.mailfrom=ffmpeg-devel-bounces@ffmpeg.org Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id DEDE768A2A8; Fri, 27 Oct 2017 06:57:10 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-vk0-f47.google.com (mail-vk0-f47.google.com [209.85.213.47]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id E564C68A1E6 for ; Fri, 27 Oct 2017 06:57:03 +0300 (EEST) Received: by mail-vk0-f47.google.com with SMTP id h142so25452vkf.7 for ; Thu, 26 Oct 2017 20:57:12 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:in-reply-to:references:from:date:message-id:subject:to; bh=ncrNS8WPm2eoRaH7dT722hyi9u8cz3ffrSyQ5t0BK+M=; b=FxilSrE5bmB6BeTWlJLXy0oA3V1gMy8RRlEPSXoZOXU19PU8LYxU+ib+ho61AQ7qK+ lrRGwAzQCxjzpv79ZgNmDg9GQXxYhmtNEvpAEhH3qRnWsxUMgEUHSgCzPvyKifr6j+ZG Gzyrf6zBVJQl4QGBIYe7baDqLVNVkE8uBquNfI6/oIvQJKi5aAc1W5GHNiqTsd15t21O Uum7grxWX4NFbGsJffn+H8U9Xp7jPXBAPM+eC8NAhRGepMmOOiGA51ax7i4aYiRvYWA3 ZDZdYlDDUwkT7uCPnbwpIPT0+RWsGvt7ek7nvelUgNCFNZ+s3GU2Puzd7FXYtY+kieK1 DwQg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to; bh=ncrNS8WPm2eoRaH7dT722hyi9u8cz3ffrSyQ5t0BK+M=; b=HwHYu7DMDz0mJWprrvj1C/rUwORDQW3W+2sjtOvmweVnXwUeAg0uG7YVH/CODIjC3L umsLAMLOmnqAYaIjpiNJ85JAAneCc5gAo4zQsWYKXwnBaThsZOWyMBQrdVb0IGzz9hWl XnxzBkVX0mmqn/XLPyanEWBMLXKT2Et3pHhxfLLYh8vFgaxbPDdTk0IztfCFc08695cr XAWvfWY3Y+4KrGjoNYte1jnfKLxeEzCnzqMmDZxSUEzsnT2N37Bw64fNXeIBOxUmB1cK 4V1Ltsq+8SqQOiZg6bnuNqqDB6Jw9KxTa/FU6au2MeQR+UACNJTEtAUq88tjaImF9bc5 F2Tw== X-Gm-Message-State: AMCzsaW3s4JEgvIhakoO2ZgOl2F0D6RSzUv9CB4EyEIwjaKXGkKOc8Ka MnLLBzpeH9DWK1XdVQrmF5YNGpOyacmW2Zyi6iGjFryv X-Received: by 10.31.108.209 with SMTP id j78mr5470248vki.129.1509076311057; Thu, 26 Oct 2017 20:51:51 -0700 (PDT) MIME-Version: 1.0 Received: by 10.31.47.23 with HTTP; Thu, 26 Oct 2017 20:51:50 -0700 (PDT) In-Reply-To: <20171024214835.GL6009@nb4> References: <20171020232635.GN6009@nb4> <20171023231828.22216-1-isasi@google.com> <20171024214835.GL6009@nb4> From: Sasi Inguva Date: Thu, 26 Oct 2017 20:51:50 -0700 Message-ID: To: FFmpeg development discussions and patches X-Content-Filtered-By: Mailman/MimeDel 2.1.20 Subject: Re: [FFmpeg-devel] [PATCH] lavf/mov.c: Fix parsing of edit list atoms with invalid elst entry count. X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" On Tue, Oct 24, 2017 at 2:48 PM, Michael Niedermayer wrote: > On Mon, Oct 23, 2017 at 04:18:28PM -0700, Sasi Inguva wrote: > > Signed-off-by: Sasi Inguva > > --- > > 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 > > From 188602e662868a79c7f85e9193e9aedc9ba1a170 Mon Sep 17 00:00:00 2001 From: Sasi Inguva 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. Signed-off-by: Sasi Inguva --- 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(-) create mode 100644 tests/ref/fate/mov-invalid-elst-entry-count diff --git a/libavformat/mov.c b/libavformat/mov.c index b22a116140..bd019317bf 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,21 @@ 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) { + if (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; + } else { + edit_count = atom.size / elst_entry_size; + if (edit_count * elst_entry_size != atom.size) { + av_log(c->fc, AV_LOG_WARNING, "ELST atom of %"PRId64" bytes, bigger than %d entries.", atom.size, edit_count); + } + } + } if (!edit_count) return 0; @@ -4617,17 +4633,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..ef250a12d8 100644 --- a/tests/fate/mov.mak +++ b/tests/fate/mov.mak @@ -6,6 +6,7 @@ FATE_MOV = fate-mov-3elist \ fate-mov-1elist-ends-last-bframe \ fate-mov-2elist-elist1-ends-bframe \ fate-mov-3elist-encrypted \ + fate-mov-invalid-elst-entry-count \ fate-mov-gpmf-remux \ FATE_MOV_FFPROBE = fate-mov-aac-2048-priming \ @@ -39,6 +40,9 @@ fate-mov-1elist-ends-last-bframe: CMD = framemd5 -i $(TARGET_SAMPLES)/mov/mov-1e # Makes sure that we handle timestamps of packets in case of multiple edit lists with one of them ending on a B-frame correctly. fate-mov-2elist-elist1-ends-bframe: CMD = framemd5 -i $(TARGET_SAMPLES)/mov/mov-2elist-elist1-ends-bframe.mov +# Makes sure that we handle invalid edit list entry count correctly. +fate-mov-invalid-elst-entry-count: CMD = framemd5 -i $(TARGET_SAMPLES)/mov/invalid_elst_entry_count.mov + fate-mov-aac-2048-priming: CMD = run ffprobe$(PROGSSUF)$(EXESUF) -show_packets -print_format compact $(TARGET_SAMPLES)/mov/aac-2048-priming.mov fate-mov-zombie: CMD = run ffprobe$(PROGSSUF)$(EXESUF) -show_streams -show_packets -show_frames -bitexact -print_format compact $(TARGET_SAMPLES)/mov/white_zombie_scrunch-part.mov diff --git a/tests/ref/fate/mov-invalid-elst-entry-count b/tests/ref/fate/mov-invalid-elst-entry-count new file mode 100644 index 0000000000..13b575816b --- /dev/null +++ b/tests/ref/fate/mov-invalid-elst-entry-count @@ -0,0 +1,57 @@ +#format: frame checksums +#version: 2 +#hash: MD5 +#tb 0: 1/24 +#media_type 0: video +#codec_id 0: rawvideo +#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 -- 2.15.0.rc2.357.g7e34df9404-goog