From 04b62bd7ceb45d338799768a9c8b139f5b0ad77e Mon Sep 17 00:00:00 2001 From: Andreas Rheinhardt Date: Fri, 17 May 2019 00:30:15 +0200 Subject: [PATCH] avformat/matroskadec: Improve invalid length error handling 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 2485d8edf4..26a1d702a4 100644 --- a/libavformat/matroskadec.c +++ b/libavformat/matroskadec.c @@ -1169,6 +1169,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, @@ -1253,12 +1255,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; @@ -1297,6 +1293,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