[FFmpeg-devel] [PATCH] matroskadec: add generic element length validation.

[Reimar Döffinger]

This validate the length of a mkv element directly after reading
it.
This has the advantage that it is easy to add new limits and makes
it less likely to forget to add checks and also avoids issues like
bits of the length value above the first 32 being ignored because
the parsing functions only takes an int.
Previously discussed in the "mkv 0-byte integer parsing" thread.
---
 libavformat/matroskadec.c |   17 +++++++++++++++++
 1 files changed, 17 insertions(+), 0 deletions(-)

diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
index 1a87f4a..9a670e8 100644
--- a/libavformat/matroskadec.c
+++ b/libavformat/matroskadec.c
@@ -58,6 +58,7 @@ typedef enum {
     EBML_NEST,
     EBML_PASS,
     EBML_STOP,
+    EBML_TYPE_COUNT
 } EbmlType;
 
 typedef const struct EbmlSyntax {
@@ -780,6 +781,16 @@ static int ebml_parse_nest(MatroskaDemuxContext *matroska, EbmlSyntax *syntax,
 static int ebml_parse_elem(MatroskaDemuxContext *matroska,
                            EbmlSyntax *syntax, void *data)
 {
+    static const uint64_t max_lengths[EBML_TYPE_COUNT] = {
+        [EBML_UINT]  = 8,
+        [EBML_FLOAT] = 8,
+        // max. 16 MB for strings
+        [EBML_STR]   = 0x1000000,
+        [EBML_UTF8]  = 0x1000000,
+        // max. 256 MB for binary data
+        [EBML_BIN]   = 0x10000000,
+        // no limits for anything else
+    };
     ByteIOContext *pb = matroska->ctx->pb;
     uint32_t id = syntax->id;
     uint64_t length;
@@ -798,6 +809,12 @@ static int ebml_parse_elem(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;
+        }
     }
 
     switch (syntax->type) {
-- 
1.7.2.3