From 7dfaa57b6f9b55f17ffe824bd8988bb71ae11353 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Dr=C3=B6ge?= Date: Fri, 20 Oct 2023 00:09:57 +0300 Subject: [PATCH] mxfdemux: Store GstMXFDemuxEssenceTrack in their own fixed allocation Previously they were stored inline inside a GArray, but as references to the tracks were stored in various other places although the array could still be updated (and reallocated!), this could lead to dangling references in various places. Instead now store them in a GPtrArray in their own allocation so each track's memory position stays fixed. Fixes ZDI-CAN-22299 Fixes https://gitlab.freedesktop.org/gstreamer/gstreamer/-/issues/3055 Part-of: CVE: CVE-2023-44446 Upstream-Status: Backport [https://gitlab.freedesktop.org/gstreamer/gstreamer/-/commit/7dfaa57b6f9b55f1] Signed-off-by: Archana Polampalli --- gst/mxf/mxfdemux.c | 117 ++++++++++++++++++++------------------------- gst/mxf/mxfdemux.h | 2 +- 2 files changed, 52 insertions(+), 67 deletions(-) diff --git a/gst/mxf/mxfdemux.c b/gst/mxf/mxfdemux.c index b0ccc17..7eb990c 100644 --- a/gst/mxf/mxfdemux.c +++ b/gst/mxf/mxfdemux.c @@ -170,10 +170,25 @@ gst_mxf_demux_partition_free (GstMXFDemuxPartition * partition) } static void -gst_mxf_demux_reset_mxf_state (GstMXFDemux * demux) +gst_mxf_demux_essence_track_free (GstMXFDemuxEssenceTrack * t) { - guint i; + if (t->offsets) + g_array_free (t->offsets, TRUE); + + g_free (t->mapping_data); + + if (t->tags) + gst_tag_list_unref (t->tags); + + if (t->caps) + gst_caps_unref (t->caps); + + g_free (t); +} +static void +gst_mxf_demux_reset_mxf_state (GstMXFDemux * demux) +{ GST_DEBUG_OBJECT (demux, "Resetting MXF state"); g_list_foreach (demux->partitions, (GFunc) gst_mxf_demux_partition_free, @@ -183,22 +198,7 @@ gst_mxf_demux_reset_mxf_state (GstMXFDemux * demux) demux->current_partition = NULL; - for (i = 0; i < demux->essence_tracks->len; i++) { - GstMXFDemuxEssenceTrack *t = - &g_array_index (demux->essence_tracks, GstMXFDemuxEssenceTrack, i); - - if (t->offsets) - g_array_free (t->offsets, TRUE); - - g_free (t->mapping_data); - - if (t->tags) - gst_tag_list_unref (t->tags); - - if (t->caps) - gst_caps_unref (t->caps); - } - g_array_set_size (demux->essence_tracks, 0); + g_ptr_array_set_size (demux->essence_tracks, 0); } static void @@ -216,7 +216,7 @@ gst_mxf_demux_reset_linked_metadata (GstMXFDemux * demux) for (i = 0; i < demux->essence_tracks->len; i++) { GstMXFDemuxEssenceTrack *track = - &g_array_index (demux->essence_tracks, GstMXFDemuxEssenceTrack, i); + g_ptr_array_index (demux->essence_tracks, i); track->source_package = NULL; track->delta_id = -1; @@ -419,7 +419,7 @@ gst_mxf_demux_partition_postcheck (GstMXFDemux * demux, for (i = 0; i < demux->essence_tracks->len; i++) { GstMXFDemuxEssenceTrack *cand = - &g_array_index (demux->essence_tracks, GstMXFDemuxEssenceTrack, i); + g_ptr_array_index (demux->essence_tracks, i); if (cand->body_sid != partition->partition.body_sid) continue; @@ -866,8 +866,7 @@ gst_mxf_demux_update_essence_tracks (GstMXFDemux * demux) for (k = 0; k < demux->essence_tracks->len; k++) { GstMXFDemuxEssenceTrack *tmp = - &g_array_index (demux->essence_tracks, GstMXFDemuxEssenceTrack, - k); + g_ptr_array_index (demux->essence_tracks, k); if (tmp->track_number == track->parent.track_number && tmp->body_sid == edata->body_sid) { @@ -885,24 +884,24 @@ gst_mxf_demux_update_essence_tracks (GstMXFDemux * demux) } if (!etrack) { - GstMXFDemuxEssenceTrack tmp; + GstMXFDemuxEssenceTrack *tmp = g_new0 (GstMXFDemuxEssenceTrack, 1); + + tmp->body_sid = edata->body_sid; + tmp->index_sid = edata->index_sid; + tmp->track_number = track->parent.track_number; + tmp->track_id = track->parent.track_id; + memcpy (&tmp->source_package_uid, &package->parent.package_uid, 32); - memset (&tmp, 0, sizeof (tmp)); - tmp.body_sid = edata->body_sid; - tmp.index_sid = edata->index_sid; - tmp.track_number = track->parent.track_number; - tmp.track_id = track->parent.track_id; - memcpy (&tmp.source_package_uid, &package->parent.package_uid, 32); if (demux->current_partition->partition.body_sid == edata->body_sid && demux->current_partition->partition.body_offset == 0) - tmp.position = 0; + tmp->position = 0; else - tmp.position = -1; + tmp->position = -1; - g_array_append_val (demux->essence_tracks, tmp); + g_ptr_array_add (demux->essence_tracks, tmp); etrack = - &g_array_index (demux->essence_tracks, GstMXFDemuxEssenceTrack, + g_ptr_array_index (demux->essence_tracks, demux->essence_tracks->len - 1); new = TRUE; } @@ -1050,13 +1049,7 @@ gst_mxf_demux_update_essence_tracks (GstMXFDemux * demux) next: if (new) { - g_free (etrack->mapping_data); - if (etrack->tags) - gst_tag_list_unref (etrack->tags); - if (etrack->caps) - gst_caps_unref (etrack->caps); - - g_array_remove_index (demux->essence_tracks, + g_ptr_array_remove_index (demux->essence_tracks, demux->essence_tracks->len - 1); } } @@ -1069,7 +1062,8 @@ gst_mxf_demux_update_essence_tracks (GstMXFDemux * demux) for (i = 0; i < demux->essence_tracks->len; i++) { GstMXFDemuxEssenceTrack *etrack = - &g_array_index (demux->essence_tracks, GstMXFDemuxEssenceTrack, i); + g_ptr_array_index (demux->essence_tracks, i); + if (!etrack->source_package || !etrack->source_track || !etrack->caps) { GST_ERROR_OBJECT (demux, "Failed to update essence track %u", i); @@ -1438,7 +1432,7 @@ gst_mxf_demux_update_tracks (GstMXFDemux * demux) for (k = 0; k < demux->essence_tracks->len; k++) { GstMXFDemuxEssenceTrack *tmp = - &g_array_index (demux->essence_tracks, GstMXFDemuxEssenceTrack, k); + g_ptr_array_index (demux->essence_tracks, k); if (tmp->source_package == source_package && tmp->source_track == source_track) { @@ -1927,8 +1921,7 @@ gst_mxf_demux_pad_set_component (GstMXFDemux * demux, GstMXFDemuxPad * pad, pad->current_essence_track = NULL; for (k = 0; k < demux->essence_tracks->len; k++) { - GstMXFDemuxEssenceTrack *tmp = - &g_array_index (demux->essence_tracks, GstMXFDemuxEssenceTrack, k); + GstMXFDemuxEssenceTrack *tmp = g_ptr_array_index (demux->essence_tracks, k); if (tmp->source_package == source_package && tmp->source_track == source_track) { @@ -2712,7 +2705,7 @@ gst_mxf_demux_handle_generic_container_essence_element (GstMXFDemux * demux, if (!etrack) { for (i = 0; i < demux->essence_tracks->len; i++) { GstMXFDemuxEssenceTrack *tmp = - &g_array_index (demux->essence_tracks, GstMXFDemuxEssenceTrack, i); + g_ptr_array_index (demux->essence_tracks, i); if (tmp->body_sid == demux->current_partition->partition.body_sid && (tmp->track_number == track_number || tmp->track_number == 0)) { @@ -3933,8 +3926,7 @@ from_track_offset: gst_mxf_demux_set_partition_for_offset (demux, demux->offset); for (i = 0; i < demux->essence_tracks->len; i++) { - GstMXFDemuxEssenceTrack *t = - &g_array_index (demux->essence_tracks, GstMXFDemuxEssenceTrack, i); + GstMXFDemuxEssenceTrack *t = g_ptr_array_index (demux->essence_tracks, i); if (index_start_position != -1 && t == etrack) t->position = index_start_position; @@ -3958,8 +3950,7 @@ from_track_offset: /* Handle EOS */ for (i = 0; i < demux->essence_tracks->len; i++) { GstMXFDemuxEssenceTrack *t = - &g_array_index (demux->essence_tracks, GstMXFDemuxEssenceTrack, - i); + g_ptr_array_index (demux->essence_tracks, i); if (t->position > 0) t->duration = t->position; @@ -4197,8 +4188,7 @@ gst_mxf_demux_pull_and_handle_klv_packet (GstMXFDemux * demux) guint i; for (i = 0; i < demux->essence_tracks->len; i++) { GstMXFDemuxEssenceTrack *etrack = - &g_array_index (demux->essence_tracks, GstMXFDemuxEssenceTrack, - i); + g_ptr_array_index (demux->essence_tracks, i); if (etrack->body_sid != partition->partition.body_sid) continue; @@ -4669,9 +4659,8 @@ gst_mxf_demux_pad_to_track_and_position (GstMXFDemux * demux, /* Get the corresponding essence track for the given source package and stream id */ for (i = 0; i < demux->essence_tracks->len; i++) { GstMXFDemuxEssenceTrack *track = - &g_array_index (demux->essence_tracks, GstMXFDemuxEssenceTrack, i); - GST_LOG_OBJECT (pad, - "Looking at essence track body_sid:%d index_sid:%d", + g_ptr_array_index (demux->essence_tracks, i); + GST_LOG_OBJECT (pad, "Looking at essence track body_sid:%d index_sid:%d", track->body_sid, track->index_sid); if (clip->source_track_id == 0 || (track->track_id == clip->source_track_id && mxf_umid_is_equal (&clip->source_package_id, @@ -4920,8 +4909,7 @@ gst_mxf_demux_seek_push (GstMXFDemux * demux, GstEvent * event) } for (i = 0; i < demux->essence_tracks->len; i++) { - GstMXFDemuxEssenceTrack *t = - &g_array_index (demux->essence_tracks, GstMXFDemuxEssenceTrack, i); + GstMXFDemuxEssenceTrack *t = g_ptr_array_index (demux->essence_tracks, i); t->position = -1; } @@ -5359,8 +5347,7 @@ gst_mxf_demux_seek_pull (GstMXFDemux * demux, GstEvent * event) } for (i = 0; i < demux->essence_tracks->len; i++) { - GstMXFDemuxEssenceTrack *t = - &g_array_index (demux->essence_tracks, GstMXFDemuxEssenceTrack, i); + GstMXFDemuxEssenceTrack *t = g_ptr_array_index (demux->essence_tracks, i); t->position = -1; } @@ -5659,7 +5646,7 @@ gst_mxf_demux_sink_event (GstPad * pad, GstObject * parent, GstEvent * event) for (i = 0; i < demux->essence_tracks->len; i++) { GstMXFDemuxEssenceTrack *t = - &g_array_index (demux->essence_tracks, GstMXFDemuxEssenceTrack, i); + g_ptr_array_index (demux->essence_tracks, i); if (t->position > 0) t->duration = t->position; @@ -5700,8 +5687,7 @@ gst_mxf_demux_sink_event (GstPad * pad, GstObject * parent, GstEvent * event) for (i = 0; i < demux->essence_tracks->len; i++) { GstMXFDemuxEssenceTrack *etrack = - &g_array_index (demux->essence_tracks, GstMXFDemuxEssenceTrack, - i); + g_ptr_array_index (demux->essence_tracks, i); etrack->position = -1; } ret = TRUE; @@ -5725,8 +5711,7 @@ gst_mxf_demux_sink_event (GstPad * pad, GstObject * parent, GstEvent * event) for (i = 0; i < demux->essence_tracks->len; i++) { GstMXFDemuxEssenceTrack *t = - &g_array_index (demux->essence_tracks, GstMXFDemuxEssenceTrack, - i); + g_ptr_array_index (demux->essence_tracks, i); t->position = -1; } demux->current_partition = NULL; @@ -5999,7 +5984,7 @@ gst_mxf_demux_finalize (GObject * object) g_ptr_array_free (demux->src, TRUE); demux->src = NULL; - g_array_free (demux->essence_tracks, TRUE); + g_ptr_array_free (demux->essence_tracks, TRUE); demux->essence_tracks = NULL; g_hash_table_destroy (demux->metadata); @@ -6076,8 +6061,8 @@ gst_mxf_demux_init (GstMXFDemux * demux) g_rw_lock_init (&demux->metadata_lock); demux->src = g_ptr_array_new (); - demux->essence_tracks = - g_array_new (FALSE, FALSE, sizeof (GstMXFDemuxEssenceTrack)); + demux->essence_tracks = g_ptr_array_new_with_free_func ((GDestroyNotify) + gst_mxf_demux_essence_track_free); gst_segment_init (&demux->segment, GST_FORMAT_TIME); diff --git a/gst/mxf/mxfdemux.h b/gst/mxf/mxfdemux.h index d079a1d..1dc8a4e 100644 --- a/gst/mxf/mxfdemux.h +++ b/gst/mxf/mxfdemux.h @@ -266,7 +266,7 @@ struct _GstMXFDemux GList *partitions; GstMXFDemuxPartition *current_partition; - GArray *essence_tracks; + GPtrArray *essence_tracks; GList *pending_index_table_segments; GList *index_tables; /* one per BodySID / IndexSID */ -- 2.40.0