diff mbox series

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

Message ID 20200720073249.32014-1-andreas.rheinhardt@gmail.com
State Accepted
Commit 880519c1de3f2bfad04e6fef93e0bf41129ff99e
Headers show
Series [FFmpeg-devel] avformat/matroskadec: Avoid undefined pointer arithmetic | expand

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

Andreas Rheinhardt July 20, 2020, 7:32 a.m. UTC 
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@gmail.com>
---
 libavformat/matroskadec.c | 22 +++++++++-------------
 1 file changed, 9 insertions(+), 13 deletions(-)
diff mbox series

Patch

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;