qtdemux: Fix length checks and offsets in stsd entry parsing

Thanks to Antonio Morales for finding and reporting the issue.

Fixes GHSL-2024-242
Fixes https://gitlab.freedesktop.org/gstreamer/gstreamer/-/issues/3845

Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/8060>

diff --git a/subprojects/gst-plugins-good/gst/isomp4/qtdemux.c b/subprojects/gst-plugins-good/gst/isomp4/qtdemux.c
index 55ba59152c7a63490ef5a3538e251212c6fc0210..fb157552eb7575a0b1456bc492d7e2641a9d351e 100644 (file)
--- a/subprojects/gst-plugins-good/gst/isomp4/qtdemux.c
+++ b/subprojects/gst-plugins-good/gst/isomp4/qtdemux.c
@@ -12237,43 +12237,35 @@ qtdemux_parse_trak (GstQTDemux * qtdemux, GNode * trak)
           case FOURCC_avc1:
           case FOURCC_avc3:
           {
-            guint len = QT_UINT32 (stsd_entry_data);
+            guint32 len = QT_UINT32 (stsd_entry_data);
             len = len <= 0x56 ? 0 : len - 0x56;
             const guint8 *avc_data = stsd_entry_data + 0x56;
 
             /* find avcC */
-            while (len >= 0x8) {
-              guint size;
-
-              if (QT_UINT32 (avc_data) <= 0x8)
-                size = 0;
-              else if (QT_UINT32 (avc_data) <= len)
-                size = QT_UINT32 (avc_data) - 0x8;
-              else
-                size = len - 0x8;
+            while (len >= 8) {
+              guint32 size = QT_UINT32 (avc_data);
 
-              /* No real data, so skip */
-              if (size < 1) {
-                len -= 8;
-                avc_data += 8;
-                continue;
-              }
+              if (size < 8 || size > len)
+                break;
 
-              switch (QT_FOURCC (avc_data + 0x4)) {
+              switch (QT_FOURCC (avc_data + 4)) {
                 case FOURCC_avcC:
                 {
                   /* parse, if found */
                   GstBuffer *buf;
 
+                  if (size < 8 + 1)
+                    break;
+
                   GST_DEBUG_OBJECT (qtdemux, "found avcC codec_data in stsd");
 
                   /* First 4 bytes are the length of the atom, the next 4 bytes
                    * are the fourcc, the next 1 byte is the version, and the
                    * subsequent bytes are profile_tier_level structure like data. */
                   gst_codec_utils_h264_caps_set_level_and_profile (entry->caps,
-                      avc_data + 8 + 1, size - 1);
-                  buf = gst_buffer_new_and_alloc (size);
-                  gst_buffer_fill (buf, 0, avc_data + 0x8, size);
+                      avc_data + 8 + 1, size - 8 - 1);
+                  buf = gst_buffer_new_and_alloc (size - 8);
+                  gst_buffer_fill (buf, 0, avc_data + 8, size - 8);
                   gst_caps_set_simple (entry->caps,
                       "codec_data", GST_TYPE_BUFFER, buf, NULL);
                   gst_buffer_unref (buf);
@@ -12284,6 +12276,9 @@ qtdemux_parse_trak (GstQTDemux * qtdemux, GNode * trak)
                 {
                   GstBuffer *buf;
 
+                  if (size < 8 + 40 + 1)
+                    break;
+
                   GST_DEBUG_OBJECT (qtdemux, "found strf codec_data in stsd");
 
                   /* First 4 bytes are the length of the atom, the next 4 bytes
@@ -12291,17 +12286,14 @@ qtdemux_parse_trak (GstQTDemux * qtdemux, GNode * trak)
                    * next 1 byte is the version, and the
                    * subsequent bytes are sequence parameter set like data. */
 
-                  size -= 40;   /* we'll be skipping BITMAPINFOHEADER */
-                  if (size > 1) {
-                    gst_codec_utils_h264_caps_set_level_and_profile
-                        (entry->caps, avc_data + 8 + 40 + 1, size - 1);
+                  gst_codec_utils_h264_caps_set_level_and_profile
+                      (entry->caps, avc_data + 8 + 40 + 1, size - 8 - 40 - 1);
 
-                    buf = gst_buffer_new_and_alloc (size);
-                    gst_buffer_fill (buf, 0, avc_data + 8 + 40, size);
-                    gst_caps_set_simple (entry->caps,
-                        "codec_data", GST_TYPE_BUFFER, buf, NULL);
-                    gst_buffer_unref (buf);
-                  }
+                  buf = gst_buffer_new_and_alloc (size - 8 - 40);
+                  gst_buffer_fill (buf, 0, avc_data + 8 + 40, size - 8 - 40);
+                  gst_caps_set_simple (entry->caps,
+                      "codec_data", GST_TYPE_BUFFER, buf, NULL);
+                  gst_buffer_unref (buf);
                   break;
                 }
                 case FOURCC_btrt:
@@ -12309,11 +12301,11 @@ qtdemux_parse_trak (GstQTDemux * qtdemux, GNode * trak)
                   guint avg_bitrate, max_bitrate;
 
                   /* bufferSizeDB, maxBitrate and avgBitrate - 4 bytes each */
-                  if (size < 12)
+                  if (size < 8 + 12)
                     break;
 
-                  max_bitrate = QT_UINT32 (avc_data + 0xc);
-                  avg_bitrate = QT_UINT32 (avc_data + 0x10);
+                  max_bitrate = QT_UINT32 (avc_data + 8 + 4);
+                  avg_bitrate = QT_UINT32 (avc_data + 8 + 8);
 
                   if (!max_bitrate && !avg_bitrate)
                     break;
@@ -12345,8 +12337,8 @@ qtdemux_parse_trak (GstQTDemux * qtdemux, GNode * trak)
                   break;
               }
 
-              len -= size + 8;
-              avc_data += size + 8;
+              len -= size;
+              avc_data += size;
             }
 
             break;
@@ -12357,44 +12349,36 @@ qtdemux_parse_trak (GstQTDemux * qtdemux, GNode * trak)
           case FOURCC_dvh1:
           case FOURCC_dvhe:
           {
-            guint len = QT_UINT32 (stsd_entry_data);
+            guint32 len = QT_UINT32 (stsd_entry_data);
             len = len <= 0x56 ? 0 : len - 0x56;
             const guint8 *hevc_data = stsd_entry_data + 0x56;
 
             /* find hevc */
-            while (len >= 0x8) {
-              guint size;
-
-              if (QT_UINT32 (hevc_data) <= 0x8)
-                size = 0;
-              else if (QT_UINT32 (hevc_data) <= len)
-                size = QT_UINT32 (hevc_data) - 0x8;
-              else
-                size = len - 0x8;
+            while (len >= 8) {
+              guint32 size = QT_UINT32 (hevc_data);
 
-              /* No real data, so skip */
-              if (size < 1) {
-                len -= 8;
-                hevc_data += 8;
-                continue;
-              }
+              if (size < 8 || size > len)
+                break;
 
-              switch (QT_FOURCC (hevc_data + 0x4)) {
+              switch (QT_FOURCC (hevc_data + 4)) {
                 case FOURCC_hvcC:
                 {
                   /* parse, if found */
                   GstBuffer *buf;
 
+                  if (size < 8 + 1)
+                    break;
+
                   GST_DEBUG_OBJECT (qtdemux, "found hvcC codec_data in stsd");
 
                   /* First 4 bytes are the length of the atom, the next 4 bytes
                    * are the fourcc, the next 1 byte is the version, and the
                    * subsequent bytes are sequence parameter set like data. */
                   gst_codec_utils_h265_caps_set_level_tier_and_profile
-                      (entry->caps, hevc_data + 8 + 1, size - 1);
+                      (entry->caps, hevc_data + 8 + 1, size - 8 - 1);
 
-                  buf = gst_buffer_new_and_alloc (size);
-                  gst_buffer_fill (buf, 0, hevc_data + 0x8, size);
+                  buf = gst_buffer_new_and_alloc (size - 8);
+                  gst_buffer_fill (buf, 0, hevc_data + 8, size - 8);
                   gst_caps_set_simple (entry->caps,
                       "codec_data", GST_TYPE_BUFFER, buf, NULL);
                   gst_buffer_unref (buf);
@@ -12403,8 +12387,8 @@ qtdemux_parse_trak (GstQTDemux * qtdemux, GNode * trak)
                 default:
                   break;
               }
-              len -= size + 8;
-              hevc_data += size + 8;
+              len -= size;
+              hevc_data += size;
             }
             break;
           }
@@ -12784,36 +12768,25 @@ qtdemux_parse_trak (GstQTDemux * qtdemux, GNode * trak)
           }
           case FOURCC_vc_1:
           {
-            guint len = QT_UINT32 (stsd_entry_data);
+            guint32 len = QT_UINT32 (stsd_entry_data);
             len = len <= 0x56 ? 0 : len - 0x56;
             const guint8 *vc1_data = stsd_entry_data + 0x56;
 
             /* find dvc1 */
             while (len >= 8) {
-              guint size;
-
-              if (QT_UINT32 (vc1_data) <= 8)
-                size = 0;
-              else if (QT_UINT32 (vc1_data) <= len)
-                size = QT_UINT32 (vc1_data) - 8;
-              else
-                size = len - 8;
+              guint32 size = QT_UINT32 (vc1_data);
 
-              /* No real data, so skip */
-              if (size < 1) {
-                len -= 8;
-                vc1_data += 8;
-                continue;
-              }
+              if (size < 8 || size > len)
+                break;
 
-              switch (QT_FOURCC (vc1_data + 0x4)) {
+              switch (QT_FOURCC (vc1_data + 4)) {
                 case GST_MAKE_FOURCC ('d', 'v', 'c', '1'):
                 {
                   GstBuffer *buf;
 
                   GST_DEBUG_OBJECT (qtdemux, "found dvc1 codec_data in stsd");
-                  buf = gst_buffer_new_and_alloc (size);
-                  gst_buffer_fill (buf, 0, vc1_data + 8, size);
+                  buf = gst_buffer_new_and_alloc (size - 8);
+                  gst_buffer_fill (buf, 0, vc1_data + 8, size - 8);
                   gst_caps_set_simple (entry->caps,
                       "codec_data", GST_TYPE_BUFFER, buf, NULL);
                   gst_buffer_unref (buf);
@@ -12822,36 +12795,25 @@ qtdemux_parse_trak (GstQTDemux * qtdemux, GNode * trak)
                 default:
                   break;
               }
-              len -= size + 8;
-              vc1_data += size + 8;
+              len -= size;
+              vc1_data += size;
             }
             break;
           }
           case FOURCC_av01:
           {
-            guint len = QT_UINT32 (stsd_entry_data);
+            guint32 len = QT_UINT32 (stsd_entry_data);
             len = len <= 0x56 ? 0 : len - 0x56;
             const guint8 *av1_data = stsd_entry_data + 0x56;
 
             /* find av1C */
-            while (len >= 0x8) {
-              guint size;
-
-              if (QT_UINT32 (av1_data) <= 0x8)
-                size = 0;
-              else if (QT_UINT32 (av1_data) <= len)
-                size = QT_UINT32 (av1_data) - 0x8;
-              else
-                size = len - 0x8;
+            while (len >= 8) {
+              guint32 size = QT_UINT32 (av1_data);
 
-              /* No real data, so skip */
-              if (size < 1) {
-                len -= 8;
-                av1_data += 8;
-                continue;
-              }
+              if (size < 8 || size > len)
+                break;
 
-              switch (QT_FOURCC (av1_data + 0x4)) {
+              switch (QT_FOURCC (av1_data + 4)) {
                 case FOURCC_av1C:
                 {
                   /* parse, if found */
@@ -12861,7 +12823,7 @@ qtdemux_parse_trak (GstQTDemux * qtdemux, GNode * trak)
                       "found av1C codec_data in stsd of size %d", size);
 
                   /* not enough data, just ignore and hope for the best */
-                  if (size < 4)
+                  if (size < 8 + 4)
                     break;
 
                   /* Content is:
@@ -12910,9 +12872,9 @@ qtdemux_parse_trak (GstQTDemux * qtdemux, GNode * trak)
                             (gint) (pres_delay_field & 0x0F) + 1, NULL);
                       }
 
-                      buf = gst_buffer_new_and_alloc (size);
+                      buf = gst_buffer_new_and_alloc (size - 8);
                       GST_BUFFER_FLAG_SET (buf, GST_BUFFER_FLAG_HEADER);
-                      gst_buffer_fill (buf, 0, av1_data + 8, size);
+                      gst_buffer_fill (buf, 0, av1_data + 8, size - 8);
                       gst_caps_set_simple (entry->caps,
                           "codec_data", GST_TYPE_BUFFER, buf, NULL);
                       gst_buffer_unref (buf);
@@ -12930,8 +12892,8 @@ qtdemux_parse_trak (GstQTDemux * qtdemux, GNode * trak)
                   break;
               }
 
-              len -= size + 8;
-              av1_data += size + 8;
+              len -= size;
+              av1_data += size;
             }
 
             break;
@@ -12942,29 +12904,18 @@ qtdemux_parse_trak (GstQTDemux * qtdemux, GNode * trak)
              * vp08, vp09, and vp10 fourcc. */
           case FOURCC_vp09:
           {
-            guint len = QT_UINT32 (stsd_entry_data);
+            guint32 len = QT_UINT32 (stsd_entry_data);
             len = len <= 0x56 ? 0 : len - 0x56;
             const guint8 *vpcc_data = stsd_entry_data + 0x56;
 
             /* find vpcC */
-            while (len >= 0x8) {
-              guint size;
-
-              if (QT_UINT32 (vpcc_data) <= 0x8)
-                size = 0;
-              else if (QT_UINT32 (vpcc_data) <= len)
-                size = QT_UINT32 (vpcc_data) - 0x8;
-              else
-                size = len - 0x8;
+            while (len >= 8) {
+              guint32 size = QT_UINT32 (vpcc_data);
 
-              /* No real data, so skip */
-              if (size < 1) {
-                len -= 8;
-                vpcc_data += 8;
-                continue;
-              }
+              if (size < 8 || size > len)
+                break;
 
-              switch (QT_FOURCC (vpcc_data + 0x4)) {
+              switch (QT_FOURCC (vpcc_data + 4)) {
                 case FOURCC_vpcC:
                 {
                   const gchar *profile_str = NULL;
@@ -12980,7 +12931,7 @@ qtdemux_parse_trak (GstQTDemux * qtdemux, GNode * trak)
 
                   /* the meaning of "size" is length of the atom body, excluding
                    * atom length and fourcc fields */
-                  if (size < 12)
+                  if (size < 8 + 12)
                     break;
 
                   /* Content is:
@@ -13086,8 +13037,8 @@ qtdemux_parse_trak (GstQTDemux * qtdemux, GNode * trak)
                   break;
               }
 
-              len -= size + 8;
-              vpcc_data += size + 8;
+              len -= size;
+              vpcc_data += size;
             }
 
             break;
@@ -13428,7 +13379,7 @@ qtdemux_parse_trak (GstQTDemux * qtdemux, GNode * trak)
         }
         case FOURCC_wma_:
         {
-          guint len = QT_UINT32 (stsd_entry_data);
+          guint32 len = QT_UINT32 (stsd_entry_data);
           len = len <= offset ? 0 : len - offset;
           const guint8 *wfex_data = stsd_entry_data + offset;
           const gchar *codec_name = NULL;
@@ -13453,21 +13404,10 @@ qtdemux_parse_trak (GstQTDemux * qtdemux, GNode * trak)
 
           /* find wfex */
           while (len >= 8) {
-            guint size;
+            guint32 size = QT_UINT32 (wfex_data);
 
-            if (QT_UINT32 (wfex_data) <= 0x8)
-              size = 0;
-            else if (QT_UINT32 (wfex_data) <= len)
-              size = QT_UINT32 (wfex_data) - 8;
-            else
-              size = len - 8;
-
-            /* No real data, so skip */
-            if (size < 1) {
-              len -= 8;
-              wfex_data += 8;
-              continue;
-            }
+            if (size < 8 || size > len)
+              break;
 
             switch (QT_FOURCC (wfex_data + 4)) {
               case GST_MAKE_FOURCC ('w', 'f', 'e', 'x'):
@@ -13512,12 +13452,12 @@ qtdemux_parse_trak (GstQTDemux * qtdemux, GNode * trak)
                     "width", G_TYPE_INT, wfex.wBitsPerSample,
                     "depth", G_TYPE_INT, wfex.wBitsPerSample, NULL);
 
-                if (size > wfex.cbSize) {
+                if (size > 8 + wfex.cbSize) {
                   GstBuffer *buf;
 
-                  buf = gst_buffer_new_and_alloc (size - wfex.cbSize);
+                  buf = gst_buffer_new_and_alloc (size - 8 - wfex.cbSize);
                   gst_buffer_fill (buf, 0, wfex_data + 8 + wfex.cbSize,
-                      size - wfex.cbSize);
+                      size - 8 - wfex.cbSize);
                   gst_caps_set_simple (entry->caps,
                       "codec_data", GST_TYPE_BUFFER, buf, NULL);
                   gst_buffer_unref (buf);
@@ -13534,8 +13474,8 @@ qtdemux_parse_trak (GstQTDemux * qtdemux, GNode * trak)
               default:
                 break;
             }
-            len -= size + 8;
-            wfex_data += size + 8;
+            len -= size;
+            wfex_data += size;
           }
           break;
         }