From patchwork Wed Mar 27 11:18:44 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Diego Felix de Souza via ffmpeg-devel X-Patchwork-Id: 12483 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 916CE448DC9 for ; Wed, 27 Mar 2019 13:21:27 +0200 (EET) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 8061968A996; Wed, 27 Mar 2019 13:21:27 +0200 (EET) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wm1-f66.google.com (mail-wm1-f66.google.com [209.85.128.66]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 1E4A068A8BD for ; Wed, 27 Mar 2019 13:21:19 +0200 (EET) Received: by mail-wm1-f66.google.com with SMTP id f3so15818522wmj.4 for ; Wed, 27 Mar 2019 04:21:19 -0700 (PDT) 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=+FW+1ssbJDSEIPQOe5q+INo6xq0/jI9rJIqlSSwPZpk=; b=LDXx8c6iiqwG9rcGY4FSxS8lmf3GteBuUvd4Wotmp8ud7T8hBqSSay2VUr1j5VCCRN N0oaOaTo2LRhz9ZaMFhAhA1DB//hJxBkG07DJxG+qcCpYoHNZHvnTqrtlZ61eCsmDe9/ UIPkXaT/e/0fyb/LtWjJayeCkHEC980jVC8CjRY+bQIPbBuiAEimV6ciC87YmatNqeZd pl3RqJh4/g2j8p7MRAP+jvo0RuY4lcDUPSRs6cnm7FkKnqu3fGjJHX3pBxAO83ZN8Ilb 93Nv5C1JL5iWHHeSjqWp9IOT+7uOQZjTcVOpY6RhLGOmag86e5jUcjiAsLj42CszArtA LWHA== X-Gm-Message-State: APjAAAUAaDCGfE3q55ZCXmSl5quDPlbpLRDZMl/BTGHZWJ8o8edjGfDK EI3v8v9JnpS10nbuC8No/59m3QXMnhg= X-Google-Smtp-Source: APXvYqyI385yv2best09ZQuo+KHqqJOqJcFVYKqvDMjevIqvCv0D47ih1FjEPNLhoD+6HHJg1pbSkg== X-Received: by 2002:a1c:3d6:: with SMTP id 205mr11820975wmd.66.1553685678457; Wed, 27 Mar 2019 04:21:18 -0700 (PDT) Received: from localhost.localdomain (ipbcc08c44.dynamic.kabel-deutschland.de. [188.192.140.68]) by smtp.googlemail.com with ESMTPSA id h10sm31745448wrs.27.2019.03.27.04.21.17 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 27 Mar 2019 04:21:17 -0700 (PDT) To: ffmpeg-devel@ffmpeg.org Date: Wed, 27 Mar 2019 12:18:44 +0100 Message-Id: <20190327111852.3784-14-andreas.rheinhardt@googlemail.com> X-Mailer: git-send-email 2.19.2 In-Reply-To: <20190327111852.3784-1-andreas.rheinhardt@googlemail.com> References: <20190308092604.3752-1-andreas.rheinhardt@googlemail.com> <20190327111852.3784-1-andreas.rheinhardt@googlemail.com> MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH 13/21] 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: , X-Patchwork-Original-From: Andreas Rheinhardt via ffmpeg-devel From: Diego Felix de Souza via ffmpeg-devel Reply-To: FFmpeg development discussions and patches Cc: Andreas Rheinhardt , robux4@ycbcr.xyz 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 | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c index 6fa324c0cc..0179e5426e 100644 --- a/libavformat/matroskadec.c +++ b/libavformat/matroskadec.c @@ -1180,11 +1180,29 @@ 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 && - (pos + length) > (level->start + level->length)) { + + if (length != EBML_UNKNOWN_LENGTH && + level->length != EBML_UNKNOWN_LENGTH) { + uint64_t elem_end = pos + length, + level_end = level->start + level->length; + + if (level_end < elem_end) { + av_log(matroska->ctx, AV_LOG_ERROR, + "Element at 0x%"PRIx64" ending at 0x%"PRIx64" exceeds " + "containing master element ending at 0x%"PRIx64"\n", + pos, elem_end, level_end); + return AVERROR_INVALIDDATA; + } + } else if (level->length != EBML_UNKNOWN_LENGTH) { + av_log(matroska->ctx, AV_LOG_ERROR, "Unknown-sized element " + "at 0x%"PRIx64" inside parent with finite size\n", pos); + return AVERROR_INVALIDDATA; + } else if (length == EBML_UNKNOWN_LENGTH && id != MATROSKA_ID_CLUSTER) { + // According to the specifications only clusters and segments + // are allowed to be unknown-sized. av_log(matroska->ctx, AV_LOG_ERROR, - "Invalid length 0x%"PRIx64" > 0x%"PRIx64" in parent\n", - length, level->start + level->length); + "Found unknown-sized element other than a cluster at " + "0x%"PRIx64". Dropping the invalid element.\n", pos); return AVERROR_INVALIDDATA; } }