From patchwork Thu Jun 7 20:42:51 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jacob Trimble X-Patchwork-Id: 9283 Delivered-To: ffmpegpatchwork@gmail.com Received: by 2002:a02:11c:0:0:0:0:0 with SMTP id c28-v6csp45885jad; Thu, 7 Jun 2018 13:43:12 -0700 (PDT) X-Google-Smtp-Source: ADUXVKIQ7+TwJRgLSJKcd0YZh8gpHTNhR2KejRCgmCTkHGvs4YrOh8zw99G0BOBM2gsH+dDnZz+X X-Received: by 2002:a1c:4787:: with SMTP id m7-v6mr2749898wmi.92.1528404192679; Thu, 07 Jun 2018 13:43:12 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1528404192; cv=none; d=google.com; s=arc-20160816; b=lpImDbFf+FS15m1sT1Bh/koVWy2ltr8/a1q99hG3XMA0NmFBdl72bM2pLDKm1CLpJ+ 2QQQc1COzIV6ENopGJOQiPUMiGn/2I1hBrFyXTz2legXzKGJyIHnMuLceA7gTjsSSHoO 6ycKvtMElbjicIKAN5ak4SZah+jGa19s5ux9Q7GM8kuZEmmEdsL3C2wnmGcSYsN2B7xT qqodbZ4QLupo7DbUntKUqrQiSiFgn3XgT4nijWiH2oFWVTkt+S9TIM7xFWrQpHJG+7GR UE3Dc6qXLZRraDys3TrqyAVybV9jw3ZSxpjm6He65hO6gIi0EgzenpzFkdRppkMay3iu ToMw== 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:in-reply-to:references:mime-version :dkim-signature:delivered-to:arc-authentication-results; bh=xj91huAC0nsG02H0AVaBoVQ4FLsGICiBmkdPaWMWacA=; b=Uu9jjx7emI7CylTkb8wUyYi8F86jMUi5ivbeavF5i5uqhNyB/lV04w0kShHCtZIBXN M1J+kZJEIxYH7sqd9cNX6uT+l1c9omMxLHqa5LY6y0E/9JhBMnxLdaejRPDtd2IWdBtY INP2WPWo7JNBaY+/FUvBUlbkvVI7FA5I/wOWyBmho94bX0nloccctZ7pn1uKi6eS/4hd 3HXvX54zUKObGRL16dQY1EFixyqwcdA0bYgMvNvWpudwEYChRHaqATYTYh2s+uQRzWPC RXvaOhYtX+ILWCWroePM5Q0/txg8qe/mC2xJsqF6bPXkFvp4pPu8EjW+4KRbSyt1Fl4G yErA== ARC-Authentication-Results: i=1; mx.google.com; dkim=neutral (body hash did not verify) header.i=@google.com header.s=20161025 header.b=U45CiD+7; 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 t10-v6si3602wrm.351.2018.06.07.13.43.11; Thu, 07 Jun 2018 13:43:12 -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=U45CiD+7; 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 91B6468A5EF; Thu, 7 Jun 2018 23:42:22 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-lf0-f68.google.com (mail-lf0-f68.google.com [209.85.215.68]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 6D93368A09E for ; Thu, 7 Jun 2018 23:42:16 +0300 (EEST) Received: by mail-lf0-f68.google.com with SMTP id n3-v6so16691039lfe.12 for ; Thu, 07 Jun 2018 13:43:04 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to; bh=w5t1k89d3Ds+ZWOHa5/wLqnXgonRtH4MtptCAb+9TSQ=; b=U45CiD+7jxDzO+Yv/ydnrG9FkwXo7j8HEPRQodS4rlEQBpWU+sCoJQGhBBTrWjqDDH aaXCq/I65sM1b/6LxMbRVhIRQ9GqFM5pA4jI25yNB09VB4Pc9zZMwhfBfG4HoOusxDea RG92Xq8HEyGNvo9oDx1iNKS3LiBFtiaf+lSnM15hLCyV0fomKz7DDxQDf1r7+b2JatoY OIjGM9IiJo+xlz+PxKOudefklLqI4t5BqHRrNqkdLzPZ7OxxM+iJYCHNqz7QTtKlm9G9 NYk094gaEDHMZNWJBjBJbVuamUjmGU1yJum+RBFGqQB/9gTnXZL03Kg/VXVv1epuAlDg 9y3g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to; bh=w5t1k89d3Ds+ZWOHa5/wLqnXgonRtH4MtptCAb+9TSQ=; b=bJob5zXnz/+drjxjMfJPF5XMDS3WUBHHSSvh/calKNerD7g7R/w2p6MYGUwv1yveRq QTEghwpkyd7aM1lL3xfvXjFaTt1R/vbE5yu7hAOkKCJF54b7pSwquZ/ripMQrsCdLxBU M/bKZ7IYe1rol3epE0rSrWT2smJP7Depgp5d3Gt/y5Urhn7rg0DPJxHuF6RreYfFV5aj O0MFUq2oxrOqotelP2qfWlh1uLfQTMJFCcJ8Hm3cc/yfSHAGiW7l9XGupK+IiElXMAEQ 3Er4yG3c4LJ8KmdFUr8e6EafvjT2MJC2bsikeEBcktAt/jAv97ZyD3Fk1jFZhjnnPHf/ B6nA== X-Gm-Message-State: APt69E04ioGyw89Od1kop9O3G0zzcFUDiZcTCe1ADowzYJUHzRSFJ1iw sI3s3e9vg8Zzeu1kqt3p9iUFoBqyqNMl7tCwMy7425Ez X-Received: by 2002:a2e:5f8f:: with SMTP id x15-v6mr2487910lje.70.1528404183151; Thu, 07 Jun 2018 13:43:03 -0700 (PDT) MIME-Version: 1.0 References: <20180607173633.127204-1-modmaker@google.com> In-Reply-To: <20180607173633.127204-1-modmaker@google.com> From: Jacob Trimble Date: Thu, 7 Jun 2018 13:42:51 -0700 Message-ID: To: FFmpeg development discussions and patches Subject: Re: [FFmpeg-devel] [PATCH] avformat/mov: Fix reading saio/saiz for clear content. 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 Thu, Jun 7, 2018 at 10:38 AM Jacob Trimble wrote: > > Found by Chrome's ClusterFuzz: http://crbug.com/850389 > > Signed-off-by: Jacob Trimble > --- > libavformat/mov.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/libavformat/mov.c b/libavformat/mov.c > index 4ad19122b3..d07171b3f4 100644 > --- a/libavformat/mov.c > +++ b/libavformat/mov.c > @@ -6041,6 +6041,11 @@ static int mov_read_saiz(MOVContext *c, AVIOContext *pb, MOVAtom atom) > if (ret != 1) > return ret; > > + if (!sc->cenc.default_encrypted_sample) { > + // Didn't see a 'schm' or 'tenc' atom, so it isn't encrypted. > + return 0; > + } > + > if (encryption_index->nb_encrypted_samples) { > // This can happen if we have both saio/saiz and senc atoms. > av_log(c->fc, AV_LOG_DEBUG, "Ignoring duplicate encryption info in saiz\n"); > @@ -6095,6 +6100,11 @@ static int mov_read_saio(MOVContext *c, AVIOContext *pb, MOVAtom atom) > if (ret != 1) > return ret; > > + if (!sc->cenc.default_encrypted_sample) { > + // Didn't see a 'schm' or 'tenc' atom, so it isn't encrypted. > + return 0; > + } > + > if (encryption_index->nb_encrypted_samples) { > // This can happen if we have both saio/saiz and senc atoms. > av_log(c->fc, AV_LOG_DEBUG, "Ignoring duplicate encryption info in saio\n"); > -- > 2.17.1.1185.g55be947832-goog > Based on comments downstream, I've added error checks for the encrypted type of saio/saiz atoms. From e4185c0fd08a1baedcf81935ff0f5ac9a97eba4e Mon Sep 17 00:00:00 2001 From: Jacob Trimble Date: Thu, 7 Jun 2018 10:29:33 -0700 Subject: [PATCH] avformat/mov: Fix reading saio/saiz for clear content. This validates that the common encryption saio/saiz atoms only appear when the data is actually encrypted. This also ignores those atoms in clear content. Found by Chrome's ClusterFuzz: http://crbug.com/850389 Signed-off-by: Jacob Trimble --- libavformat/mov.c | 71 ++++++++++++++++++++++++++++++++++++----------- 1 file changed, 55 insertions(+), 16 deletions(-) diff --git a/libavformat/mov.c b/libavformat/mov.c index 4ad19122b3..2fca025889 100644 --- a/libavformat/mov.c +++ b/libavformat/mov.c @@ -6035,7 +6035,7 @@ static int mov_read_saiz(MOVContext *c, AVIOContext *pb, MOVAtom atom) MOVEncryptionIndex *encryption_index; MOVStreamContext *sc; int ret; - unsigned int sample_count; + unsigned int sample_count, aux_info_type, aux_info_param; ret = get_current_encryption_info(c, &encryption_index, &sc); if (ret != 1) @@ -6054,14 +6054,33 @@ static int mov_read_saiz(MOVContext *c, AVIOContext *pb, MOVAtom atom) avio_r8(pb); /* version */ if (avio_rb24(pb) & 0x01) { /* flags */ - if (avio_rb32(pb) != sc->cenc.default_encrypted_sample->scheme) { - av_log(c->fc, AV_LOG_DEBUG, "Ignoring saiz box with non-zero aux_info_type\n"); - return 0; - } - if (avio_rb32(pb) != 0) { - av_log(c->fc, AV_LOG_DEBUG, "Ignoring saiz box with non-zero aux_info_type_parameter\n"); - return 0; + aux_info_type = avio_rb32(pb); + aux_info_param = avio_rb32(pb); + if (sc->cenc.default_encrypted_sample) { + if (aux_info_type != sc->cenc.default_encrypted_sample->scheme) { + av_log(c->fc, AV_LOG_DEBUG, "Ignoring saiz box with non-zero aux_info_type\n"); + return 0; + } + if (aux_info_param != 0) { + av_log(c->fc, AV_LOG_DEBUG, "Ignoring saiz box with non-zero aux_info_type_parameter\n"); + return 0; + } + } else { + // Didn't see 'schm' or 'tenc', so this isn't encrypted. + if ((aux_info_type == MKBETAG('c','e','n','c') || + aux_info_type == MKBETAG('c','e','n','s') || + aux_info_type == MKBETAG('c','b','c','1') || + aux_info_type == MKBETAG('c','b','c','s')) && + aux_info_param == 0) { + av_log(c->fc, AV_LOG_ERROR, "Saw encrypted saiz without schm/tenc\n"); + return AVERROR_INVALIDDATA; + } else { + return 0; + } } + } else if (!sc->cenc.default_encrypted_sample) { + // Didn't see 'schm' or 'tenc', so this isn't encrypted. + return 0; } encryption_index->auxiliary_info_default_size = avio_r8(pb); @@ -6089,7 +6108,8 @@ static int mov_read_saio(MOVContext *c, AVIOContext *pb, MOVAtom atom) MOVEncryptionIndex *encryption_index; MOVStreamContext *sc; int i, ret; - unsigned int version, entry_count, alloc_size = 0; + unsigned int version, entry_count, aux_info_type, aux_info_param; + unsigned int alloc_size = 0; ret = get_current_encryption_info(c, &encryption_index, &sc); if (ret != 1) @@ -6108,14 +6128,33 @@ static int mov_read_saio(MOVContext *c, AVIOContext *pb, MOVAtom atom) version = avio_r8(pb); /* version */ if (avio_rb24(pb) & 0x01) { /* flags */ - if (avio_rb32(pb) != sc->cenc.default_encrypted_sample->scheme) { - av_log(c->fc, AV_LOG_DEBUG, "Ignoring saio box with non-zero aux_info_type\n"); - return 0; - } - if (avio_rb32(pb) != 0) { - av_log(c->fc, AV_LOG_DEBUG, "Ignoring saio box with non-zero aux_info_type_parameter\n"); - return 0; + aux_info_type = avio_rb32(pb); + aux_info_param = avio_rb32(pb); + if (sc->cenc.default_encrypted_sample) { + if (aux_info_type != sc->cenc.default_encrypted_sample->scheme) { + av_log(c->fc, AV_LOG_DEBUG, "Ignoring saio box with non-zero aux_info_type\n"); + return 0; + } + if (aux_info_param != 0) { + av_log(c->fc, AV_LOG_DEBUG, "Ignoring saio box with non-zero aux_info_type_parameter\n"); + return 0; + } + } else { + // Didn't see 'schm' or 'tenc', so this isn't encrypted. + if ((aux_info_type == MKBETAG('c','e','n','c') || + aux_info_type == MKBETAG('c','e','n','s') || + aux_info_type == MKBETAG('c','b','c','1') || + aux_info_type == MKBETAG('c','b','c','s')) && + aux_info_param == 0) { + av_log(c->fc, AV_LOG_ERROR, "Saw encrypted saio without schm/tenc\n"); + return AVERROR_INVALIDDATA; + } else { + return 0; + } } + } else if (!sc->cenc.default_encrypted_sample) { + // Didn't see 'schm' or 'tenc', so this isn't encrypted. + return 0; } entry_count = avio_rb32(pb); -- 2.17.1.1185.g55be947832-goog