From 13b48016b3ef1e822c393c2871b0a561ce19ecb3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Sebastian=20Dr=C3=B6ge?= <sebastian@centricular.com>
Date: Fri, 4 Oct 2024 13:00:57 +0300
Subject: [PATCH 1/7] wavparse: Check for short reads when parsing headers in
 pull mode

And also return the actual flow return to the caller instead of always returning
GST_FLOW_ERROR.

Thanks to Antonio Morales for finding and reporting the issue.

Fixes GHSL-2024-258, GHSL-2024-260
Fixes https://gitlab.freedesktop.org/gstreamer/gstreamer/-/issues/3886
Fixes https://gitlab.freedesktop.org/gstreamer/gstreamer/-/issues/3888

Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/8042>
---
 gst/wavparse/gstwavparse.c                | 63 ++++++++++++++-----
 1 file changed, 46 insertions(+), 17 deletions(-)

diff --git a/gst/wavparse/gstwavparse.c b/gst/wavparse/gstwavparse.c
index d074f273c501..97d5591fae8f 100644
--- a/gst/wavparse/gstwavparse.c
+++ b/gst/wavparse/gstwavparse.c
@@ -1097,6 +1097,24 @@ parse_ds64 (GstWavParse * wav, GstBuffer * buf)
   return TRUE;
 }
 
+static GstFlowReturn
+gst_wavparse_pull_range_exact (GstWavParse * wav, guint64 offset, guint size,
+    GstBuffer ** buffer)
+{
+  GstFlowReturn res;
+
+  res = gst_pad_pull_range (wav->sinkpad, offset, size, buffer);
+  if (res != GST_FLOW_OK)
+    return res;
+
+  if (gst_buffer_get_size (*buffer) < size) {
+    gst_clear_buffer (buffer);
+    return GST_FLOW_EOS;
+  }
+
+  return res;
+}
+
 static GstFlowReturn
 gst_wavparse_stream_headers (GstWavParse * wav)
 {
@@ -1292,9 +1310,9 @@ gst_wavparse_stream_headers (GstWavParse * wav)
 
       buf = NULL;
       if ((res =
-              gst_pad_pull_range (wav->sinkpad, wav->offset, 8,
+              gst_wavparse_pull_range_exact (wav, wav->offset, 8,
                   &buf)) != GST_FLOW_OK)
-        goto header_read_error;
+        goto header_pull_error;
       gst_buffer_map (buf, &map, GST_MAP_READ);
       tag = GST_READ_UINT32_LE (map.data);
       size = GST_READ_UINT32_LE (map.data + 4);
@@ -1397,9 +1415,9 @@ gst_wavparse_stream_headers (GstWavParse * wav)
             gst_buffer_unref (buf);
             buf = NULL;
             if ((res =
-                    gst_pad_pull_range (wav->sinkpad, wav->offset + 8,
+                    gst_wavparse_pull_range_exact (wav, wav->offset + 8,
                         data_size, &buf)) != GST_FLOW_OK)
-              goto header_read_error;
+              goto header_pull_error;
             gst_buffer_extract (buf, 0, &wav->fact, 4);
             wav->fact = GUINT32_FROM_LE (wav->fact);
             gst_buffer_unref (buf);
@@ -1444,9 +1462,9 @@ gst_wavparse_stream_headers (GstWavParse * wav)
           gst_buffer_unref (buf);
           buf = NULL;
           if ((res =
-                  gst_pad_pull_range (wav->sinkpad, wav->offset + 8,
-                      size, &buf)) != GST_FLOW_OK)
-            goto header_read_error;
+                  gst_wavparse_pull_range_exact (wav, wav->offset + 8, size,
+                      &buf)) != GST_FLOW_OK)
+            goto header_pull_error;
           gst_buffer_map (buf, &map, GST_MAP_READ);
           acid = (const gst_riff_acid *) map.data;
           tempo = acid->tempo;
@@ -1484,9 +1502,9 @@ gst_wavparse_stream_headers (GstWavParse * wav)
           gst_buffer_unref (buf);
           buf = NULL;
           if ((res =
-                  gst_pad_pull_range (wav->sinkpad, wav->offset, 12,
+                  gst_wavparse_pull_range_exact (wav, wav->offset, 12,
                       &buf)) != GST_FLOW_OK)
-            goto header_read_error;
+            goto header_pull_error;
           gst_buffer_extract (buf, 8, &ltag, 4);
           ltag = GUINT32_FROM_LE (ltag);
         }
@@ -1513,9 +1531,9 @@ gst_wavparse_stream_headers (GstWavParse * wav)
               buf = NULL;
               if (data_size > 0) {
                 if ((res =
-                        gst_pad_pull_range (wav->sinkpad, wav->offset,
+                        gst_wavparse_pull_range_exact (wav, wav->offset,
                             data_size, &buf)) != GST_FLOW_OK)
-                  goto header_read_error;
+                  goto header_pull_error;
               }
             }
             if (data_size > 0) {
@@ -1553,9 +1571,9 @@ gst_wavparse_stream_headers (GstWavParse * wav)
               buf = NULL;
               wav->offset += 12;
               if ((res =
-                      gst_pad_pull_range (wav->sinkpad, wav->offset,
+                      gst_wavparse_pull_range_exact (wav, wav->offset,
                           data_size, &buf)) != GST_FLOW_OK)
-                goto header_read_error;
+                goto header_pull_error;
               gst_buffer_map (buf, &map, GST_MAP_READ);
               gst_wavparse_adtl_chunk (wav, (const guint8 *) map.data,
                   data_size);
@@ -1599,9 +1617,9 @@ gst_wavparse_stream_headers (GstWavParse * wav)
           gst_buffer_unref (buf);
           buf = NULL;
           if ((res =
-                  gst_pad_pull_range (wav->sinkpad, wav->offset,
+                  gst_wavparse_pull_range_exact (wav, wav->offset,
                       data_size, &buf)) != GST_FLOW_OK)
-            goto header_read_error;
+            goto header_pull_error;
           gst_buffer_map (buf, &map, GST_MAP_READ);
           if (!gst_wavparse_cue_chunk (wav, (const guint8 *) map.data,
                   data_size)) {
@@ -1643,9 +1661,9 @@ gst_wavparse_stream_headers (GstWavParse * wav)
           gst_buffer_unref (buf);
           buf = NULL;
           if ((res =
-                  gst_pad_pull_range (wav->sinkpad, wav->offset,
+                  gst_wavparse_pull_range_exact (wav, wav->offset,
                       data_size, &buf)) != GST_FLOW_OK)
-            goto header_read_error;
+            goto header_pull_error;
           gst_buffer_map (buf, &map, GST_MAP_READ);
           if (!gst_wavparse_smpl_chunk (wav, (const guint8 *) map.data,
                   data_size)) {
@@ -1797,6 +1815,17 @@ header_read_error:
         ("Couldn't read in header %d (%s)", res, gst_flow_get_name (res)));
     goto fail;
   }
+header_pull_error:
+  {
+    if (res == GST_FLOW_EOS) {
+      GST_WARNING_OBJECT (wav, "Couldn't pull header %d (%s)", res,
+          gst_flow_get_name (res));
+    } else {
+      GST_ELEMENT_ERROR (wav, STREAM, DEMUX, (NULL),
+          ("Couldn't pull header %d (%s)", res, gst_flow_get_name (res)));
+    }
+    goto exit;
+  }
 }
 
 /*
-- 
GitLab


From 4c198f4891cfabde868944d55ff98925e7beb757 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Sebastian=20Dr=C3=B6ge?= <sebastian@centricular.com>
Date: Fri, 4 Oct 2024 13:09:43 +0300
Subject: [PATCH 2/7] wavparse: Make sure enough data for the tag list tag is
 available before parsing

Thanks to Antonio Morales for finding and reporting the issue.

Fixes GHSL-2024-258
Fixes https://gitlab.freedesktop.org/gstreamer/gstreamer/-/issues/3886

Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/8042>
---
 gst/wavparse/gstwavparse.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/gst/wavparse/gstwavparse.c b/gst/wavparse/gstwavparse.c
index 97d5591fae8f..21cb48c07eb3 100644
--- a/gst/wavparse/gstwavparse.c
+++ b/gst/wavparse/gstwavparse.c
@@ -1489,6 +1489,10 @@ gst_wavparse_stream_headers (GstWavParse * wav)
       case GST_RIFF_TAG_LIST:{
         guint32 ltag;
 
+        /* Need at least the ltag */
+        if (size < 4)
+          goto exit;
+
         if (wav->streaming) {
           const guint8 *data = NULL;
 
-- 
GitLab


From 296e17b4ea81e5c228bb853f6037b654fdca7d47 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Sebastian=20Dr=C3=B6ge?= <sebastian@centricular.com>
Date: Fri, 4 Oct 2024 13:15:27 +0300
Subject: [PATCH 3/7] wavparse: Fix parsing of acid chunk

Simply casting the bytes to a struct can lead to crashes because of unaligned
reads, and is also missing the endianness swapping that is necessary on big
endian architectures.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/8042>
---
 gst/wavparse/gstwavparse.c      | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/gst/wavparse/gstwavparse.c b/gst/wavparse/gstwavparse.c
index 21cb48c07eb3..6a0c44638ea2 100644
--- a/gst/wavparse/gstwavparse.c
+++ b/gst/wavparse/gstwavparse.c
@@ -1434,8 +1434,7 @@ gst_wavparse_stream_headers (GstWavParse * wav)
         break;
       }
       case GST_RIFF_TAG_acid:{
-        const gst_riff_acid *acid = NULL;
-        const guint data_size = sizeof (gst_riff_acid);
+        const guint data_size = 24;
         gfloat tempo;
 
         GST_INFO_OBJECT (wav, "Have acid chunk");
@@ -1449,13 +1448,13 @@ gst_wavparse_stream_headers (GstWavParse * wav)
           break;
         }
         if (wav->streaming) {
+          const guint8 *data;
           if (!gst_wavparse_peek_chunk (wav, &tag, &size)) {
             goto exit;
           }
           gst_adapter_flush (wav->adapter, 8);
-          acid = (const gst_riff_acid *) gst_adapter_map (wav->adapter,
-              data_size);
-          tempo = acid->tempo;
+          data = gst_adapter_map (wav->adapter, data_size);
+          tempo = GST_READ_FLOAT_LE (data + 20);
           gst_adapter_unmap (wav->adapter);
         } else {
           GstMapInfo map;
@@ -1466,8 +1465,7 @@ gst_wavparse_stream_headers (GstWavParse * wav)
                       &buf)) != GST_FLOW_OK)
             goto header_pull_error;
           gst_buffer_map (buf, &map, GST_MAP_READ);
-          acid = (const gst_riff_acid *) map.data;
-          tempo = acid->tempo;
+          tempo = GST_READ_FLOAT_LE (map.data + 20);
           gst_buffer_unmap (buf, &map);
         }
         /* send data as tags */
-- 
GitLab


From c72025cabdfcb2fe30d24eda7bb9d1d01a1b6555 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Sebastian=20Dr=C3=B6ge?= <sebastian@centricular.com>
Date: Fri, 4 Oct 2024 13:21:44 +0300
Subject: [PATCH 4/7] wavparse: Check that at least 4 bytes are available
 before parsing cue chunks

Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/8042>
---
 gst/wavparse/gstwavparse.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/gst/wavparse/gstwavparse.c b/gst/wavparse/gstwavparse.c
index 6a0c44638ea2..5655ee3825ca 100644
--- a/gst/wavparse/gstwavparse.c
+++ b/gst/wavparse/gstwavparse.c
@@ -790,6 +790,11 @@ gst_wavparse_cue_chunk (GstWavParse * wav, const guint8 * data, guint32 size)
     return TRUE;
   }
 
+  if (size < 4) {
+    GST_WARNING_OBJECT (wav, "broken file %d", size);
+    return FALSE;
+  }
+
   ncues = GST_READ_UINT32_LE (data);
 
   if (size < 4 + ncues * 24) {
-- 
GitLab


From 93d79c22a82604adc5512557c1238f72f41188c4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Sebastian=20Dr=C3=B6ge?= <sebastian@centricular.com>
Date: Fri, 4 Oct 2024 13:22:02 +0300
Subject: [PATCH 5/7] wavparse: Check that at least 32 bytes are available
 before parsing smpl chunks

Thanks to Antonio Morales for finding and reporting the issue.

Fixes GHSL-2024-259
Fixes https://gitlab.freedesktop.org/gstreamer/gstreamer/-/issues/3887

Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/8042>
---
 gst/wavparse/gstwavparse.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/gst/wavparse/gstwavparse.c b/gst/wavparse/gstwavparse.c
index 5655ee3825ca..8a04805ed427 100644
--- a/gst/wavparse/gstwavparse.c
+++ b/gst/wavparse/gstwavparse.c
@@ -894,6 +894,9 @@ gst_wavparse_smpl_chunk (GstWavParse * wav, const guint8 * data, guint32 size)
 {
   guint32 note_number;
 
+  if (size < 32)
+    return FALSE;
+
   /*
      manufacturer_id = GST_READ_UINT32_LE (data);
      product_id = GST_READ_UINT32_LE (data + 4);
-- 
GitLab


From 526d0eef0d850c8f2fa1bf0aef15a836797f1a67 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Sebastian=20Dr=C3=B6ge?= <sebastian@centricular.com>
Date: Fri, 4 Oct 2024 13:27:27 +0300
Subject: [PATCH 6/7] wavparse: Fix clipping of size to the file size

The size does not include the 8 bytes tag and length, so an additional 8 bytes
must be removed here. 8 bytes are always available at this point because
otherwise the parsing of the tag and length right above would've failed.

Thanks to Antonio Morales for finding and reporting the issue.

Fixes GHSL-2024-260
Fixes https://gitlab.freedesktop.org/gstreamer/gstreamer/-/issues/3888

Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/8042>
---
 gst/wavparse/gstwavparse.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/gst/wavparse/gstwavparse.c b/gst/wavparse/gstwavparse.c
index 8a04805ed427..998cbb276dbf 100644
--- a/gst/wavparse/gstwavparse.c
+++ b/gst/wavparse/gstwavparse.c
@@ -1338,10 +1338,11 @@ gst_wavparse_stream_headers (GstWavParse * wav)
     }
 
     /* Clip to upstream size if known */
-    if (upstream_size > 0 && size + wav->offset > upstream_size) {
+    if (upstream_size > 0 && size + 8 + wav->offset > upstream_size) {
       GST_WARNING_OBJECT (wav, "Clipping chunk size to file size");
       g_assert (upstream_size >= wav->offset);
-      size = upstream_size - wav->offset;
+      g_assert (upstream_size - wav->offset >= 8);
+      size = upstream_size - wav->offset - 8;
     }
 
     /* wav is a st00pid format, we don't know for sure where data starts.
-- 
GitLab


From 4f381d15014471b026020d0990a5f5a9f420a22b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Sebastian=20Dr=C3=B6ge?= <sebastian@centricular.com>
Date: Fri, 4 Oct 2024 13:51:00 +0300
Subject: [PATCH 7/7] wavparse: Check size before reading ds64 chunk

Thanks to Antonio Morales for finding and reporting the issue.

Fixes GHSL-2024-261
Fixes https://gitlab.freedesktop.org/gstreamer/gstreamer/-/issues/3889

Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/8042>
---
 gst/wavparse/gstwavparse.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/gst/wavparse/gstwavparse.c b/gst/wavparse/gstwavparse.c
index 998cbb276dbf..958868de6d9e 100644
--- a/gst/wavparse/gstwavparse.c
+++ b/gst/wavparse/gstwavparse.c
@@ -1088,6 +1088,11 @@ parse_ds64 (GstWavParse * wav, GstBuffer * buf)
   guint32 sampleCountLow, sampleCountHigh;
 
   gst_buffer_map (buf, &map, GST_MAP_READ);
+  if (map.size < 6 * 4) {
+    GST_WARNING_OBJECT (wav, "Too small ds64 chunk (%" G_GSIZE_FORMAT ")",
+        map.size);
+    return FALSE;
+  }
   dataSizeLow = GST_READ_UINT32_LE (map.data + 2 * 4);
   dataSizeHigh = GST_READ_UINT32_LE (map.data + 3 * 4);
   sampleCountLow = GST_READ_UINT32_LE (map.data + 4 * 4);
-- 
GitLab

