From patchwork Mon Jul 22 03:27:13 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 14028 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 D75B444841E for ; Mon, 22 Jul 2019 06:39:14 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id AF8BB68A380; Mon, 22 Jul 2019 06:39:14 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wr1-f65.google.com (mail-wr1-f65.google.com [209.85.221.65]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 1087168A380 for ; Mon, 22 Jul 2019 06:39:08 +0300 (EEST) Received: by mail-wr1-f65.google.com with SMTP id n9so37822518wru.0 for ; Sun, 21 Jul 2019 20:39:08 -0700 (PDT) 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=/K/3xuv/lN89iqBzZQcgr898WjNqkArPMdeBqTIcAsg=; b=tgGUO0vGYmVFvHcmP9BzZA5EW74/lVE/oS+h8vbMsNikWfXi7Da1cIH7pBMvruHyh1 p8i7a9GtqM+rQ7XqqVpjXhhjdyzyCQ9Rd5wYHbP1MjFedRRzc40tFoL9VTZ3nQWlA+kv 1uMOMzm3MryPanE1qMpWrlyzZOFNGuuSRinGMzjXLtMVv6XzU2diQ9oplC+Hg2GtvfpB kpD0xBPW0frqfbl6faFZbDlIGwpdIs3O1cayNMpP5VgHbSXgQ8rwK52nJxNFmBFduEv+ EsC8BscBS9MnXYKkOkApksHs46Dq1OJT9hp0/JDBWlvUbHz9dFAoIcIleMs2G3aFxaQ/ 7KrQ== 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=/K/3xuv/lN89iqBzZQcgr898WjNqkArPMdeBqTIcAsg=; b=sLn2qyMSOddnpdeOEN81B4WApvprcZ8QwuC7R5tGB4cEY+r91HkMPc+i51mTq/U6Yu CjayO6kUBZOHpeQzjJTF2wJRq1NyiMiQgf/3tU0GU/+BXg5/ZGrXX+FNRy4XY5L50ZJU sK+WWhTgyPK9LEt5YXIW/KYdlGapVce5Ki6Qul5RaYFiQiHJY3AhR3iHSMVJY3tcojr8 KwN0vzjVpSEw+69L3vsBZtakQ0sCQm1LA5+Y82uxrN8udtpCUYNYq3j++SpNcCnSOtih 197gZbwDxjZp9JVsldEszQRADi/sdsr7DCMlgFXcjABoKLG0JMQxhVaoUmTGdVGfaw99 geXQ== X-Gm-Message-State: APjAAAXCSM4y3ja3Bc9UGyxun2XUo6YHiGgUt0v0yFRVhiUulZR53FPw riMLpgPZ6040L4bHlZJOiGBBSw/O X-Google-Smtp-Source: APXvYqw6/0IfrcS6URlu4lWaFuxscbE144DISnaZhdAc+M/1BpfyUhaCUwj4kiZ+1c5sGlJSG7LF6w== X-Received: by 2002:a05:6000:1203:: with SMTP id e3mr39045064wrx.300.1563766436464; Sun, 21 Jul 2019 20:33:56 -0700 (PDT) Received: from localhost.localdomain (ipbcc08b8f.dynamic.kabel-deutschland.de. [188.192.139.143]) by smtp.gmail.com with ESMTPSA id t1sm49233202wra.74.2019.07.21.20.33.55 (version=TLS1_3 cipher=AEAD-AES256-GCM-SHA384 bits=256/256); Sun, 21 Jul 2019 20:33:56 -0700 (PDT) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Mon, 22 Jul 2019 05:27:13 +0200 Message-Id: <20190722032716.40032-2-andreas.rheinhardt@gmail.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190722032716.40032-1-andreas.rheinhardt@gmail.com> References: <20190722032716.40032-1-andreas.rheinhardt@gmail.com> MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH 2/4] h264_mp4toannexb_bsf: Improve extradata overread checks 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" 1. Currently during parsing the extradata, h264_mp4toannexb checks for overreads by adding the size of the current unit to the current position pointer and comparing this to the end position of the extradata. But pointer comparisons and pointer arithmetic is only defined if it does not exceed the object it is used on (one past the last element of an array is allowed, too). In practice, this might lead to overflows. Therefore the check has been changed. 2. The minimal size of an AVCDecoderConfigurationRecord is actually 7: Four bytes containing version, profile and level, one byte for length size and one byte each for the numbers of SPS and PPS. This has been changed. The byte for the number of PPS has been forgotten. 3. The earlier code also did not detect an error if the extradata ended directly after the last SPS in the SPS array (if any) although the number of PPS has to come afterwards. A check for this has been integrated into the general overread check. 4. The earlier code also might overread when reading the size of the next unit. Given that this overread is not dangerous (the extradata is supposed to be padded), only a comment for it has been added; the error itself will be detected as part of the normal check for overreads. Signed-off-by: Andreas Rheinhardt --- libavcodec/h264_mp4toannexb_bsf.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/libavcodec/h264_mp4toannexb_bsf.c b/libavcodec/h264_mp4toannexb_bsf.c index 8b2899f300..076dfa31a7 100644 --- a/libavcodec/h264_mp4toannexb_bsf.c +++ b/libavcodec/h264_mp4toannexb_bsf.c @@ -71,7 +71,8 @@ static int h264_extradata_to_annexb(AVBSFContext *ctx, const int padding) int total_size = 0; uint8_t *out = NULL, unit_nb, sps_done = 0, sps_seen = 0, pps_seen = 0; - const uint8_t *extradata = ctx->par_in->extradata + 4; + const uint8_t *extradata = ctx->par_in->extradata + 4, + *extradata_end = ctx->par_in->extradata + ctx->par_in->extradata_size; static const uint8_t nalu_header[4] = { 0, 0, 0, 1 }; int length_size = (*extradata++ & 0x3) + 1; // retrieve length coded size @@ -89,10 +90,11 @@ static int h264_extradata_to_annexb(AVBSFContext *ctx, const int padding) while (unit_nb--) { int err; - unit_size = AV_RB16(extradata); + unit_size = AV_RB16(extradata); /* possible overread ok due to padding */ + extradata += 2; total_size += unit_size + 4; - if (extradata + 2 + unit_size > ctx->par_in->extradata + ctx->par_in->extradata_size) { - av_log(ctx, AV_LOG_ERROR, "Packet header is not contained in global extradata, " + if (extradata_end - extradata < unit_size + !sps_done) { + av_log(ctx, AV_LOG_ERROR, "Global extradata truncated, " "corrupted stream or invalid MP4/AVCC bitstream\n"); av_free(out); return AVERROR(EINVAL); @@ -100,8 +102,8 @@ static int h264_extradata_to_annexb(AVBSFContext *ctx, const int padding) if ((err = av_reallocp(&out, total_size + padding)) < 0) return err; memcpy(out + total_size - unit_size - 4, nalu_header, 4); - memcpy(out + total_size - unit_size, extradata + 2, unit_size); - extradata += 2 + unit_size; + memcpy(out + total_size - unit_size, extradata, unit_size); + extradata += unit_size; pps: if (!unit_nb && !sps_done++) { unit_nb = *extradata++; /* number of pps unit(s) */ @@ -144,7 +146,7 @@ static int h264_mp4toannexb_init(AVBSFContext *ctx) (extra_size >= 4 && AV_RB32(ctx->par_in->extradata) == 1)) { av_log(ctx, AV_LOG_VERBOSE, "The input looks like it is Annex B already\n"); - } else if (extra_size >= 6) { + } else if (extra_size >= 7) { ret = h264_extradata_to_annexb(ctx, AV_INPUT_BUFFER_PADDING_SIZE); if (ret < 0) return ret;