Blame SOURCES/9efd93e20dd7789e4172ad6c8f4108271b3fb1ee.patch

ee39b2
From 9efd93e20dd7789e4172ad6c8f4108271b3fb1ee Mon Sep 17 00:00:00 2001
ee39b2
From: =?UTF-8?q?Sebastian=20Dr=C3=B6ge?= <sebastian@centricular.com>
ee39b2
Date: Thu, 4 Mar 2021 13:05:19 +0200
ee39b2
Subject: [PATCH] matroskademux: Fix extraction of multichannel WavPack
ee39b2
ee39b2
The old code had a couple of issues that all lead to potential memory
ee39b2
safety bugs.
ee39b2
ee39b2
  - Use a constant for the Wavpack4Header size instead of using sizeof.
ee39b2
    It's written out into the data and not from the struct and who knows
ee39b2
    what special alignment/padding requirements some C compilers have.
ee39b2
  - gst_buffer_set_size() does not realloc the buffer when setting a
ee39b2
    bigger size than allocated, it only allows growing up to the maximum
ee39b2
    allocated size. Instead use a GstAdapter to collect all the blocks
ee39b2
    and take out everything at once in the end.
ee39b2
  - Check that enough data is actually available in the input and
ee39b2
    otherwise handle it an error in all cases instead of silently
ee39b2
    ignoring it.
ee39b2
ee39b2
Among other things this fixes out of bounds writes because the code
ee39b2
assumed gst_buffer_set_size() can grow the buffer and simply wrote after
ee39b2
the end of the buffer.
ee39b2
ee39b2
Thanks to Natalie Silvanovich for reporting.
ee39b2
ee39b2
Fixes https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/-/issues/859
ee39b2
ee39b2
Part-of: <https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/-/merge_requests/904>
ee39b2
---
ee39b2
 gst/matroska/matroska-demux.c | 99 +++++++++++++++++++----------------
ee39b2
 gst/matroska/matroska-ids.h   |  2 +
ee39b2
 2 files changed, 55 insertions(+), 46 deletions(-)
ee39b2
ee39b2
diff --git a/gst/matroska/matroska-demux.c b/gst/matroska/matroska-demux.c
ee39b2
index 4eb3d2a9f..f890ae611 100644
ee39b2
--- a/gst/matroska/matroska-demux.c
ee39b2
+++ b/gst/matroska/matroska-demux.c
ee39b2
@@ -3706,6 +3706,12 @@ gst_matroska_demux_add_wvpk_header (GstElement * element,
ee39b2
     guint32 block_samples, tmp;
ee39b2
     gsize size = gst_buffer_get_size (*buf);
ee39b2
 
ee39b2
+    if (size < 4) {
ee39b2
+      GST_ERROR_OBJECT (element, "Too small wavpack buffer");
ee39b2
+      gst_buffer_unmap (*buf, &map);
ee39b2
+      return GST_FLOW_ERROR;
ee39b2
+    }
ee39b2
+
ee39b2
     gst_buffer_extract (*buf, 0, &tmp, sizeof (guint32));
ee39b2
     block_samples = GUINT32_FROM_LE (tmp);
ee39b2
     /* we need to reconstruct the header of the wavpack block */
ee39b2
@@ -3713,10 +3719,10 @@ gst_matroska_demux_add_wvpk_header (GstElement * element,
ee39b2
     /* -20 because ck_size is the size of the wavpack block -8
ee39b2
      * and lace_size is the size of the wavpack block + 12
ee39b2
      * (the three guint32 of the header that already are in the buffer) */
ee39b2
-    wvh.ck_size = size + sizeof (Wavpack4Header) - 20;
ee39b2
+    wvh.ck_size = size + WAVPACK4_HEADER_SIZE - 20;
ee39b2
 
ee39b2
     /* block_samples, flags and crc are already in the buffer */
ee39b2
-    newbuf = gst_buffer_new_allocate (NULL, sizeof (Wavpack4Header) - 12, NULL);
ee39b2
+    newbuf = gst_buffer_new_allocate (NULL, WAVPACK4_HEADER_SIZE - 12, NULL);
ee39b2
 
ee39b2
     gst_buffer_map (newbuf, &outmap, GST_MAP_WRITE);
ee39b2
     data = outmap.data;
ee39b2
@@ -3741,9 +3747,11 @@ gst_matroska_demux_add_wvpk_header (GstElement * element,
ee39b2
     audiocontext->wvpk_block_index += block_samples;
ee39b2
   } else {
ee39b2
     guint8 *outdata = NULL;
ee39b2
-    guint outpos = 0;
ee39b2
-    gsize buf_size, size, out_size = 0;
ee39b2
+    gsize buf_size, size;
ee39b2
     guint32 block_samples, flags, crc, blocksize;
ee39b2
+    GstAdapter *adapter;
ee39b2
+
ee39b2
+    adapter = gst_adapter_new ();
ee39b2
 
ee39b2
     gst_buffer_map (*buf, &map, GST_MAP_READ);
ee39b2
     buf_data = map.data;
ee39b2
@@ -3752,6 +3760,7 @@ gst_matroska_demux_add_wvpk_header (GstElement * element,
ee39b2
     if (buf_size < 4) {
ee39b2
       GST_ERROR_OBJECT (element, "Too small wavpack buffer");
ee39b2
       gst_buffer_unmap (*buf, &map);
ee39b2
+      g_object_unref (adapter);
ee39b2
       return GST_FLOW_ERROR;
ee39b2
     }
ee39b2
 
ee39b2
@@ -3773,59 +3782,57 @@ gst_matroska_demux_add_wvpk_header (GstElement * element,
ee39b2
       data += 4;
ee39b2
       size -= 4;
ee39b2
 
ee39b2
-      if (blocksize == 0 || size < blocksize)
ee39b2
-        break;
ee39b2
-
ee39b2
-      g_assert ((newbuf == NULL) == (outdata == NULL));
ee39b2
+      if (blocksize == 0 || size < blocksize) {
ee39b2
+        GST_ERROR_OBJECT (element, "Too small wavpack buffer");
ee39b2
+        gst_buffer_unmap (*buf, &map);
ee39b2
+        g_object_unref (adapter);
ee39b2
+        return GST_FLOW_ERROR;
ee39b2
+      }
ee39b2
 
ee39b2
-      if (newbuf == NULL) {
ee39b2
-        out_size = sizeof (Wavpack4Header) + blocksize;
ee39b2
-        newbuf = gst_buffer_new_allocate (NULL, out_size, NULL);
ee39b2
+      g_assert (newbuf == NULL);
ee39b2
 
ee39b2
-        gst_buffer_copy_into (newbuf, *buf,
ee39b2
-            GST_BUFFER_COPY_TIMESTAMPS | GST_BUFFER_COPY_FLAGS, 0, -1);
ee39b2
+      newbuf =
ee39b2
+          gst_buffer_new_allocate (NULL, WAVPACK4_HEADER_SIZE + blocksize,
ee39b2
+          NULL);
ee39b2
+      gst_buffer_map (newbuf, &outmap, GST_MAP_WRITE);
ee39b2
+      outdata = outmap.data;
ee39b2
+
ee39b2
+      outdata[0] = 'w';
ee39b2
+      outdata[1] = 'v';
ee39b2
+      outdata[2] = 'p';
ee39b2
+      outdata[3] = 'k';
ee39b2
+      outdata += 4;
ee39b2
+
ee39b2
+      GST_WRITE_UINT32_LE (outdata, blocksize + WAVPACK4_HEADER_SIZE - 8);
ee39b2
+      GST_WRITE_UINT16_LE (outdata + 4, wvh.version);
ee39b2
+      GST_WRITE_UINT8 (outdata + 6, wvh.track_no);
ee39b2
+      GST_WRITE_UINT8 (outdata + 7, wvh.index_no);
ee39b2
+      GST_WRITE_UINT32_LE (outdata + 8, wvh.total_samples);
ee39b2
+      GST_WRITE_UINT32_LE (outdata + 12, wvh.block_index);
ee39b2
+      GST_WRITE_UINT32_LE (outdata + 16, block_samples);
ee39b2
+      GST_WRITE_UINT32_LE (outdata + 20, flags);
ee39b2
+      GST_WRITE_UINT32_LE (outdata + 24, crc);
ee39b2
+      outdata += 28;
ee39b2
+
ee39b2
+      memcpy (outdata, data, blocksize);
ee39b2
 
ee39b2
-        outpos = 0;
ee39b2
-        gst_buffer_map (newbuf, &outmap, GST_MAP_WRITE);
ee39b2
-        outdata = outmap.data;
ee39b2
-      } else {
ee39b2
-        gst_buffer_unmap (newbuf, &outmap);
ee39b2
-        out_size += sizeof (Wavpack4Header) + blocksize;
ee39b2
-        gst_buffer_set_size (newbuf, out_size);
ee39b2
-        gst_buffer_map (newbuf, &outmap, GST_MAP_WRITE);
ee39b2
-        outdata = outmap.data;
ee39b2
-      }
ee39b2
+      gst_buffer_unmap (newbuf, &outmap);
ee39b2
+      gst_adapter_push (adapter, newbuf);
ee39b2
+      newbuf = NULL;
ee39b2
 
ee39b2
-      outdata[outpos] = 'w';
ee39b2
-      outdata[outpos + 1] = 'v';
ee39b2
-      outdata[outpos + 2] = 'p';
ee39b2
-      outdata[outpos + 3] = 'k';
ee39b2
-      outpos += 4;
ee39b2
-
ee39b2
-      GST_WRITE_UINT32_LE (outdata + outpos,
ee39b2
-          blocksize + sizeof (Wavpack4Header) - 8);
ee39b2
-      GST_WRITE_UINT16_LE (outdata + outpos + 4, wvh.version);
ee39b2
-      GST_WRITE_UINT8 (outdata + outpos + 6, wvh.track_no);
ee39b2
-      GST_WRITE_UINT8 (outdata + outpos + 7, wvh.index_no);
ee39b2
-      GST_WRITE_UINT32_LE (outdata + outpos + 8, wvh.total_samples);
ee39b2
-      GST_WRITE_UINT32_LE (outdata + outpos + 12, wvh.block_index);
ee39b2
-      GST_WRITE_UINT32_LE (outdata + outpos + 16, block_samples);
ee39b2
-      GST_WRITE_UINT32_LE (outdata + outpos + 20, flags);
ee39b2
-      GST_WRITE_UINT32_LE (outdata + outpos + 24, crc);
ee39b2
-      outpos += 28;
ee39b2
-
ee39b2
-      memmove (outdata + outpos, data, blocksize);
ee39b2
-      outpos += blocksize;
ee39b2
       data += blocksize;
ee39b2
       size -= blocksize;
ee39b2
     }
ee39b2
     gst_buffer_unmap (*buf, &map);
ee39b2
-    gst_buffer_unref (*buf);
ee39b2
 
ee39b2
-    if (newbuf)
ee39b2
-      gst_buffer_unmap (newbuf, &outmap);
ee39b2
+    newbuf = gst_adapter_take_buffer (adapter, gst_adapter_available (adapter));
ee39b2
+    g_object_unref (adapter);
ee39b2
 
ee39b2
+    gst_buffer_copy_into (newbuf, *buf,
ee39b2
+        GST_BUFFER_COPY_TIMESTAMPS | GST_BUFFER_COPY_FLAGS, 0, -1);
ee39b2
+    gst_buffer_unref (*buf);
ee39b2
     *buf = newbuf;
ee39b2
+
ee39b2
     audiocontext->wvpk_block_index += block_samples;
ee39b2
   }
ee39b2
 
ee39b2
diff --git a/gst/matroska/matroska-ids.h b/gst/matroska/matroska-ids.h
ee39b2
index 9b263d8a1..a0d68343f 100644
ee39b2
--- a/gst/matroska/matroska-ids.h
ee39b2
+++ b/gst/matroska/matroska-ids.h
ee39b2
@@ -667,6 +667,8 @@ typedef struct _Wavpack4Header {
ee39b2
   guint32 crc;           /* crc for actual decoded data                    */
ee39b2
 } Wavpack4Header;
ee39b2
 
ee39b2
+#define WAVPACK4_HEADER_SIZE (32)
ee39b2
+
ee39b2
 typedef enum {
ee39b2
   GST_MATROSKA_TRACK_ENCODING_SCOPE_FRAME = (1<<0),
ee39b2
   GST_MATROSKA_TRACK_ENCODING_SCOPE_CODEC_DATA = (1<<1),
ee39b2
-- 
ee39b2
GitLab
ee39b2