From patchwork Fri Feb 10 00:03:57 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Matthew Wolenetz X-Patchwork-Id: 2471 Delivered-To: ffmpegpatchwork@gmail.com Received: by 10.103.89.21 with SMTP id n21csp242683vsb; Thu, 9 Feb 2017 16:04:51 -0800 (PST) X-Received: by 10.223.174.183 with SMTP id y52mr5531159wrc.112.1486685091478; Thu, 09 Feb 2017 16:04:51 -0800 (PST) Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org. [79.124.17.100]) by mx.google.com with ESMTP id h5si362862wmf.41.2017.02.09.16.04.50; Thu, 09 Feb 2017 16:04:51 -0800 (PST) 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=@chromium.org; 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; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=chromium.org Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 75FF8689B5B; Fri, 10 Feb 2017 02:04:43 +0200 (EET) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wr0-f171.google.com (mail-wr0-f171.google.com [209.85.128.171]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id DECE86899EB for ; Fri, 10 Feb 2017 02:04:36 +0200 (EET) Received: by mail-wr0-f171.google.com with SMTP id k90so95430126wrc.3 for ; Thu, 09 Feb 2017 16:04:40 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=mime-version:reply-to:in-reply-to:references:from:date:message-id :subject:to; bh=lPTjoU6hHACmUi8kGGpjwOuX408fzXGkrpXA39rHhbk=; b=n+VoB4O8Jnm03fLzboX3fAOtc8lv9pIszHPYmb8lF73TFWA7Ipb4qooFyO7qq1jzgs Q7cSPNuu1/oVISkY01bb6PnCT3+yyFdImBSwk7N16V+Zym2zBJSOoFpq3Gj+jr931XvU YAgCe8Ik3T+9880vPCIypfJ0V2wijL5Ryas+A= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:reply-to:in-reply-to:references :from:date:message-id:subject:to; bh=lPTjoU6hHACmUi8kGGpjwOuX408fzXGkrpXA39rHhbk=; b=E797RsDyAj+OYtgaw8iRY77Rd/kQGXxw5fqm1oqQhm2kiMlcT0a8o4q5AWGcE560xl f1kE/0otBSLhD4lf17lvXVZFsrx08ahNtL8kti7CVtO+24KMMIr1yo62UqY9cr/RnKFN xcwI8FxY1UpJwPNX6mO+iLdVx1hLvBvLNubAFHXzFP3P0iMdIeDm6g0jIaR/jWLdT/v9 8B39VxwAJPGFFyyQdM5pWBFZFnJCsp1+nMDyFEtLLXTH3ntxL2isky53rCF8XjnblPA0 E9mZA8HZcSn4MmHOssvDwY6T8QQdnlXQjRHgylikRGFw2SkLi8D1WiuZgCFQwaPLAxtH 8rNQ== X-Gm-Message-State: AMke39kBElc0dI2RWveoI7BbQKKUnts9RqJytZNStK9MNHxny0Dmrr38/kj3eON5wG+iKn/i X-Received: by 10.223.151.53 with SMTP id r50mr4757620wrb.129.1486685079471; Thu, 09 Feb 2017 16:04:39 -0800 (PST) Received: from mail-wr0-f170.google.com (mail-wr0-f170.google.com. [209.85.128.170]) by smtp.gmail.com with ESMTPSA id v67sm20819049wrc.45.2017.02.09.16.04.38 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 09 Feb 2017 16:04:38 -0800 (PST) Received: by mail-wr0-f170.google.com with SMTP id k90so95429751wrc.3 for ; Thu, 09 Feb 2017 16:04:38 -0800 (PST) X-Received: by 10.223.143.45 with SMTP id p42mr4773458wrb.120.1486685077968; Thu, 09 Feb 2017 16:04:37 -0800 (PST) MIME-Version: 1.0 Received: by 10.28.74.220 with HTTP; Thu, 9 Feb 2017 16:03:57 -0800 (PST) In-Reply-To: References: <0297849e-fd4c-6d2a-2127-3f15b91222f2@googlemail.com> From: Matthew Wolenetz Date: Thu, 9 Feb 2017 16:03:57 -0800 X-Gmail-Original-Message-ID: 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: Avoid heap allocation wraps and OOB in mov_read_{senc, saiz, udta_string}() 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" I've separated and updated the mov_read_{senc,saiz}() patch, attached. It avoids allocation wraps in those two functions. On Wed, Feb 8, 2017 at 3:48 PM, Matthew Wolenetz wrote: > I've separated and updated the mov_read_udta_string() patch, attached. > It prevents accessing MOVContext.meta_keys[0] in that method. That array > is 1-based. > > On Wed, Dec 14, 2016 at 5:40 PM, Andreas Cadhalpun < > andreas.cadhalpun@googlemail.com> wrote: > >> On 15.12.2016 00:37, Matthew Wolenetz wrote: >> > From 8622f9398e7c89a664c4c2ceff9d35b89ff17bb5 Mon Sep 17 00:00:00 2001 >> > From: Matt Wolenetz >> > Date: Tue, 6 Dec 2016 12:54:23 -0800 >> > Subject: [PATCH] lavf/mov.c: Avoid heap allocation wraps and OOB in >> > mov_read_{senc,saiz,udta_string}() >> > >> > Core of patch is from paul@paulmehta.com >> > Reference https://crbug.com/643952 >> > --- >> > libavformat/mov.c | 11 ++++++++--- >> > 1 file changed, 8 insertions(+), 3 deletions(-) >> > >> > diff --git a/libavformat/mov.c b/libavformat/mov.c >> > index e506d20..87ad91a 100644 >> > --- a/libavformat/mov.c >> > +++ b/libavformat/mov.c >> > @@ -404,7 +404,7 @@ retry: >> > return ret; >> > } else if (!key && c->found_hdlr_mdta && c->meta_keys) { >> > uint32_t index = AV_RB32(&atom.type); >> > - if (index < c->meta_keys_count) { >> > + if (index < c->meta_keys_count && index > 0) { >> >> This should be in a separate patch. >> >> > key = c->meta_keys[index]; >> > } else { >> > av_log(c->fc, AV_LOG_WARNING, >> > @@ -4502,8 +4502,8 @@ static int mov_read_senc(MOVContext *c, >> AVIOContext *pb, MOVAtom atom) >> > >> > avio_rb32(pb); /* entries */ >> > >> > - if (atom.size < 8) { >> > - av_log(c->fc, AV_LOG_ERROR, "senc atom size %"PRId64" too >> small\n", atom.size); >> > + if (atom.size < 8 || atom.size > UINT_MAX) { >> > + av_log(c->fc, AV_LOG_ERROR, "senc atom size %"PRId64" >> invalid\n", atom.size); >> > return AVERROR_INVALIDDATA; >> > } >> > >> > @@ -4571,6 +4571,11 @@ static int mov_read_saiz(MOVContext *c, >> AVIOContext *pb, MOVAtom atom) >> > return 0; >> > } >> > >> > + if (atom.size > UINT_MAX) { >> > + av_log(c->fc, AV_LOG_ERROR, "saiz atom auxiliary_info_sizes >> size %"PRId64" invalid\n", atom.size); >> > + return AVERROR_INVALIDDATA; >> > + } >> > + >> > /* save the auxiliary info sizes as is */ >> > data_size = atom.size - atom_header_size; >> > >> >> And these should also check for SIZE_MAX. >> >> Best regards, >> Andreas >> _______________________________________________ >> ffmpeg-devel mailing list >> ffmpeg-devel@ffmpeg.org >> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> > > From bba91f9adf4d875814ec4e418e02316cbcf63f6f Mon Sep 17 00:00:00 2001 From: Matt Wolenetz Date: Wed, 14 Dec 2016 15:27:49 -0800 Subject: [PATCH] lavf/mov.c: Avoid heap allocation wraps in mov_read_{senc,saiz}() Core of patch is from paul@paulmehta.com Reference https://crbug.com/643952 (senc,saiz portions) Signed-off-by: Matt Wolenetz --- libavformat/mov.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/libavformat/mov.c b/libavformat/mov.c index ca49786ea2..36b65b3b08 100644 --- a/libavformat/mov.c +++ b/libavformat/mov.c @@ -4968,8 +4968,8 @@ static int mov_read_senc(MOVContext *c, AVIOContext *pb, MOVAtom atom) avio_rb32(pb); /* entries */ - if (atom.size < 8) { - av_log(c->fc, AV_LOG_ERROR, "senc atom size %"PRId64" too small\n", atom.size); + if (atom.size < 8 || atom.size > FFMIN(INT_MAX, SIZE_MAX)) { + av_log(c->fc, AV_LOG_ERROR, "senc atom size %"PRId64" invalid\n", atom.size); return AVERROR_INVALIDDATA; } @@ -5037,6 +5037,11 @@ static int mov_read_saiz(MOVContext *c, AVIOContext *pb, MOVAtom atom) return 0; } + if (atom.size > FFMIN(INT_MAX, SIZE_MAX)) { + av_log(c->fc, AV_LOG_ERROR, "saiz atom auxiliary_info_sizes size %"PRId64" invalid\n", atom.size); + return AVERROR_INVALIDDATA; + } + /* save the auxiliary info sizes as is */ data_size = atom.size - atom_header_size; -- 2.11.0.483.g087da7b7c-goog