1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
|
From 9181191511f9c0be6a89c98b311f49d66bd46dc3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Sebastian=20Dr=C3=B6ge?= <sebastian@centricular.com>
Date: Thu, 4 Mar 2021 13:05:19 +0200
Subject: [PATCH] matroskademux: Fix extraction of multichannel WavPack
The old code had a couple of issues that all lead to potential memory
safety bugs.
- Use a constant for the Wavpack4Header size instead of using sizeof.
It's written out into the data and not from the struct and who knows
what special alignment/padding requirements some C compilers have.
- gst_buffer_set_size() does not realloc the buffer when setting a
bigger size than allocated, it only allows growing up to the maximum
allocated size. Instead use a GstAdapter to collect all the blocks
and take out everything at once in the end.
- Check that enough data is actually available in the input and
otherwise handle it an error in all cases instead of silently
ignoring it.
Among other things this fixes out of bounds writes because the code
assumed gst_buffer_set_size() can grow the buffer and simply wrote after
the end of the buffer.
Thanks to Natalie Silvanovich for reporting.
Fixes https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/-/issues/859
Part-of: <https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/-/merge_requests/903>
Upstream-Status: Backport
https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/-/commit/9181191511f9c0be6a89c98b311f49d66bd46dc3?merge_request_iid=903
CVE: CVE-2021-3497
Signed-off-by: Chee Yang Lee <chee.yang.lee@intel.com>
---
gst/matroska/matroska-demux.c | 99 +++++++++++++++++++----------------
gst/matroska/matroska-ids.h | 2 +
2 files changed, 55 insertions(+), 46 deletions(-)
diff --git a/gst/matroska/matroska-demux.c b/gst/matroska/matroska-demux.c
index 467815986..0e47ee7b5 100644
--- a/gst/matroska/matroska-demux.c
+++ b/gst/matroska/matroska-demux.c
@@ -3851,6 +3851,12 @@ gst_matroska_demux_add_wvpk_header (GstElement * element,
guint32 block_samples, tmp;
gsize size = gst_buffer_get_size (*buf);
+ if (size < 4) {
+ GST_ERROR_OBJECT (element, "Too small wavpack buffer");
+ gst_buffer_unmap (*buf, &map);
+ return GST_FLOW_ERROR;
+ }
+
gst_buffer_extract (*buf, 0, &tmp, sizeof (guint32));
block_samples = GUINT32_FROM_LE (tmp);
/* we need to reconstruct the header of the wavpack block */
@@ -3858,10 +3864,10 @@ gst_matroska_demux_add_wvpk_header (GstElement * element,
/* -20 because ck_size is the size of the wavpack block -8
* and lace_size is the size of the wavpack block + 12
* (the three guint32 of the header that already are in the buffer) */
- wvh.ck_size = size + sizeof (Wavpack4Header) - 20;
+ wvh.ck_size = size + WAVPACK4_HEADER_SIZE - 20;
/* block_samples, flags and crc are already in the buffer */
- newbuf = gst_buffer_new_allocate (NULL, sizeof (Wavpack4Header) - 12, NULL);
+ newbuf = gst_buffer_new_allocate (NULL, WAVPACK4_HEADER_SIZE - 12, NULL);
gst_buffer_map (newbuf, &outmap, GST_MAP_WRITE);
data = outmap.data;
@@ -3886,9 +3892,11 @@ gst_matroska_demux_add_wvpk_header (GstElement * element,
audiocontext->wvpk_block_index += block_samples;
} else {
guint8 *outdata = NULL;
- guint outpos = 0;
- gsize buf_size, size, out_size = 0;
+ gsize buf_size, size;
guint32 block_samples, flags, crc, blocksize;
+ GstAdapter *adapter;
+
+ adapter = gst_adapter_new ();
gst_buffer_map (*buf, &map, GST_MAP_READ);
buf_data = map.data;
@@ -3897,6 +3905,7 @@ gst_matroska_demux_add_wvpk_header (GstElement * element,
if (buf_size < 4) {
GST_ERROR_OBJECT (element, "Too small wavpack buffer");
gst_buffer_unmap (*buf, &map);
+ g_object_unref (adapter);
return GST_FLOW_ERROR;
}
@@ -3918,59 +3927,57 @@ gst_matroska_demux_add_wvpk_header (GstElement * element,
data += 4;
size -= 4;
- if (blocksize == 0 || size < blocksize)
- break;
-
- g_assert ((newbuf == NULL) == (outdata == NULL));
+ if (blocksize == 0 || size < blocksize) {
+ GST_ERROR_OBJECT (element, "Too small wavpack buffer");
+ gst_buffer_unmap (*buf, &map);
+ g_object_unref (adapter);
+ return GST_FLOW_ERROR;
+ }
- if (newbuf == NULL) {
- out_size = sizeof (Wavpack4Header) + blocksize;
- newbuf = gst_buffer_new_allocate (NULL, out_size, NULL);
+ g_assert (newbuf == NULL);
- gst_buffer_copy_into (newbuf, *buf,
- GST_BUFFER_COPY_TIMESTAMPS | GST_BUFFER_COPY_FLAGS, 0, -1);
+ newbuf =
+ gst_buffer_new_allocate (NULL, WAVPACK4_HEADER_SIZE + blocksize,
+ NULL);
+ gst_buffer_map (newbuf, &outmap, GST_MAP_WRITE);
+ outdata = outmap.data;
+
+ outdata[0] = 'w';
+ outdata[1] = 'v';
+ outdata[2] = 'p';
+ outdata[3] = 'k';
+ outdata += 4;
+
+ GST_WRITE_UINT32_LE (outdata, blocksize + WAVPACK4_HEADER_SIZE - 8);
+ GST_WRITE_UINT16_LE (outdata + 4, wvh.version);
+ GST_WRITE_UINT8 (outdata + 6, wvh.track_no);
+ GST_WRITE_UINT8 (outdata + 7, wvh.index_no);
+ GST_WRITE_UINT32_LE (outdata + 8, wvh.total_samples);
+ GST_WRITE_UINT32_LE (outdata + 12, wvh.block_index);
+ GST_WRITE_UINT32_LE (outdata + 16, block_samples);
+ GST_WRITE_UINT32_LE (outdata + 20, flags);
+ GST_WRITE_UINT32_LE (outdata + 24, crc);
+ outdata += 28;
+
+ memcpy (outdata, data, blocksize);
- outpos = 0;
- gst_buffer_map (newbuf, &outmap, GST_MAP_WRITE);
- outdata = outmap.data;
- } else {
- gst_buffer_unmap (newbuf, &outmap);
- out_size += sizeof (Wavpack4Header) + blocksize;
- gst_buffer_set_size (newbuf, out_size);
- gst_buffer_map (newbuf, &outmap, GST_MAP_WRITE);
- outdata = outmap.data;
- }
+ gst_buffer_unmap (newbuf, &outmap);
+ gst_adapter_push (adapter, newbuf);
+ newbuf = NULL;
- outdata[outpos] = 'w';
- outdata[outpos + 1] = 'v';
- outdata[outpos + 2] = 'p';
- outdata[outpos + 3] = 'k';
- outpos += 4;
-
- GST_WRITE_UINT32_LE (outdata + outpos,
- blocksize + sizeof (Wavpack4Header) - 8);
- GST_WRITE_UINT16_LE (outdata + outpos + 4, wvh.version);
- GST_WRITE_UINT8 (outdata + outpos + 6, wvh.track_no);
- GST_WRITE_UINT8 (outdata + outpos + 7, wvh.index_no);
- GST_WRITE_UINT32_LE (outdata + outpos + 8, wvh.total_samples);
- GST_WRITE_UINT32_LE (outdata + outpos + 12, wvh.block_index);
- GST_WRITE_UINT32_LE (outdata + outpos + 16, block_samples);
- GST_WRITE_UINT32_LE (outdata + outpos + 20, flags);
- GST_WRITE_UINT32_LE (outdata + outpos + 24, crc);
- outpos += 28;
-
- memmove (outdata + outpos, data, blocksize);
- outpos += blocksize;
data += blocksize;
size -= blocksize;
}
gst_buffer_unmap (*buf, &map);
- gst_buffer_unref (*buf);
- if (newbuf)
- gst_buffer_unmap (newbuf, &outmap);
+ newbuf = gst_adapter_take_buffer (adapter, gst_adapter_available (adapter));
+ g_object_unref (adapter);
+ gst_buffer_copy_into (newbuf, *buf,
+ GST_BUFFER_COPY_TIMESTAMPS | GST_BUFFER_COPY_FLAGS, 0, -1);
+ gst_buffer_unref (*buf);
*buf = newbuf;
+
audiocontext->wvpk_block_index += block_samples;
}
diff --git a/gst/matroska/matroska-ids.h b/gst/matroska/matroska-ids.h
index 429213f77..8d4a685a9 100644
--- a/gst/matroska/matroska-ids.h
+++ b/gst/matroska/matroska-ids.h
@@ -688,6 +688,8 @@ typedef struct _Wavpack4Header {
guint32 crc; /* crc for actual decoded data */
} Wavpack4Header;
+#define WAVPACK4_HEADER_SIZE (32)
+
typedef enum {
GST_MATROSKA_TRACK_ENCODING_SCOPE_FRAME = (1<<0),
GST_MATROSKA_TRACK_ENCODING_SCOPE_CODEC_DATA = (1<<1),
--
GitLab
|
