From patchwork Thu Nov 17 00:08:31 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Cadhalpun X-Patchwork-Id: 1450 Delivered-To: ffmpegpatchwork@gmail.com Received: by 10.103.90.1 with SMTP id o1csp478468vsb; Wed, 16 Nov 2016 16:08:43 -0800 (PST) X-Received: by 10.28.74.133 with SMTP id n5mr12651941wmi.132.1479341323123; Wed, 16 Nov 2016 16:08:43 -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 n204si9206030wmf.128.2016.11.16.16.08.42; Wed, 16 Nov 2016 16:08:43 -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 DD69E689ED4; Thu, 17 Nov 2016 02:08:39 +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 01170689E1B for ; Thu, 17 Nov 2016 02:08:32 +0200 (EET) Received: by mail-wm0-f68.google.com with SMTP id g23so16358526wme.1 for ; Wed, 16 Nov 2016 16:08:34 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=googlemail.com; s=20120113; h=from:to:subject:message-id:date:user-agent:mime-version :content-transfer-encoding; bh=WeZScgY4yUjBU2jucMVjw1WV/h7CuJrnDXmCoyzQnjE=; b=ZT8rwqKShq0qdCGehcg2M4qc7nDwCxxUs5DpR2u9rthL9Q9Zbk1iWVs6mDSEkmtS7z 3Cm4udxqzuotdotaprovyQl52JhQzMeVROg4BqQH7EX/2mA2qUzqQaCop8IL9Pex/bpI jk4y92DPqbxpMG0XHbz+mnVidqRK5e+BTtE5mlej1wLah33M29c1rNDk71UUKF5e88YB BS7ddKjoMwJgaLUez9XWPPC7olB7fCW9ylzZDGSDMFqex+s8V9gS9AmRVhEpWQ8C8t+c M99J4F6Md20b4A2KXZ4DQ99mUlReOSuHPNdnXnF9UHXVuQSGG6H3GZAQYsFpfzEdS+6z JJaw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:subject:message-id:date:user-agent :mime-version:content-transfer-encoding; bh=WeZScgY4yUjBU2jucMVjw1WV/h7CuJrnDXmCoyzQnjE=; b=Vjrsjjre8ePZ0x0lLLRuh2wTIaJFsemnnGylwhoyYFwn04cHA5/d+y0nfcIwEll5hi NH2VEM5NdGWeGysnk17kNrYjcowWhOPciH89J3qR1vsk6pMHT5em4rv+Ppy0UdBsLtEg 8GeD9STlVYLQcSQwXHgQYn14opwg+/Yudyjw85yZroPBKk+JSlWoIMvK+6UpkhBpUehL SYsX+hRoWZ9ymiElNH+fGsZ6IBQzJHId+p864I64LYNpLzXNElsQovfW/s6qtd/EzjYB vpoVnUjjAMMtW0j+c3He4oAYkzthJGSp3WRrKq2bhYrnDJYP1QpAQvMsgeA5etBIwx7F syOA== X-Gm-Message-State: ABUngvc9mA2mcntB8kUPHsMrtYc0FfdnhPNC/0GVaEOgIsFvY7Mxa+WgGLtZp9dmt9eONw== X-Received: by 10.28.226.139 with SMTP id z133mr14033203wmg.139.1479341313065; Wed, 16 Nov 2016 16:08:33 -0800 (PST) Received: from [192.168.2.21] (p5B072246.dip0.t-ipconnect.de. [91.7.34.70]) by smtp.googlemail.com with ESMTPSA id bj1sm278835wjc.17.2016.11.16.16.08.32 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 16 Nov 2016 16:08:32 -0800 (PST) From: Andreas Cadhalpun X-Google-Original-From: Andreas Cadhalpun To: FFmpeg development discussions and patches Message-ID: <6d786760-6b84-d460-8928-d2a5331d8a60@googlemail.com> Date: Thu, 17 Nov 2016 01:08:31 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Icedove/45.4.0 MIME-Version: 1.0 Subject: [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" All the fields can be set via options read from the ffm file and thus have to be sanitized. A negative extradata size for example gets passed to memcpy in avcodec_parameters_from_context causing a segmentation fault. Signed-off-by: Andreas Cadhalpun --- The other fields like qmin etc. seem to be completely ignored since the switch to codecpar, except possibly by ffserver. --- libavformat/ffmdec.c | 138 +++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 102 insertions(+), 36 deletions(-) diff --git a/libavformat/ffmdec.c b/libavformat/ffmdec.c index 6b09be2..e66ba5c 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,55 @@ static int ffm_append_recommended_configuration(AVStream *st, char **conf) return 0; } +#define SANITIZE_PARAMETER(parameter, name, check, default) { \ + if (check) { \ + av_log(codec, AV_LOG_WARNING, "Invalid " name " %d\n", codec->parameter); \ + codec->parameter = default; \ + } \ +} + +static void ffm_sanitize_parameters(AVCodecContext *codec) +{ + if (codec->time_base.num < 0 || codec->time_base.den <= 0) { + av_log(codec, AV_LOG_WARNING, "Invalid time base %d/%d\n", + codec->time_base.num, codec->time_base.den); + codec->time_base.num = 0; + codec->time_base.den = 1; + } + if (codec->bit_rate < 0) { + av_log(codec, AV_LOG_WARNING, "Invalid bit rate %"PRId64"\n", codec->bit_rate); + codec->bit_rate = 0; + } + if ((codec->width || codec->height) && av_image_check_size(codec->width, codec->height, 0, codec) < 0) { + codec->width = 0; + codec->height = 0; + } + if (codec->sample_aspect_ratio.num < 0 || codec->sample_aspect_ratio.den < 0) { + av_log(codec, AV_LOG_WARNING, "Invalid sample aspect ratio %d/%d\n", + codec->sample_aspect_ratio.num, codec->sample_aspect_ratio.den); + codec->sample_aspect_ratio.num = 0; + codec->sample_aspect_ratio.den = 1; + } + SANITIZE_PARAMETER(pix_fmt, "pixel format", codec->pix_fmt < AV_PIX_FMT_NONE || codec->pix_fmt > AV_PIX_FMT_NB, AV_PIX_FMT_NONE) + SANITIZE_PARAMETER(bits_per_coded_sample, "bits per coded sample", codec->bits_per_coded_sample < 0, 0) + SANITIZE_PARAMETER(bits_per_raw_sample, "bits per raw sample", codec->bits_per_raw_sample < 0, 0) + SANITIZE_PARAMETER(extradata_size, "extradata size", codec->extradata_size < 0 || codec->extradata_size >= FF_MAX_EXTRADATA_SIZE, 0) + SANITIZE_PARAMETER(color_range, "color range", (unsigned)codec->color_range > AVCOL_RANGE_NB, AVCOL_RANGE_UNSPECIFIED) + SANITIZE_PARAMETER(color_primaries, "color primaries", (unsigned)codec->color_primaries > AVCOL_PRI_NB, AVCOL_PRI_UNSPECIFIED) + SANITIZE_PARAMETER(color_trc, "color transfer characteristics ", (unsigned)codec->color_trc > AVCOL_TRC_NB, AVCOL_TRC_UNSPECIFIED) + SANITIZE_PARAMETER(colorspace, "color space", (unsigned)codec->colorspace > AVCOL_SPC_NB, AVCOL_SPC_UNSPECIFIED) + SANITIZE_PARAMETER(chroma_sample_location, "chroma location", (unsigned)codec->chroma_sample_location > AVCHROMA_LOC_NB, AVCHROMA_LOC_UNSPECIFIED) + SANITIZE_PARAMETER(has_b_frames, "video delay", codec->has_b_frames < 0, 0) + SANITIZE_PARAMETER(sample_fmt, "sample format", codec->sample_fmt < AV_SAMPLE_FMT_NONE || codec->sample_fmt > AV_SAMPLE_FMT_NB, AV_SAMPLE_FMT_NONE) + SANITIZE_PARAMETER(channels, "number of channels", codec->channels < 0, 0) + SANITIZE_PARAMETER(sample_rate, "sample rate", codec->sample_rate < 0, 0) + SANITIZE_PARAMETER(block_align, "block alignment", codec->block_align < 0, 0) + SANITIZE_PARAMETER(frame_size, "frame size", codec->frame_size < 0, 0) + SANITIZE_PARAMETER(initial_padding, "initial padding", codec->initial_padding < 0, 0) + SANITIZE_PARAMETER(trailing_padding, "trailing padding", codec->trailing_padding < 0, 0) + SANITIZE_PARAMETER(seek_preroll, "seek preroll", codec->seek_preroll < 0, 0) +} + static int ffm2_read_header(AVFormatContext *s) { FFMContext *ffm = s->priv_data; @@ -346,23 +397,29 @@ static int ffm2_read_header(AVFormatContext *s) } codec->codec_type = avio_r8(pb); if (codec->codec_type != codec_desc->type) { - av_log(s, AV_LOG_ERROR, "Codec type mismatch: expected %d, found %d\n", + av_log(s, AV_LOG_WARNING, "Codec type mismatch: expected %d, found %d\n", codec_desc->type, codec->codec_type); - codec->codec_id = AV_CODEC_ID_NONE; - codec->codec_type = AVMEDIA_TYPE_UNKNOWN; - goto fail; + codec->codec_type = codec_desc->type; } 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); + codec->bit_rate = 0; + } 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); - codec->extradata = av_mallocz(size + AV_INPUT_BUFFER_PADDING_SIZE); - if (!codec->extradata) - return AVERROR(ENOMEM); - codec->extradata_size = size; - avio_read(pb, codec->extradata, size); + if (size < 0 || size >= FF_MAX_EXTRADATA_SIZE) { + av_log(s, AV_LOG_WARNING, "Invalid extradata size %d\n", size); + } else { + codec->extradata = av_mallocz(size + AV_INPUT_BUFFER_PADDING_SIZE); + if (!codec->extradata) + return AVERROR(ENOMEM); + codec->extradata_size = size; + avio_read(pb, codec->extradata, size); + } } break; case MKBETAG('S', 'T', 'V', 'I'): @@ -372,20 +429,23 @@ static int ffm2_read_header(AVFormatContext *s) } codec->time_base.num = avio_rb32(pb); codec->time_base.den = avio_rb32(pb); - if (codec->time_base.num <= 0 || codec->time_base.den <= 0) { - av_log(s, AV_LOG_ERROR, "Invalid time base %d/%d\n", + if (codec->time_base.num < 0 || codec->time_base.den <= 0) { + av_log(s, AV_LOG_WARNING, "Invalid time base %d/%d\n", codec->time_base.num, codec->time_base.den); - ret = AVERROR_INVALIDDATA; - goto fail; + codec->time_base.num = 0; + codec->time_base.den = 1; } codec->width = avio_rb16(pb); codec->height = avio_rb16(pb); + if (av_image_check_size(codec->width, codec->height, 0, s) < 0) { + codec->width = 0; + codec->height = 0; + } codec->gop_size = avio_rb16(pb); codec->pix_fmt = avio_rb32(pb); if (!av_pix_fmt_desc_get(codec->pix_fmt)) { - av_log(s, AV_LOG_ERROR, "Invalid pix fmt id: %d\n", codec->pix_fmt); + av_log(s, AV_LOG_WARNING, "Invalid pix fmt id: %d\n", codec->pix_fmt); codec->pix_fmt = AV_PIX_FMT_NONE; - goto fail; } codec->qmin = avio_r8(pb); codec->qmax = avio_r8(pb); @@ -432,13 +492,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; - } + SANITIZE_PARAMETER(sample_rate, "sample rate", codec->sample_rate < 0, 0) codec->channels = avio_rl16(pb); + SANITIZE_PARAMETER(channels, "number of channels", codec->channels < 0, 0) codec->frame_size = avio_rl16(pb); + SANITIZE_PARAMETER(frame_size, "frame size", codec->frame_size < 0, 0) break; case MKBETAG('C', 'P', 'R', 'V'): if (f_cprv++) { @@ -469,6 +527,7 @@ static int ffm2_read_header(AVFormatContext *s) } avio_get_str(pb, INT_MAX, buffer, size); av_set_options_string(codec, buffer, "=", ","); + ffm_sanitize_parameters(codec); if ((ret = ffm_append_recommended_configuration(st, &buffer)) < 0) goto fail; break; @@ -484,6 +543,7 @@ static int ffm2_read_header(AVFormatContext *s) } avio_get_str(pb, INT_MAX, buffer, size); av_set_options_string(codec, buffer, "=", ","); + ffm_sanitize_parameters(codec); if ((ret = ffm_append_recommended_configuration(st, &buffer)) < 0) goto fail; break; @@ -563,11 +623,9 @@ static int ffm_read_header(AVFormatContext *s) } codec->codec_type = avio_r8(pb); /* codec_type */ if (codec->codec_type != codec_desc->type) { - av_log(s, AV_LOG_ERROR, "Codec type mismatch: expected %d, found %d\n", + av_log(s, AV_LOG_WARNING, "Codec type mismatch: expected %d, found %d\n", codec_desc->type, codec->codec_type); - codec->codec_id = AV_CODEC_ID_NONE; - codec->codec_type = AVMEDIA_TYPE_UNKNOWN; - goto fail; + codec->codec_type = codec_desc->type; } codec->bit_rate = avio_rb32(pb); codec->flags = avio_rb32(pb); @@ -579,18 +637,22 @@ static int ffm_read_header(AVFormatContext *s) codec->time_base.num = avio_rb32(pb); codec->time_base.den = avio_rb32(pb); if (codec->time_base.num <= 0 || codec->time_base.den <= 0) { - av_log(s, AV_LOG_ERROR, "Invalid time base %d/%d\n", + av_log(s, AV_LOG_WARNING, "Invalid time base %d/%d\n", codec->time_base.num, codec->time_base.den); - goto fail; + codec->time_base.num = 0; + codec->time_base.den = 1; } codec->width = avio_rb16(pb); codec->height = avio_rb16(pb); + if (av_image_check_size(codec->width, codec->height, 0, s) < 0) { + codec->width = 0; + codec->height = 0; + } codec->gop_size = avio_rb16(pb); codec->pix_fmt = avio_rb32(pb); if (!av_pix_fmt_desc_get(codec->pix_fmt)) { - av_log(s, AV_LOG_ERROR, "Invalid pix fmt id: %d\n", codec->pix_fmt); + av_log(s, AV_LOG_WARNING, "Invalid pix fmt id: %d\n", codec->pix_fmt); codec->pix_fmt = AV_PIX_FMT_NONE; - goto fail; } codec->qmin = avio_r8(pb); codec->qmax = avio_r8(pb); @@ -633,9 +695,9 @@ 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; + if (codec->sample_rate < 0) { + av_log(s, AV_LOG_WARNING, "Invalid sample rate %d\n", codec->sample_rate); + codec->sample_rate = 0; } codec->channels = avio_rl16(pb); codec->frame_size = avio_rl16(pb); @@ -645,11 +707,15 @@ static int ffm_read_header(AVFormatContext *s) } if (codec->flags & AV_CODEC_FLAG_GLOBAL_HEADER) { int size = avio_rb32(pb); - codec->extradata = av_mallocz(size + AV_INPUT_BUFFER_PADDING_SIZE); - if (!codec->extradata) - return AVERROR(ENOMEM); - codec->extradata_size = size; - avio_read(pb, codec->extradata, size); + if (size < 0 || size >= FF_MAX_EXTRADATA_SIZE) { + av_log(s, AV_LOG_WARNING, "Invalid extradata size %d\n", size); + } else { + codec->extradata = av_mallocz(size + AV_INPUT_BUFFER_PADDING_SIZE); + if (!codec->extradata) + return AVERROR(ENOMEM); + codec->extradata_size = size; + avio_read(pb, codec->extradata, size); + } } avcodec_parameters_from_context(st->codecpar, codec);