From patchwork Fri Mar 8 09:26:03 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 12247 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 BB49E44903F for ; Fri, 8 Mar 2019 11:27:36 +0200 (EET) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id AA9FB68A7BC; Fri, 8 Mar 2019 11:27:36 +0200 (EET) 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 0FB1768A7BC for ; Fri, 8 Mar 2019 11:27:34 +0200 (EET) Received: by mail-wr1-f66.google.com with SMTP id l12so4114714wrp.6 for ; Fri, 08 Mar 2019 01:27:34 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=googlemail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=O4fhrLgpSM7sgkLbV5Y1pVDcrCP6LhEOmGLK0RdRPNE=; b=c9Ymapgn9KPuqfkIcjU8kbyKPphBS3avuzzFAF6HF4/bgGqKFPDkuq/CwiHtGpiZpV IKeIFW+cepGWisSUrd/SQ+miNcU+NG1i042UHbwMSbvjVP2IxB4mOYreyGFzup5KU9DO LxuHlTVKqyvNpVZjSFMnTN9+bW/YLYzAn2BbbO16wJu/YbuqM28IefHmiyPHIW3yYLNE RX7MkGmMKNqDK/bJCgMLk8BRrE9+j6vFa44BlijoiCxDp+sAUqv8T/Yd7g88QFlxGoSI zesH284IHhwEVTs0JV1lnAvk3l+C0IBcemfU87dut4yIR60ebOWaZI6xkhiYOc4Pt0O3 URfQ== 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=O4fhrLgpSM7sgkLbV5Y1pVDcrCP6LhEOmGLK0RdRPNE=; b=RLffxW8YJWJnrdN24fHQVq9e+TYMZFLJR7CceJBqMHV9aJon/PgCwndDlO8T4WxSTm B53ROFL9LRivjF7/ZhWGM6/dqUQ+APGdA0mfTHOTGoKkWIWanpG0ffTplgivIdIO/A8P wXjMfy09M6Ak7vvRaXvlrBGet4j2YZuoGaXJ8MCiaq/TxGD54uef+fNMvJVYPoKTz8Sh YebObxVtEWQ+noltKK3Q/cSNfxMCqdIV8H3HYniuEH5E2haa6ZF0uz/+X2gJgr5ESQS5 oqIihhGY+GeKUITwUi2zJz+G6VPJg0e6oY7eKUyQ6vE6bQKqrCQnntNq8u+LHH9BUirQ RtuA== X-Gm-Message-State: APjAAAVkjZIl7LIlXld2koGoeo2+uv8mUDjuD1xD8JFoQWbQDS6i8m6c QbmxAXddrusN67or5F9J62G/SDMBf24= X-Google-Smtp-Source: APXvYqwTvjJcDKzKXu5HG5305in9LstDunXJ1V15zV1tM3bJs7rjEKDD+qeDiyPBzOxyjEQ3WBfDWg== X-Received: by 2002:a5d:4a05:: with SMTP id m5mr10666869wrq.46.1552037254255; Fri, 08 Mar 2019 01:27:34 -0800 (PST) Received: from localhost.localdomain (ipbcc08c44.dynamic.kabel-deutschland.de. [188.192.140.68]) by smtp.googlemail.com with ESMTPSA id v20sm12179825wmj.2.2019.03.08.01.27.33 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 08 Mar 2019 01:27:33 -0800 (PST) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Fri, 8 Mar 2019 10:26:03 +0100 Message-Id: <20190308092604.3752-10-andreas.rheinhardt@googlemail.com> X-Mailer: git-send-email 2.19.2 In-Reply-To: <20190308092604.3752-1-andreas.rheinhardt@googlemail.com> References: <20190308092604.3752-1-andreas.rheinhardt@googlemail.com> MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH 09/10] avformat/matroskadec: Improve length check 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" The earlier code had three flaws: 1. The case of an unknown-sized element inside a finite-sized element (which is against the specifications) was not caught. 2. The error message wasn't helpful: It compared the length of the child with the offset of the end of the parent and claimed that the first exceeds the latter, although that is not necessarily true. 3. Unknown-sized elements that are not parsed can't be skipped. Given that according to the Matroska specifications only the segment and the clusters can be of unknown-size, this is handled by not allowing any other units to have infinite size whereas the earlier code would seek back by 1 byte upon encountering an infinite-size element that ought to be skipped. Signed-off-by: Andreas Rheinhardt --- libavformat/matroskadec.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c index d899ee5744..6c796e8a4a 100644 --- a/libavformat/matroskadec.c +++ b/libavformat/matroskadec.c @@ -1172,11 +1172,22 @@ static int ebml_parse_elem(MatroskaDemuxContext *matroska, if (matroska->num_levels > 0) { MatroskaLevel *level = &matroska->levels[matroska->num_levels - 1]; int64_t pos = avio_tell(pb); - if (level->length != EBML_UNKNOWN_LENGTH && + if (length == EBML_UNKNOWN_LENGTH && + level->length != EBML_UNKNOWN_LENGTH) { + av_log(matroska->ctx, AV_LOG_ERROR, "Unknown-sized element " + "inside parent with finite size\n"); + return AVERROR_INVALIDDATA; + } else if (level->length != EBML_UNKNOWN_LENGTH && (pos + length) > (level->start + level->length)) { av_log(matroska->ctx, AV_LOG_ERROR, - "Invalid length 0x%"PRIx64" > 0x%"PRIx64" in parent\n", - length, level->start + level->length); + "Element ending at 0x%"PRIx64" exceeds containing " + "master element ending at 0x%"PRIx64"\n", + pos + length, level->start + level->length); + return AVERROR_INVALIDDATA; + } else if (length == EBML_UNKNOWN_LENGTH && id != MATROSKA_ID_CLUSTER) { + av_log(matroska->ctx, AV_LOG_ERROR, + "Found unknown-sized element other than a cluster" + " in a segment. Dropping the invalid element.\n"); return AVERROR_INVALIDDATA; } }