From patchwork Mon Dec 9 22:25:59 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 16682 Return-Path: X-Original-To: patchwork@ffaux-bg.ffmpeg.org Delivered-To: patchwork@ffaux-bg.ffmpeg.org Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by ffaux.localdomain (Postfix) with ESMTP id E8CFD44AA6E for ; Tue, 10 Dec 2019 00:34:04 +0200 (EET) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id D240F68B6FE; Tue, 10 Dec 2019 00:34:04 +0200 (EET) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wr1-f67.google.com (mail-wr1-f67.google.com [209.85.221.67]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 7A61A68B0BD for ; Tue, 10 Dec 2019 00:33:58 +0200 (EET) Received: by mail-wr1-f67.google.com with SMTP id w15so18012400wru.4 for ; Mon, 09 Dec 2019 14:33:58 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=VfdhCRx6cmRG3Qi81NVPLrCVh2qvSUZTJiOs2v13lbk=; b=hXnrGRun+aE+ATHS4/Dfhj/FmL/SO81XwBsO/nokMuon8zuSn1AUrIh2+aWGd76NAo uiLXKl/cflcBFnBdpAD7+y2YWZChjr+Cgjf0Sf34mN6/vcO7VtQosJvm+Ke+5TbD17xb RZ/YX9bMMbo1GFcmpF3gTXEOOxxUkhDiX/dL851i6IQ5FeDH1oyJ8XhlYlWbP4psb2qA axul3XN+PmMnwH+1HTvKKMNLJlRKeEcpYjiyK6BJMhIkghG4/J4ZWT/6knjlS5OVfBDy MPBIYa4HGwF44qCqvAmBwHofPJCttlMKNLt6HM5RTyV6Utg8wBP++kcrJPzSQD4JU9wV hSeA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=VfdhCRx6cmRG3Qi81NVPLrCVh2qvSUZTJiOs2v13lbk=; b=MtMuUNhek69QtfRbv9tm/3NASQogYICAsU0XTrL4zD0eIozseeyM6FVeCK+Xmt/Eez redu8eiZ04yBNuSRQ/QcGzO4ZOuy32K/foTGEnhwpg5U33v4meOYIC/3Y/s5im4/Fjng UDIp9e1Mar82qt26O1QVhSiN7nyYnEzuIS8iGu6ohxDHsmoALvSVcvD5kpVjBxjVGpB2 s6qPKs5t0C+y/cCk/8w7Fn2sBpV4zGVEi0UBhlq2CjLyR2gEXRksoZkDYhyZYCBqreZc /gW1P68+io5A+A0OGgvTrcRTPy9mLydcUH4xaIyUDtWz2zp01TU3pLD/ol3L4RxTE6rX 5ovw== X-Gm-Message-State: APjAAAVP4JuQ8XLBTJQ+QYt2EyDNo9wALQTyhHea+mL5cMUWFOHiuPcU wDJKtFQtiDFBzPWlSEJxIG/mT98J X-Google-Smtp-Source: APXvYqx004pgXCvgOnHiGxB5fRMWLJgwJ3SCqjkBDYiDl5apuboZfCgsTber0MOkuGB2NhaYi0NxuA== X-Received: by 2002:adf:fa87:: with SMTP id h7mr4406080wrr.172.1575930837602; Mon, 09 Dec 2019 14:33:57 -0800 (PST) Received: from sblaptop.fritz.box (ipbcc08e23.dynamic.kabel-deutschland.de. [188.192.142.35]) by smtp.gmail.com with ESMTPSA id v22sm865893wml.11.2019.12.09.14.33.56 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 09 Dec 2019 14:33:57 -0800 (PST) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Mon, 9 Dec 2019 23:25:59 +0100 Message-Id: <20191209222604.28920-2-andreas.rheinhardt@gmail.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20191209222604.28920-1-andreas.rheinhardt@gmail.com> References: <20191209222604.28920-1-andreas.rheinhardt@gmail.com> MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH 2/7] avcodec/cbs: Check for overflow when assembling fragments 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 Cc: Andreas Rheinhardt Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Up until now, the various functions for assembling fragments did not check for overflow in the computation of the size of the output fragment or whether the size they allocate actually fits into an int (as it must given that the AVBuffer API is based on size fields of type int). In order to optimize this, the overflow check is partially done generically: The size of all the units is just added while checking for it to be <= INT_MAX - AV_INPUT_BUFFER_PADDING_SIZE. The result is accessible to the function doing the actual assembling. Signed-off-by: Andreas Rheinhardt --- In case of H.26x, the new allocation might be slightly larger than the old one (e.g. two units of odd size would be independently rounded down when multiplying by 3 followed by dividing by two). But that hardly matters. libavcodec/cbs.c | 10 ++++++++++ libavcodec/cbs_av1.c | 7 +------ libavcodec/cbs_h2645.c | 12 +++++++----- libavcodec/cbs_jpeg.c | 9 ++++++--- libavcodec/cbs_mpeg2.c | 9 +++++---- libavcodec/cbs_vp9.c | 6 ++++-- 6 files changed, 33 insertions(+), 20 deletions(-) diff --git a/libavcodec/cbs.c b/libavcodec/cbs.c index 805049404b..e70e11fb99 100644 --- a/libavcodec/cbs.c +++ b/libavcodec/cbs.c @@ -343,6 +343,7 @@ static int cbs_write_unit_data(CodedBitstreamContext *ctx, int ff_cbs_write_fragment_data(CodedBitstreamContext *ctx, CodedBitstreamFragment *frag) { + size_t fragment_size; int err, i; for (i = 0; i < frag->nb_units; i++) { @@ -366,6 +367,15 @@ int ff_cbs_write_fragment_data(CodedBitstreamContext *ctx, av_buffer_unref(&frag->data_ref); frag->data = NULL; + fragment_size = 0; + for (i = 0; i < frag->nb_units; i++) { + if (frag->units[i].data_size > INT_MAX - AV_INPUT_BUFFER_PADDING_SIZE + - fragment_size) + return AVERROR(ERANGE); + fragment_size += frag->units[i].data_size; + } + frag->data_size = fragment_size; + err = ctx->codec->assemble_fragment(ctx, frag); if (err < 0) { av_log(ctx->log_ctx, AV_LOG_ERROR, "Failed to assemble fragment.\n"); diff --git a/libavcodec/cbs_av1.c b/libavcodec/cbs_av1.c index bbe4461130..1c51839f16 100644 --- a/libavcodec/cbs_av1.c +++ b/libavcodec/cbs_av1.c @@ -1223,13 +1223,9 @@ static int cbs_av1_write_obu(CodedBitstreamContext *ctx, static int cbs_av1_assemble_fragment(CodedBitstreamContext *ctx, CodedBitstreamFragment *frag) { - size_t size, pos; + size_t size = frag->data_size, pos; int i; - size = 0; - for (i = 0; i < frag->nb_units; i++) - size += frag->units[i].data_size; - frag->data_ref = av_buffer_alloc(size + AV_INPUT_BUFFER_PADDING_SIZE); if (!frag->data_ref) return AVERROR(ENOMEM); @@ -1243,7 +1239,6 @@ static int cbs_av1_assemble_fragment(CodedBitstreamContext *ctx, pos += frag->units[i].data_size; } av_assert0(pos == size); - frag->data_size = size; return 0; } diff --git a/libavcodec/cbs_h2645.c b/libavcodec/cbs_h2645.c index 5f71d80584..4b17780836 100644 --- a/libavcodec/cbs_h2645.c +++ b/libavcodec/cbs_h2645.c @@ -1392,11 +1392,13 @@ static int cbs_h2645_assemble_fragment(CodedBitstreamContext *ctx, av_assert0(frag->units[i].data); } - max_size = 0; - for (i = 0; i < frag->nb_units; i++) { - // Start code + content with worst-case emulation prevention. - max_size += 4 + frag->units[i].data_size * 3 / 2; - } + // Content with worst-case emulation prevention. + max_size = frag->data_size + frag->data_size / 2; + if (frag->nb_units > (int)(INT_MAX - AV_INPUT_BUFFER_PADDING_SIZE + - max_size + 3) / 4) + return AVERROR(ERANGE); + // Start codes + max_size += 4 * frag->nb_units; data = av_realloc(NULL, max_size + AV_INPUT_BUFFER_PADDING_SIZE); if (!data) diff --git a/libavcodec/cbs_jpeg.c b/libavcodec/cbs_jpeg.c index 2bb6c8d18c..6c131d2e2e 100644 --- a/libavcodec/cbs_jpeg.c +++ b/libavcodec/cbs_jpeg.c @@ -391,13 +391,12 @@ static int cbs_jpeg_assemble_fragment(CodedBitstreamContext *ctx, { const CodedBitstreamUnit *unit; uint8_t *data; - size_t size, dp, sp; + size_t size = frag->data_size, dp, sp; int i; - size = 4; // SOI + EOI. + size += 4; // SOI + EOI. for (i = 0; i < frag->nb_units; i++) { unit = &frag->units[i]; - size += 2 + unit->data_size; if (unit->type == JPEG_MARKER_SOS) { for (sp = 0; sp < unit->data_size; sp++) { if (unit->data[sp] == 0xff) @@ -405,6 +404,10 @@ static int cbs_jpeg_assemble_fragment(CodedBitstreamContext *ctx, } } } + if (frag->nb_units > (int)(INT_MAX - AV_INPUT_BUFFER_PADDING_SIZE + - size + 1) / 2) + return AVERROR(ERANGE); + size += 2 * frag->nb_units; frag->data_ref = av_buffer_alloc(size + AV_INPUT_BUFFER_PADDING_SIZE); if (!frag->data_ref) diff --git a/libavcodec/cbs_mpeg2.c b/libavcodec/cbs_mpeg2.c index 255f033734..e42c602476 100644 --- a/libavcodec/cbs_mpeg2.c +++ b/libavcodec/cbs_mpeg2.c @@ -384,12 +384,13 @@ static int cbs_mpeg2_assemble_fragment(CodedBitstreamContext *ctx, CodedBitstreamFragment *frag) { uint8_t *data; - size_t size, dp; + size_t dp, size = frag->data_size; int i; - size = 0; - for (i = 0; i < frag->nb_units; i++) - size += 3 + frag->units[i].data_size; + if (frag->nb_units > (INT_MAX - AV_INPUT_BUFFER_PADDING_SIZE + - size + 2) / 3) + return AVERROR(ERANGE); + size += 3 * frag->nb_units; frag->data_ref = av_buffer_alloc(size + AV_INPUT_BUFFER_PADDING_SIZE); if (!frag->data_ref) diff --git a/libavcodec/cbs_vp9.c b/libavcodec/cbs_vp9.c index f6cfaa3b36..ea7747182e 100644 --- a/libavcodec/cbs_vp9.c +++ b/libavcodec/cbs_vp9.c @@ -595,9 +595,11 @@ static int cbs_vp9_assemble_fragment(CodedBitstreamContext *ctx, sfi.bytes_per_framesize_minus_1 = size_len - 1; sfi.frames_in_superframe_minus_1 = frag->nb_units - 1; - size = 2; + size = 2 + size_len * frag->nb_units + frag->data_size; + if (size > INT_MAX - AV_INPUT_BUFFER_PADDING_SIZE) + return AVERROR(ERANGE); + for (i = 0; i < frag->nb_units; i++) { - size += size_len + frag->units[i].data_size; sfi.frame_sizes[i] = frag->units[i].data_size; }