From patchwork Sat Nov 19 23:37:05 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Cadhalpun X-Patchwork-Id: 1491 Delivered-To: ffmpegpatchwork@gmail.com Received: by 10.103.90.1 with SMTP id o1csp849277vsb; Sat, 19 Nov 2016 15:37:16 -0800 (PST) X-Received: by 10.28.69.217 with SMTP id l86mr5381532wmi.129.1479598636688; Sat, 19 Nov 2016 15:37:16 -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 f127si7631883wmf.124.2016.11.19.15.37.16; Sat, 19 Nov 2016 15:37:16 -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 26D2E689CD7; Sun, 20 Nov 2016 01:37:12 +0200 (EET) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wm0-f41.google.com (mail-wm0-f41.google.com [74.125.82.41]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 8984A689A66 for ; Sun, 20 Nov 2016 01:37:05 +0200 (EET) Received: by mail-wm0-f41.google.com with SMTP id g23so91230169wme.1 for ; Sat, 19 Nov 2016 15:37:07 -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=k3kEPEzsgfDdphNOhFashSCy1YLRE1emQQUEFBrjQkI=; b=eVUGz/d7upBtuJAiX3TUUetsOql0Jsp8bjXVUlcSTup9RFesDkxFfF7wGwuR8+MwA6 SNYWgl/LuzEOLb5VRBLz6VI+lzGvxpBXFbGF7AvjDwIXBizM5TnFuAtDlz5BvEho14oG y1chRk0oyCBAqi1PeE0lTTB9TUy+6g8RVk7O6uuxqT1qE40DOXjATXSWmiBgj9eTpDRR jtkc+du/oeGLIlMVm2PJ3Y1n+w0KDuO2sXos9QwdK5q1pkbdzSJYE6389sOFYjVy23rM 4AsYA3sG+6X//uuOdFQ4Gdkr3zK2QTheXv+0+qcULN14lhq5dtqBijvi2H0PgLToZjVb pMdA== 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=k3kEPEzsgfDdphNOhFashSCy1YLRE1emQQUEFBrjQkI=; b=WIQFCSk33hvNzjGGnt2QrGv+6/x8UP0rK7+kmnyr0s955oemL0jTbeUMq7p75SN20W ki3ZHnSEnXzHe1OKppEcGnn31RUkE0QPfmA7TsS+EP5PKsHtQfHrllWw1LPzG2xOEacw BCKapKv1/u9yMNrxAllsaP2Fzru7mY3hu6SXCvKc9CrT0Ndy6nS1I4/rLYPOm/sncysB oVgmrML2nbMdwAGPErsg8CbiUUFcrPQmKEmRhVOoXBNDLvFl8RYz2p3G7ZSLdT7asy0T Vd5X+bWqPC7ytDwYX01h2l3uiMNWYc9gUIP6w3XIRpEU1AAhIz0PfyuUwjeio9vdWd1c orbw== X-Gm-Message-State: AKaTC00NgX7fa1Fxr9RLLvKefrc7ODOedjq++XLR6HiTqKAD04Fz2WJxLJ+dv5VxQYYWQQ== X-Received: by 10.28.161.129 with SMTP id k123mr6517695wme.66.1479598627025; Sat, 19 Nov 2016 15:37:07 -0800 (PST) Received: from [192.168.2.21] (p5B095CF1.dip0.t-ipconnect.de. [91.9.92.241]) by smtp.googlemail.com with ESMTPSA id b7sm16530606wjm.39.2016.11.19.15.37.06 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sat, 19 Nov 2016 15:37:06 -0800 (PST) From: Andreas Cadhalpun X-Google-Original-From: Andreas Cadhalpun To: ffmpeg-devel@ffmpeg.org References: <6d786760-6b84-d460-8928-d2a5331d8a60@googlemail.com> <20161117012630.GP4824@nb4> <8cef9df2-9cc2-663f-bdbe-21484c1d1823@googlemail.com> <20161118014057.GX4824@nb4> <6ad11546-7279-977b-097d-49e84f7a14f5@googlemail.com> <20161119011433.GM4824@nb4> <54f9a7a9-4917-86aa-bd91-4872552e8282@googlemail.com> <20161119152816.GQ4824@nb4> <2bdcb4b4-d384-b727-90b2-e39ea28d50f0@googlemail.com> <20161119213644.GS4824@nb4> Message-ID: Date: Sun, 20 Nov 2016 00:37:05 +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: <20161119213644.GS4824@nb4> Subject: Re: [FFmpeg-devel] [PATCH] ffmdec: sanitize codec parameters 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 19.11.2016 22:36, Michael Niedermayer wrote: > On Sat, Nov 19, 2016 at 05:30:59PM +0100, Andreas Cadhalpun wrote: >> A hard failure is unnecessary and Carl Eugen [1] and Hendrik [2] >> convinced me that it should be avoided. > > I think there are 2 different things here > unavailable data, like a bitrate of 0 > and > wrong data like a negative bitrate > > We do not accept wrong values in general, If you say so. I'm more interested in fixing the crash than continuing this discussion, so I changed it to always error out. See attached patch. > ffm files are AFAIK generally temporary (on disk fifo) files generated > by the current muxer. If that's the case, why does the demuxer still support FFM1 even though the muxer can't create such files anymore? Best regards, Andreas From d3f33b108fe7035e1aa1bc7021dbb99aecd14821 Mon Sep 17 00:00:00 2001 From: Andreas Cadhalpun Date: Thu, 17 Nov 2016 00:04:57 +0100 Subject: [PATCH] ffmdec: validate codec parameters A negative extradata size for example gets passed to memcpy in avcodec_parameters_from_context causing a segmentation fault. Signed-off-by: Andreas Cadhalpun --- libavformat/ffmdec.c | 52 ++++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 42 insertions(+), 10 deletions(-) diff --git a/libavformat/ffmdec.c b/libavformat/ffmdec.c index 6b09be2..960e793 100644 --- a/libavformat/ffmdec.c +++ b/libavformat/ffmdec.c @@ -21,6 +21,7 @@ #include +#include "libavutil/imgutils.h" #include "libavutil/internal.h" #include "libavutil/intreadwrite.h" #include "libavutil/intfloat.h" @@ -28,6 +29,7 @@ #include "libavutil/avassert.h" #include "libavutil/avstring.h" #include "libavutil/pixdesc.h" +#include "libavcodec/internal.h" #include "avformat.h" #include "internal.h" #include "ffm.h" @@ -277,6 +279,14 @@ static int ffm_append_recommended_configuration(AVStream *st, char **conf) return 0; } +#define VALIDATE_PARAMETER(parameter, name, check) { \ + if (check) { \ + av_log(codec, AV_LOG_ERROR, "Invalid " name " %d\n", codec->parameter); \ + ret = AVERROR_INVALIDDATA; \ + goto fail; \ + } \ +} + static int ffm2_read_header(AVFormatContext *s) { FFMContext *ffm = s->priv_data; @@ -342,6 +352,7 @@ static int ffm2_read_header(AVFormatContext *s) if (!codec_desc) { av_log(s, AV_LOG_ERROR, "Invalid codec id: %d\n", codec->codec_id); codec->codec_id = AV_CODEC_ID_NONE; + ret = AVERROR_INVALIDDATA; goto fail; } codec->codec_type = avio_r8(pb); @@ -350,14 +361,25 @@ static int ffm2_read_header(AVFormatContext *s) codec_desc->type, codec->codec_type); codec->codec_id = AV_CODEC_ID_NONE; codec->codec_type = AVMEDIA_TYPE_UNKNOWN; + ret = AVERROR_INVALIDDATA; goto fail; } codec->bit_rate = avio_rb32(pb); + if (codec->bit_rate < 0) { + av_log(codec, AV_LOG_ERROR, "Invalid bit rate %"PRId64"\n", codec->bit_rate); + ret = AVERROR_INVALIDDATA; + goto fail; + } codec->flags = avio_rb32(pb); codec->flags2 = avio_rb32(pb); codec->debug = avio_rb32(pb); if (codec->flags & AV_CODEC_FLAG_GLOBAL_HEADER) { int size = avio_rb32(pb); + if (size < 0 || size >= FF_MAX_EXTRADATA_SIZE) { + av_log(s, AV_LOG_ERROR, "Invalid extradata size %d\n", size); + ret = AVERROR_INVALIDDATA; + goto fail; + } codec->extradata = av_mallocz(size + AV_INPUT_BUFFER_PADDING_SIZE); if (!codec->extradata) return AVERROR(ENOMEM); @@ -380,6 +402,9 @@ static int ffm2_read_header(AVFormatContext *s) } codec->width = avio_rb16(pb); codec->height = avio_rb16(pb); + ret = av_image_check_size(codec->width, codec->height, 0, s); + if (ret < 0) + goto fail; codec->gop_size = avio_rb16(pb); codec->pix_fmt = avio_rb32(pb); if (!av_pix_fmt_desc_get(codec->pix_fmt)) { @@ -432,13 +457,11 @@ static int ffm2_read_header(AVFormatContext *s) goto fail; } codec->sample_rate = avio_rb32(pb); - if (codec->sample_rate <= 0) { - av_log(s, AV_LOG_ERROR, "Invalid sample rate %d\n", codec->sample_rate); - ret = AVERROR_INVALIDDATA; - goto fail; - } + VALIDATE_PARAMETER(sample_rate, "sample rate", codec->sample_rate < 0) codec->channels = avio_rl16(pb); + VALIDATE_PARAMETER(channels, "number of channels", codec->channels < 0) codec->frame_size = avio_rl16(pb); + VALIDATE_PARAMETER(frame_size, "frame size", codec->frame_size < 0) break; case MKBETAG('C', 'P', 'R', 'V'): if (f_cprv++) { @@ -518,7 +541,7 @@ static int ffm_read_header(AVFormatContext *s) AVIOContext *pb = s->pb; AVCodecContext *codec; const AVCodecDescriptor *codec_desc; - int i, nb_streams; + int i, nb_streams, ret; uint32_t tag; /* header */ @@ -570,6 +593,10 @@ static int ffm_read_header(AVFormatContext *s) goto fail; } codec->bit_rate = avio_rb32(pb); + if (codec->bit_rate < 0) { + av_log(codec, AV_LOG_WARNING, "Invalid bit rate %"PRId64"\n", codec->bit_rate); + goto fail; + } codec->flags = avio_rb32(pb); codec->flags2 = avio_rb32(pb); codec->debug = avio_rb32(pb); @@ -585,6 +612,8 @@ static int ffm_read_header(AVFormatContext *s) } codec->width = avio_rb16(pb); codec->height = avio_rb16(pb); + if (av_image_check_size(codec->width, codec->height, 0, s) < 0) + goto fail; codec->gop_size = avio_rb16(pb); codec->pix_fmt = avio_rb32(pb); if (!av_pix_fmt_desc_get(codec->pix_fmt)) { @@ -633,18 +662,21 @@ static int ffm_read_header(AVFormatContext *s) break; case AVMEDIA_TYPE_AUDIO: codec->sample_rate = avio_rb32(pb); - if (codec->sample_rate <= 0) { - av_log(s, AV_LOG_ERROR, "Invalid sample rate %d\n", codec->sample_rate); - goto fail; - } + VALIDATE_PARAMETER(sample_rate, "sample rate", codec->sample_rate < 0) codec->channels = avio_rl16(pb); + VALIDATE_PARAMETER(channels, "number of channels", codec->channels < 0) codec->frame_size = avio_rl16(pb); + VALIDATE_PARAMETER(frame_size, "frame size", codec->frame_size < 0) break; default: goto fail; } if (codec->flags & AV_CODEC_FLAG_GLOBAL_HEADER) { int size = avio_rb32(pb); + if (size < 0 || size >= FF_MAX_EXTRADATA_SIZE) { + av_log(s, AV_LOG_ERROR, "Invalid extradata size %d\n", size); + goto fail; + } codec->extradata = av_mallocz(size + AV_INPUT_BUFFER_PADDING_SIZE); if (!codec->extradata) return AVERROR(ENOMEM); -- 2.10.2