From patchwork Thu Sep 5 20:16:06 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 14936 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 684D5448A33 for ; Thu, 5 Sep 2019 23:29:49 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 43648680D20; Thu, 5 Sep 2019 23:29:49 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wr1-f42.google.com (mail-wr1-f42.google.com [209.85.221.42]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 30F906804E8 for ; Thu, 5 Sep 2019 23:29:43 +0300 (EEST) Received: by mail-wr1-f42.google.com with SMTP id t16so4241431wra.6 for ; Thu, 05 Sep 2019 13:29:43 -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=sXRxgSi3mQBVYIofhcLiBMP9D7hU7UVP5KF8eda0Ups=; b=fDXHVxgQp1PnaveyyK6+LIDsAnnBJ5+cex5qtB8bfLElT+NA4NtI58IzaxXEU9unHQ sp2IYfwKapuCLCiCiOw4BTgKyRB7fLSrmXZP7wipUgXf1nvCdCmIhzxyVmb1+g7szoHh ampuKCUsKSz7mb61crWE85Uckq6OgbwCeWZ+G1UrhEXTUgsFIML9stv6tEKd6skd2RWz fE7aO7hzh7YxEMUgFNJXeCuyZODFzso1meuArcaU5I7UxPN+7H34hpDfTpYopQtboQy3 3k6Ax9wAjAVydajn683TQuKDPWwjvP8RSGJ6DRBNpBDvZ9reQpE35hF04H49rycYzmqy 8j/Q== 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=sXRxgSi3mQBVYIofhcLiBMP9D7hU7UVP5KF8eda0Ups=; b=CecneioEtWOs4K4AJPG4LkwIXPWKxH0srJX/lGQA4vTKbAI4L1QvCgHN+TBGElLjqE w0N7zAqvqg6OvWxLalZo0OlfUi9MOLa8RSF73X865/C0UynqOdaNko7EVxPkNrOZXRnh En39m5/TEXf74xN+AQ9PalSt6Tpo/LBoMz8Cz0twP024oevB0cQIwtQxdPDp23LKleBd bydBkI/w4oRj85YRk3GLTdn1Hk6GWWVrtegh2XsYfnzShKvqvKayB7hbUWSplz6Njupb E/Yj6KO5klxD0B5h8ZQFrJ025EHxIADSTYhtb0nYy7E/j2iZIjf0rpVT4aWJYu1SwBeg f29Q== X-Gm-Message-State: APjAAAV0dgX+GuyLZiYsMW3eNesE5zwtkPE1cp9WIpFxQzA2lUa7009k Zr7R0bHTkeDLmIrPJqeLrXF4LapNdCI= X-Google-Smtp-Source: APXvYqxE1WOP4LlQUAh1gpdpBWxJeNRufaSBCoo6/Xsc/5YKH0R8wfKQ4CKDFRgqW6EFXo80tpkw1Q== X-Received: by 2002:adf:e603:: with SMTP id p3mr4080793wrm.102.1567715048468; Thu, 05 Sep 2019 13:24:08 -0700 (PDT) Received: from localhost.localdomain (ipbcc0f857.dynamic.kabel-deutschland.de. [188.192.248.87]) by smtp.gmail.com with ESMTPSA id g201sm5889769wmg.34.2019.09.05.13.24.07 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 05 Sep 2019 13:24:07 -0700 (PDT) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Thu, 5 Sep 2019 22:16:06 +0200 Message-Id: <20190905201609.998-5-andreas.rheinhardt@gmail.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190905201609.998-1-andreas.rheinhardt@gmail.com> References: <20190905201609.998-1-andreas.rheinhardt@gmail.com> MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH 5/8] avformat/matroskadec: Sanitize Cues 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. The Matroska CuePoints (the index entries) contain timestamps and offsets as uint64_t. When adding them to a stream's index, they will be automatically converted to int64_t. Up until now, there was no check for whether a overflow happened during said conversion. This has been changed. Given that values that overflow would be absurdly large, they are treated as invalid data despite being in compliance with the Matroska specifications. 2. A CuePoint has to contain a timestamp and at least a CueTrackPositions which in turn needs to contain an offset. In order to check for the existence of these elements, the timestamp as well as the offset now have the default value UINT64_MAX, so that a CuePoint that doesn't contain these elements will run afoul of the overflow checks mentioned above. Signed-off-by: Andreas Rheinhardt --- libavformat/matroskadec.c | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c index 2fe147126e..e5c7a6b29d 100644 --- a/libavformat/matroskadec.c +++ b/libavformat/matroskadec.c @@ -630,7 +630,7 @@ static EbmlSyntax matroska_chapters[] = { static EbmlSyntax matroska_index_pos[] = { { MATROSKA_ID_CUETRACK, EBML_UINT, 0, offsetof(MatroskaIndexPos, track) }, - { MATROSKA_ID_CUECLUSTERPOSITION, EBML_UINT, 0, offsetof(MatroskaIndexPos, pos) }, + { MATROSKA_ID_CUECLUSTERPOSITION, EBML_UINT, 0, offsetof(MatroskaIndexPos, pos), { .u = UINT64_MAX } }, { MATROSKA_ID_CUERELATIVEPOSITION,EBML_NONE }, { MATROSKA_ID_CUEDURATION, EBML_NONE }, { MATROSKA_ID_CUEBLOCKNUMBER, EBML_NONE }, @@ -638,7 +638,7 @@ static EbmlSyntax matroska_index_pos[] = { }; static EbmlSyntax matroska_index_entry[] = { - { MATROSKA_ID_CUETIME, EBML_UINT, 0, offsetof(MatroskaIndex, time) }, + { MATROSKA_ID_CUETIME, EBML_UINT, 0, offsetof(MatroskaIndex, time), { .u = UINT64_MAX } }, { MATROSKA_ID_CUETRACKPOSITION, EBML_NEST, sizeof(MatroskaIndexPos), offsetof(MatroskaIndex, pos), { .n = matroska_index_pos } }, CHILD_OF(matroska_index) }; @@ -1896,22 +1896,34 @@ static void matroska_add_index_entries(MatroskaDemuxContext *matroska) if (index_list->nb_elem < 2) return; if (index[1].time > 1E14 / matroska->time_scale) { - av_log(matroska->ctx, AV_LOG_WARNING, "Dropping apparently-broken index.\n"); - return; + goto fail; } for (i = 0; i < index_list->nb_elem; i++) { EbmlList *pos_list = &index[i].pos; MatroskaIndexPos *pos = pos_list->elem; + + if ((int64_t)index[i].time < 0 || !pos_list->nb_elem) + goto fail; + for (j = 0; j < pos_list->nb_elem; j++) { MatroskaTrack *track = matroska_find_track_by_num(matroska, pos[j].track); + int64_t abs_pos = pos[j].pos + matroska->segment_start; + + if (abs_pos < matroska->segment_start) + goto fail; + if (track && track->stream) - av_add_index_entry(track->stream, - pos[j].pos + matroska->segment_start, + av_add_index_entry(track->stream, abs_pos, index[i].time / index_scale, 0, 0, AVINDEX_KEYFRAME); } } + return; +fail: + av_log(matroska->ctx, AV_LOG_WARNING, + "Dropping apparently broken index.\n"); + return; } static void matroska_parse_cues(MatroskaDemuxContext *matroska) {