[FFmpeg-devel,27/37] avformat/matroskadec: Redo EOF handling

Submitted by Andreas Rheinhardt on May 16, 2019, 10:30 p.m.

Details

Message ID 20190516223018.30827-28-andreas.rheinhardt@gmail.com
State Accepted
Commit 3ed2755baaf3f8a9f088f8c7b9cc7bd8c629e8fb
Headers show

Commit Message

Andreas Rheinhardt May 16, 2019, 10:30 p.m.
This commit closes the last hole in the system of checks for a
known-length file ending too early: Now an error message is emitted
in case the file ends directly after an EBML element.

Furthermore, this commit adds a check and a corresponding warning
whether there is data beyond the Matroska segment (only reasonable for
known-length segments). If everything looks alright, then parsing is
stopped as soon as EOF is reached (in contrast, the earlier code would
always call matroska_resync at the end).

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
---
 libavformat/matroskadec.c | 23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

Patch hide | download patch | download mbox

diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
index fdd6c4b86b..f769543b4a 100644
--- a/libavformat/matroskadec.c
+++ b/libavformat/matroskadec.c
@@ -1183,10 +1183,15 @@  static int ebml_parse(MatroskaDemuxContext *matroska,
                 if (matroska->is_live)
                     // in live mode, finish parsing if EOF is reached.
                     return 1;
-                if (level && level->length == EBML_UNKNOWN_LENGTH && pos == avio_tell(pb)) {
-                    // Unknown-length levels automatically end at EOF.
-                    matroska->num_levels--;
-                    return LEVEL_ENDED;
+                if (level && pos == avio_tell(pb)) {
+                    if (level->length == EBML_UNKNOWN_LENGTH) {
+                        // Unknown-length levels automatically end at EOF.
+                        matroska->num_levels--;
+                        return LEVEL_ENDED;
+                    } else {
+                        av_log(matroska->ctx, AV_LOG_ERROR, "File ended prematurely "
+                               "at pos. %"PRIu64" (0x%"PRIx64")\n", pos, pos);
+                    }
                 }
             }
             return res;
@@ -3614,6 +3619,14 @@  static int matroska_parse_cluster(MatroskaDemuxContext *matroska)
     ebml_free(matroska_blockgroup, block);
     memset(block, 0, sizeof(*block));
     } else if (!matroska->num_levels) {
+        if (!avio_feof(matroska->ctx->pb)) {
+            avio_r8(matroska->ctx->pb);
+            if (!avio_feof(matroska->ctx->pb)) {
+                av_log(matroska->ctx, AV_LOG_WARNING, "File extends beyond "
+                       "end of segment.\n");
+                return AVERROR_INVALIDDATA;
+            }
+        }
         matroska->done = 1;
         return AVERROR_EOF;
     }
@@ -3634,7 +3647,7 @@  static int matroska_read_packet(AVFormatContext *s, AVPacket *pkt)
     while (matroska_deliver_packet(matroska, pkt)) {
         if (matroska->done)
             return (ret < 0) ? ret : AVERROR_EOF;
-        if (matroska_parse_cluster(matroska) < 0)
+        if (matroska_parse_cluster(matroska) < 0 && !matroska->done)
             ret = matroska_resync(matroska, matroska->resync_pos);
     }