From patchwork Mon Nov 14 20:55:15 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Cadhalpun X-Patchwork-Id: 1423 Delivered-To: ffmpegpatchwork@gmail.com Received: by 10.103.90.1 with SMTP id o1csp1245158vsb; Mon, 14 Nov 2016 12:55:27 -0800 (PST) X-Received: by 10.194.62.178 with SMTP id z18mr24368754wjr.20.1479156927271; Mon, 14 Nov 2016 12:55:27 -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 l74si246072wmg.40.2016.11.14.12.55.26; Mon, 14 Nov 2016 12:55:27 -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=@googlemail.com; 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=QUARANTINE dis=NONE) header.from=googlemail.com Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 0078C689CB2; Mon, 14 Nov 2016 22:55:25 +0200 (EET) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wm0-f68.google.com (mail-wm0-f68.google.com [74.125.82.68]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 6040D689222 for ; Mon, 14 Nov 2016 22:55:19 +0200 (EET) Received: by mail-wm0-f68.google.com with SMTP id g23so19022217wme.1 for ; Mon, 14 Nov 2016 12:55:19 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=googlemail.com; s=20120113; h=from:subject:to:references:message-id:date:user-agent:mime-version :in-reply-to; bh=McZp+tS3Jb0MBgcwz3Hzmh3mJHrrQc8b3fKltKLw//A=; b=t++CJTqxekCt8ioWvJfegMpFXYSkbkYeZQPHZgoS2f8dVKiy0RmZ0b4jcCr5BqNZVf WX9f6jmmZ/obUuO4m6REyDXk4ngDTJ/GfmS6gqhdMKhjZESFtqtQrnJcBx/ZL29oyehL QKcRfuHhPCp5VHyQYXAHAkf420xXU/KOPWhf+f9qApjCdS4QNkzUOJj4pXlOGkFg57k7 VWHlekFtDDYjiTBQpTxcnJFIW5dGs3yRxDwEjaAcS314bSYacMRp9E0Yzaau4INvfVAh lk2On2dgBjxBHY5DBR2vZfanh6n8SPLsQ1h5dV08kkWgeteJJCzrZKcJvMGwchL/dw4w B2mw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:subject:to:references:message-id:date :user-agent:mime-version:in-reply-to; bh=McZp+tS3Jb0MBgcwz3Hzmh3mJHrrQc8b3fKltKLw//A=; b=gjg7ptfasw64j1KKQpPIs+maWYrfi2OVErc+qZ2Em3ar2/BQdIRi/S7ofvFg1+G4vK sbVk8yiJhP9fPyASbn+9XfpAScrfLVpmUtEt6D7r0OaG+/KGmQb8NpO77wJ3abAN9cu8 xk8yeYRPKN0xHzkDMEx7sW38j6gjYEyzP72rq0yokR+a7RG17dXnNzDog0wrO92DOqeX G2DPX/LuHnM2L3Uqbj+HIKZqitEOaXf69HXFmj8/3mt+B7N3Ml1QcEQqYsI9AwXQ8Z6C LRvejQm42ASHsdAGQ0QbG9tpI0bI9j24f5Cq9Q+2cZCT2k2nd+X671LS+HopD7hvsAL4 +EUA== X-Gm-Message-State: ABUngvcEQbE/9EdvtFGEFfR3yKUS+0P8YGhlyUC15Pa0O2kUgyirIqlln554XdfnzXtb+w== X-Received: by 10.194.231.8 with SMTP id tc8mr20383988wjc.193.1479156918810; Mon, 14 Nov 2016 12:55:18 -0800 (PST) Received: from [192.168.2.21] (p5B0959A3.dip0.t-ipconnect.de. [91.9.89.163]) by smtp.googlemail.com with ESMTPSA id v2sm30702886wja.41.2016.11.14.12.55.16 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 14 Nov 2016 12:55:17 -0800 (PST) From: Andreas Cadhalpun X-Google-Original-From: Andreas Cadhalpun To: libav-devel@libav.org, FFmpeg development discussions and patches References: <65a476e6-92fb-667f-7d38-65bd40756892@gentoo.org> <78469d02-ce34-0614-e81f-fe663e7bf9f0@googlemail.com> <20161114195416.24597.66443@localhost> Message-ID: <19b88f10-14fc-4b3b-2d0f-ad60a86f6de0@googlemail.com> Date: Mon, 14 Nov 2016 21:55:15 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Icedove/45.4.0 MIME-Version: 1.0 In-Reply-To: <20161114195416.24597.66443@localhost> Subject: Re: [FFmpeg-devel] [libav-devel] [PATCH] libopusdec: fix out-of-bounds read 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 14.11.2016 20:54, Anton Khirnov wrote: > Quoting Andreas Cadhalpun (2016-11-14 20:30:10) >> On 14.11.2016 00:01, Luca Barbato wrote: >>> On 13/11/2016 19:23, Andreas Cadhalpun wrote: >>>> avc->channels can be 0. >>> >>> 0 and less than zero shouldn't be an error? >> >> Such values should be rejected, wherever they are set. >> However, ensuring that is a larger change I'm currently >> working on. >> Meanwhile, this patch is a trivial fix for the potential >> security problem that can easily be backported. > > channels being zero is perfectly valid, it means the caller does not > know the channel count and expects the decoder to read it from the > bitstream. In general code this is correct, however if e.g. the matroska demuxer reads an audio stream which claims to have 0 channels, it should be rejected as broken. > This should fail for codecs that do not store this > information in the bitstream, but work fine otherwise. > > In the case of opus, the channel count is always known -- when the > extradata is present, the channel count is stored there. Otherwise the > stream is simple and can be decoded either as mono or stereo, as we > want. > > The patch does not seem to be doing the right thing -- I think it will > simply fail on the opus_multistream_decoder_create() call. Correct. > What it should do instead is just default to stereo. OK, patch doing that is attached. > Even better, you could replace the whole extradata parsing block with > a call to ff_opus_parse_extradata(), though that would require some > refactoring. The fix should be easily backportable, which excludes refactoring. Best regards, Andreas From d33ded293d15e8ceab666bea834d436f3a225bcc Mon Sep 17 00:00:00 2001 From: Andreas Cadhalpun Date: Mon, 14 Nov 2016 21:41:45 +0100 Subject: [PATCH] libopusdec: default to stereo for invalid number of channels This fixes an out-of-bounds read if avc->channels is 0. Signed-off-by: Andreas Cadhalpun --- libavcodec/libopusdec.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/libavcodec/libopusdec.c b/libavcodec/libopusdec.c index acc62f1..61f68ed 100644 --- a/libavcodec/libopusdec.c +++ b/libavcodec/libopusdec.c @@ -47,6 +47,12 @@ static av_cold int libopus_decode_init(AVCodecContext *avc) int ret, channel_map = 0, gain_db = 0, nb_streams, nb_coupled; uint8_t mapping_arr[8] = { 0, 1 }, *mapping; + if (avc->channels <= 0) { + av_log(avc, AV_LOG_WARNING, + "Invalid number of channels %d, defaulting to stereo\n", avc->channels); + avc->channels = 2; + } + avc->sample_rate = 48000; avc->sample_fmt = avc->request_sample_fmt == AV_SAMPLE_FMT_FLT ? AV_SAMPLE_FMT_FLT : AV_SAMPLE_FMT_S16; -- 2.10.2