From 97294eb8bbed1b9dad7d3f2c52dd69eb1812cc06 Mon Sep 17 00:00:00 2001 From: Edward Hervey Date: Fri, 25 Nov 2016 09:44:05 +0100 Subject: [PATCH] asfdemux: Add check for invalid/corrupt asf object An asf object can't realistically be bigger than 2**32 bytes. If it reports a size bigger than that, consider it corrupt and properly propagate the error back. https://bugzilla.gnome.org/show_bug.cgi?id=775070 --- gst/asfdemux/gstasfdemux.c | 100 ++++++++++++++++++++++++++++++++------------- 1 file changed, 72 insertions(+), 28 deletions(-) diff --git a/gst/asfdemux/gstasfdemux.c b/gst/asfdemux/gstasfdemux.c index e04f5be..c6d39b9 100644 --- a/gst/asfdemux/gstasfdemux.c +++ b/gst/asfdemux/gstasfdemux.c @@ -101,7 +101,7 @@ static void gst_asf_demux_process_queued_extended_stream_objects (GstASFDemux * demux); static gboolean gst_asf_demux_pull_headers (GstASFDemux * demux, GstFlowReturn * pflow); -static void gst_asf_demux_pull_indices (GstASFDemux * demux); +static GstFlowReturn gst_asf_demux_pull_indices (GstASFDemux * demux); static void gst_asf_demux_reset_stream_state_after_discont (GstASFDemux * asf); static gboolean gst_asf_demux_parse_data_object_start (GstASFDemux * demux, guint8 * data); @@ -896,19 +896,24 @@ gst_asf_demux_identify_guid (const ASFGuidHash * guids, ASFGuid * guid) typedef struct { AsfObjectID id; - guint64 size; + guint32 size; } AsfObject; -/* expect is true when the user is expeting an object, - * when false, it will give no warnings if the object - * is not identified +/* Peek for an object. + * + * Returns FALSE is the object is corrupted (such as the reported + * object size being greater than 2**32bits. */ static gboolean asf_demux_peek_object (GstASFDemux * demux, const guint8 * data, guint data_len, AsfObject * object, gboolean expect) { ASFGuid guid; + guint64 tmp_size; + + /* Callers should have made sure that data_len is big enough */ + g_assert (data_len >= ASF_OBJECT_HEADER_SIZE); if (data_len < ASF_OBJECT_HEADER_SIZE) return FALSE; @@ -918,7 +923,13 @@ asf_demux_peek_object (GstASFDemux * demux, const guint8 * data, guid.v3 = GST_READ_UINT32_LE (data + 8); guid.v4 = GST_READ_UINT32_LE (data + 12); - object->size = GST_READ_UINT64_LE (data + 16); + tmp_size = GST_READ_UINT64_LE (data + 16); + if (tmp_size >= G_MAXUINT) { + GST_WARNING_OBJECT (demux, + "ASF Object size corrupted (greater than 32bit)"); + return FALSE; + } + object->size = tmp_size; /* FIXME: make asf_demux_identify_object_guid() */ object->id = gst_asf_demux_identify_guid (asf_object_guids, &guid); @@ -949,17 +960,18 @@ gst_asf_demux_release_old_pads (GstASFDemux * demux) static GstFlowReturn gst_asf_demux_chain_headers (GstASFDemux * demux) { - GstFlowReturn flow; AsfObject obj; guint8 *header_data, *data = NULL; const guint8 *cdata = NULL; guint64 header_size; + GstFlowReturn flow = GST_FLOW_OK; cdata = (guint8 *) gst_adapter_map (demux->adapter, ASF_OBJECT_HEADER_SIZE); if (cdata == NULL) goto need_more_data; - asf_demux_peek_object (demux, cdata, ASF_OBJECT_HEADER_SIZE, &obj, TRUE); + if (!asf_demux_peek_object (demux, cdata, ASF_OBJECT_HEADER_SIZE, &obj, TRUE)) + goto parse_failed; if (obj.id != ASF_OBJ_HEADER) goto wrong_type; @@ -1054,29 +1066,36 @@ gst_asf_demux_pull_data (GstASFDemux * demux, guint64 offset, guint size, return TRUE; } -static void +static GstFlowReturn gst_asf_demux_pull_indices (GstASFDemux * demux) { GstBuffer *buf = NULL; guint64 offset; guint num_read = 0; + GstFlowReturn ret = GST_FLOW_OK; offset = demux->index_offset; if (G_UNLIKELY (offset == 0)) { GST_DEBUG_OBJECT (demux, "can't read indices, don't know index offset"); - return; + /* non-fatal */ + return GST_FLOW_OK; } while (gst_asf_demux_pull_data (demux, offset, 16 + 8, &buf, NULL)) { - GstFlowReturn flow; AsfObject obj; GstMapInfo map; guint8 *bufdata; + guint64 obj_size; gst_buffer_map (buf, &map, GST_MAP_READ); g_assert (map.size >= 16 + 8); - asf_demux_peek_object (demux, map.data, 16 + 8, &obj, TRUE); + if (!asf_demux_peek_object (demux, map.data, 16 + 8, &obj, TRUE)) { + gst_buffer_unmap (buf, &map); + gst_buffer_replace (&buf, NULL); + ret = GST_FLOW_ERROR; + break; + } gst_buffer_unmap (buf, &map); gst_buffer_replace (&buf, NULL); @@ -1098,16 +1117,19 @@ gst_asf_demux_pull_indices (GstASFDemux * demux) gst_buffer_map (buf, &map, GST_MAP_READ); g_assert (map.size >= obj.size); bufdata = (guint8 *) map.data; - flow = gst_asf_demux_process_object (demux, &bufdata, &obj.size); + obj_size = obj.size; + ret = gst_asf_demux_process_object (demux, &bufdata, &obj_size); gst_buffer_unmap (buf, &map); gst_buffer_replace (&buf, NULL); - if (G_UNLIKELY (flow != GST_FLOW_OK)) + if (G_UNLIKELY (ret != GST_FLOW_OK)) break; ++num_read; } + GST_DEBUG_OBJECT (demux, "read %u index objects", num_read); + return ret; } static gboolean @@ -1115,7 +1137,10 @@ gst_asf_demux_parse_data_object_start (GstASFDemux * demux, guint8 * data) { AsfObject obj; - asf_demux_peek_object (demux, data, 50, &obj, TRUE); + if (!asf_demux_peek_object (demux, data, 50, &obj, TRUE)) { + GST_WARNING_OBJECT (demux, "Corrupted data"); + return FALSE; + } if (obj.id != ASF_OBJ_DATA) { GST_WARNING_OBJECT (demux, "headers not followed by a DATA object"); return FALSE; @@ -1179,14 +1204,19 @@ gst_asf_demux_pull_headers (GstASFDemux * demux, GstFlowReturn * pflow) gst_buffer_map (buf, &map, GST_MAP_READ); g_assert (map.size >= 16 + 8); - asf_demux_peek_object (demux, map.data, 16 + 8, &obj, TRUE); + if (!asf_demux_peek_object (demux, map.data, 16 + 8, &obj, TRUE)) { + gst_buffer_unmap (buf, &map); + gst_buffer_replace (&buf, NULL); + flow = GST_FLOW_ERROR; + goto read_failed; + } gst_buffer_unmap (buf, &map); gst_buffer_replace (&buf, NULL); if (obj.id != ASF_OBJ_HEADER) goto wrong_type; - GST_LOG_OBJECT (demux, "header size = %u", (guint) obj.size); + GST_LOG_OBJECT (demux, "header size = %u", obj.size); /* pull HEADER object */ if (!gst_asf_demux_pull_data (demux, demux->base_offset, obj.size, &buf, @@ -1895,6 +1925,7 @@ gst_asf_demux_check_buffer_is_header (GstASFDemux * demux, GstBuffer * buf) { AsfObject obj; GstMapInfo map; + gboolean valid; g_assert (buf != NULL); GST_LOG_OBJECT (demux, "Checking if buffer is a header"); @@ -1908,9 +1939,11 @@ gst_asf_demux_check_buffer_is_header (GstASFDemux * demux, GstBuffer * buf) } /* check if it is a header */ - asf_demux_peek_object (demux, map.data, ASF_OBJECT_HEADER_SIZE, &obj, TRUE); + valid = + asf_demux_peek_object (demux, map.data, ASF_OBJECT_HEADER_SIZE, &obj, + TRUE); gst_buffer_unmap (buf, &map); - if (obj.id == ASF_OBJ_HEADER) { + if (valid && obj.id == ASF_OBJ_HEADER) { return TRUE; } return FALSE; @@ -1953,7 +1986,9 @@ gst_asf_demux_loop (GstASFDemux * demux) goto pause; } - gst_asf_demux_pull_indices (demux); + flow = gst_asf_demux_pull_indices (demux); + if (flow != GST_FLOW_OK) + goto pause; } g_assert (demux->state == GST_ASF_DEMUX_STATE_DATA); @@ -2186,12 +2221,11 @@ gst_asf_demux_check_header (GstASFDemux * demux) if (cdata == NULL) /* need more data */ return GST_ASF_DEMUX_CHECK_HEADER_NEED_DATA; - asf_demux_peek_object (demux, cdata, ASF_OBJECT_HEADER_SIZE, &obj, FALSE); - if (obj.id != ASF_OBJ_HEADER) { - return GST_ASF_DEMUX_CHECK_HEADER_NO; - } else { + if (asf_demux_peek_object (demux, cdata, ASF_OBJECT_HEADER_SIZE, &obj, FALSE + && obj.id == ASF_OBJ_HEADER)) return GST_ASF_DEMUX_CHECK_HEADER_YES; - } + + return GST_ASF_DEMUX_CHECK_HEADER_NO; } static GstFlowReturn @@ -4117,9 +4151,12 @@ gst_asf_demux_process_ext_stream_props (GstASFDemux * demux, guint8 * data, goto done; } + if (size < ASF_OBJECT_HEADER_SIZE) + goto not_enough_data; + /* get size of the stream object */ if (!asf_demux_peek_object (demux, data, size, &stream_obj, TRUE)) - goto not_enough_data; + goto corrupted_stream; if (stream_obj.id != ASF_OBJ_STREAM) goto expected_stream_object; @@ -4199,6 +4236,11 @@ expected_stream_object: gst_asf_get_guid_nick (asf_object_guids, stream_obj.id)); return GST_FLOW_OK; /* not absolutely fatal */ } +corrupted_stream: + { + GST_WARNING_OBJECT (demux, "Corrupted stream"); + return GST_FLOW_ERROR; + } } static const gchar * @@ -4332,7 +4374,9 @@ gst_asf_demux_process_object (GstASFDemux * demux, guint8 ** p_data, if (*p_size < ASF_OBJECT_HEADER_SIZE) return ASF_FLOW_NEED_MORE_DATA; - asf_demux_peek_object (demux, *p_data, ASF_OBJECT_HEADER_SIZE, &obj, TRUE); + if (!asf_demux_peek_object (demux, *p_data, ASF_OBJECT_HEADER_SIZE, &obj, + TRUE)) + return GST_FLOW_ERROR; gst_asf_demux_skip_bytes (ASF_OBJECT_HEADER_SIZE, p_data, p_size); obj_data_size = obj.size - ASF_OBJECT_HEADER_SIZE; @@ -4342,7 +4386,7 @@ gst_asf_demux_process_object (GstASFDemux * demux, guint8 ** p_data, gst_asf_demux_push_obj (demux, obj.id); - GST_INFO ("%s: size %" G_GUINT64_FORMAT, demux->objpath, obj.size); + GST_INFO ("%s: size %u", demux->objpath, obj.size); switch (obj.id) { case ASF_OBJ_STREAM: -- 2.7.4