[FFmpeg-devel] [PATCH] avformat/matroskadec: Avoid undefined pointer arithmetic

Andreas Rheinhardt andreas.rheinhardt at gmail.com
Mon Jul 20 10:32:49 EEST 2020


The Matroska demuxer currently always opens a GetByteContext to read the
content of the projection's private data buffer; it does this even if
there is no private data buffer in which case opening the GetByteContext
will lead to a NULL + 0 which is undefined behaviour.
Furthermore, in this case the code relied both on the implicit checks
of the bytestream2 API as well as on the fact that it returns zero
if there is not enough data available.

Both of these issues have been addressed by not using the bytestream API
any more; instead the data is simply read directly by using AV_RB. This
is possible because the offsets are constants.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
---
 libavformat/matroskadec.c | 22 +++++++++-------------
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
index cff7f0cb54..6abb5412de 100644
--- a/libavformat/matroskadec.c
+++ b/libavformat/matroskadec.c
@@ -2162,30 +2162,26 @@ static int mkv_parse_video_projection(AVStream *st, const MatroskaTrack *track,
                                       void *logctx)
 {
     AVSphericalMapping *spherical;
+    const MatroskaTrackVideoProjection *mkv_projection = &track->video.projection;
+    const uint8_t *priv_data = mkv_projection->private.data;
     enum AVSphericalProjection projection;
     size_t spherical_size;
     uint32_t l = 0, t = 0, r = 0, b = 0;
     uint32_t padding = 0;
     int ret;
-    GetByteContext gb;
 
-    bytestream2_init(&gb, track->video.projection.private.data,
-                     track->video.projection.private.size);
-
-    if (bytestream2_get_byte(&gb) != 0) {
+    if (mkv_projection->private.size && priv_data[0] != 0) {
         av_log(logctx, AV_LOG_WARNING, "Unknown spherical metadata\n");
         return 0;
     }
 
-    bytestream2_skip(&gb, 3); // flags
-
     switch (track->video.projection.type) {
     case MATROSKA_VIDEO_PROJECTION_TYPE_EQUIRECTANGULAR:
         if (track->video.projection.private.size == 20) {
-            t = bytestream2_get_be32(&gb);
-            b = bytestream2_get_be32(&gb);
-            l = bytestream2_get_be32(&gb);
-            r = bytestream2_get_be32(&gb);
+            t = AV_RB32(priv_data +  4);
+            b = AV_RB32(priv_data +  8);
+            l = AV_RB32(priv_data + 12);
+            r = AV_RB32(priv_data + 16);
 
             if (b >= UINT_MAX - t || r >= UINT_MAX - l) {
                 av_log(logctx, AV_LOG_ERROR,
@@ -2209,14 +2205,14 @@ static int mkv_parse_video_projection(AVStream *st, const MatroskaTrack *track,
             av_log(logctx, AV_LOG_ERROR, "Missing projection private properties\n");
             return AVERROR_INVALIDDATA;
         } else if (track->video.projection.private.size == 12) {
-            uint32_t layout = bytestream2_get_be32(&gb);
+            uint32_t layout = AV_RB32(priv_data + 4);
             if (layout) {
                 av_log(logctx, AV_LOG_WARNING,
                        "Unknown spherical cubemap layout %"PRIu32"\n", layout);
                 return 0;
             }
             projection = AV_SPHERICAL_CUBEMAP;
-            padding = bytestream2_get_be32(&gb);
+            padding = AV_RB32(priv_data + 8);
         } else {
             av_log(logctx, AV_LOG_ERROR, "Unknown spherical metadata\n");
             return AVERROR_INVALIDDATA;
-- 
2.20.1



More information about the ffmpeg-devel mailing list