From patchwork Thu May 16 22:30:15 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 13156 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 299E744964F for ; Fri, 17 May 2019 01:43:32 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 149E368A596; Fri, 17 May 2019 01:43:32 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wr1-f66.google.com (mail-wr1-f66.google.com [209.85.221.66]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 9D12268A3EB for ; Fri, 17 May 2019 01:43:28 +0300 (EEST) Received: by mail-wr1-f66.google.com with SMTP id r7so5008167wrr.13 for ; Thu, 16 May 2019 15:43:28 -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=Y+MpWJsHMZ9WRlHK12CCDjV4/fLn/bLmHvut7z3YEAU=; b=ZF/RMojpFfTPF4Mvky0t4zr5PauhgZHkXUtE7c0Tf5CGT9I5HGupHZDyKUnEAQ4Jyq uTT0ZiW2DBlqkBh97S253fQJ8TSCa4LkJ3fmldt5tAw1oBbD99TQ4IobNg1tSaYq4ky1 fO8s+H+P1g3Fw4lpElRy2pZ8H/ElXdGbdklcAeu9+londaZ9ZLFY1ABB0lrJqP2k+lwy 8sXCVLlMnBJ4m6gtJcEtXLSNZFPh+EVo/S/f5H/6AzNAWBnZbYK4gSJT7FHtS4e8N7Oz GApgspxztCz1wcu2QpSo7GJVYZ36COLYnWc0mYQwBBO0rWUsQ4+iLCc7Rasp+/hhzVPf mH+A== 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=Y+MpWJsHMZ9WRlHK12CCDjV4/fLn/bLmHvut7z3YEAU=; b=VsafYB5o9PLjFhmYropITYn8amGmLjjKkKIrsoiXPUaoBBgZPMkog95onXsb+drVNY DXp5ArSBKRqk7uNYFEjsXks6bhCa3x3f2R1QloygcMIMRo4gZlcFfDAkFio1R0D0PwdT RJUAWdXU/WdQw+yBu/2tRUZPtNDooa/ihh2bTn0yWWYPh9kRqWPk4t5oyVwVann2BS2P Ptcy3cRrdPS7qqfePd4xXL0qBD6mD0YS86D+NcusDYzBReWOsnkq41E/8UyfmIS+h88v oiM370HPelJA6qcFZBKEwG0+AbvTroSsG5avZ6PyMmze9jCqLEo4e18igKZZZkH4Jhua hdFQ== X-Gm-Message-State: APjAAAVKnt2LmXV1sLqBSBiu0oSEUr57duAniZk3O2R128OGonhPrKkK PNYRl0Hd6YaLsEahie422nmFCPD2 X-Google-Smtp-Source: APXvYqwRVIGHFFz36BkEuO/EEiUCyHMiWOPYvfUtKlLPRvgafy8epNXwz2sy65Q0fnU0uMNRGkbw0g== X-Received: by 2002:adf:f741:: with SMTP id z1mr32176660wrp.14.1558046607952; Thu, 16 May 2019 15:43:27 -0700 (PDT) Received: from localhost.localdomain (ipbcc18715.dynamic.kabel-deutschland.de. [188.193.135.21]) by smtp.gmail.com with ESMTPSA id i185sm11168725wmg.32.2019.05.16.15.43.27 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 16 May 2019 15:43:27 -0700 (PDT) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Fri, 17 May 2019 00:30:15 +0200 Message-Id: <20190516223018.30827-32-andreas.rheinhardt@gmail.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190516223018.30827-1-andreas.rheinhardt@gmail.com> References: <20190516223018.30827-1-andreas.rheinhardt@gmail.com> MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH 31/37] avformat/matroskadec: Improve invalid length error handling 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. Up until now, the error message for EBML numbers whose length exceeds the limits imposed upon them because of the element's type did not distinguish between known-length and unknown-length elements. As a consequence, the numerical value of the define constant EBML_UNKNOWN_LENGTH was emitted as part of the error message which is of course not appropriate. This commit changes this by adding error messages designed for unknown-length elements. 2. We impose some (arbitrary) sanity checks on the lengths of certain element types; these checks were conducted before the checks depending on whether the element exceeds its containing master element. Now the order has been reversed, because a failure at the (formerly) latter check implies that the file is truly erroneous and not only fails our arbitrary length limit. Moreover, this increases the informativeness of the error messages. 3. Furthermore, the error message in general has been changed by replacing the type of the element (something internal to this demuxer and therefore suitable as debug output at best, not as an error message intended for ordinary users) with the element ID. The element's position has been added, too. 4. Finally, the length limit for EBML_NONE elements has been changed so that all unknown-length elements of EBML_NONE-type trigger an error. This is done because unknown-length elements can't be skipped and need to be parsed, but there is no syntax to parse available for EBML_NONE elements. This is done in preparation for a further patch which allows more unknown-length elements than just clusters and segments. Signed-off-by: Andreas Rheinhardt --- libavformat/matroskadec.c | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c index f19e0fc523..85d8252f6d 100644 --- a/libavformat/matroskadec.c +++ b/libavformat/matroskadec.c @@ -1165,6 +1165,8 @@ static int ebml_parse(MatroskaDemuxContext *matroska, EbmlSyntax *syntax, void *data) { static const uint64_t max_lengths[EBML_TYPE_COUNT] = { + // Forbid unknown-length EBML_NONE elements. + [EBML_NONE] = EBML_UNKNOWN_LENGTH - 1, [EBML_UINT] = 8, [EBML_SINT] = 8, [EBML_FLOAT] = 8, @@ -1249,12 +1251,6 @@ static int ebml_parse(MatroskaDemuxContext *matroska, matroska->current_id = 0; if ((res = ebml_read_length(matroska, pb, &length)) < 0) return res; - if (max_lengths[syntax->type] && length > max_lengths[syntax->type]) { - av_log(matroska->ctx, AV_LOG_ERROR, - "Invalid length 0x%"PRIx64" > 0x%"PRIx64" for syntax element %i\n", - length, max_lengths[syntax->type], syntax->type); - return AVERROR_INVALIDDATA; - } pos_alt += res; @@ -1293,6 +1289,26 @@ static int ebml_parse(MatroskaDemuxContext *matroska, } else level_check = 0; + if (max_lengths[syntax->type] && length > max_lengths[syntax->type]) { + if (length != EBML_UNKNOWN_LENGTH) { + av_log(matroska->ctx, AV_LOG_ERROR, + "Invalid length 0x%"PRIx64" > 0x%"PRIx64" for element " + "with ID 0x%"PRIX32" at 0x%"PRIx64"\n", + length, max_lengths[syntax->type], id, pos); + } else if (syntax->type != EBML_NONE) { + av_log(matroska->ctx, AV_LOG_ERROR, + "Element with ID 0x%"PRIX32" at pos. 0x%"PRIx64" has " + "unknown length, yet the length of an element of its " + "type must be known.\n", id, pos); + } else { + av_log(matroska->ctx, AV_LOG_ERROR, + "Found unknown-length element with ID 0x%"PRIX32" at " + "pos. 0x%"PRIx64" for which no syntax for parsing is " + "available.\n", id, pos); + } + return AVERROR_INVALIDDATA; + } + if (!(pb->seekable & AVIO_SEEKABLE_NORMAL)) { // Loosing sync will likely manifest itself as encountering unknown // elements which are not reliably distinguishable from elements