From 9fe2e1c5ebae44a32530998077a26b3f7db82352 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Olivier=20Cr=C3=AAte?= Date: Mon, 30 May 2022 16:31:38 -0400 Subject: [PATCH] webrtcbin: Reject answers that don't contain the same number of m-line as offer Otherwise, it segfaults later. Also add test to validate this. Part-of: --- .../gst-plugins-bad/ext/webrtc/gstwebrtcbin.c | 56 ++++++++++++--- .../tests/check/elements/webrtcbin.c | 70 +++++++++++++++++++ 2 files changed, 117 insertions(+), 9 deletions(-) diff --git a/subprojects/gst-plugins-bad/ext/webrtc/gstwebrtcbin.c b/subprojects/gst-plugins-bad/ext/webrtc/gstwebrtcbin.c index df7b86ce3b..6278bfbdb1 100644 --- a/subprojects/gst-plugins-bad/ext/webrtc/gstwebrtcbin.c +++ b/subprojects/gst-plugins-bad/ext/webrtc/gstwebrtcbin.c @@ -5911,18 +5911,16 @@ done: return ret; } -static gboolean -check_transceivers_not_removed (GstWebRTCBin * webrtc, +static gint +transceivers_media_num_cmp (GstWebRTCBin * webrtc, GstWebRTCSessionDescription * previous, GstWebRTCSessionDescription * new) { if (!previous) - return TRUE; + return 0; - if (gst_sdp_message_medias_len (previous->sdp) > - gst_sdp_message_medias_len (new->sdp)) - return FALSE; + return gst_sdp_message_medias_len (new->sdp) - + gst_sdp_message_medias_len (previous->sdp); - return TRUE; } static gboolean @@ -6016,6 +6014,35 @@ get_previous_description (GstWebRTCBin * webrtc, SDPSource source, return NULL; } +static GstWebRTCSessionDescription * +get_last_generated_description (GstWebRTCBin * webrtc, SDPSource source, + GstWebRTCSDPType type) +{ + switch (type) { + case GST_WEBRTC_SDP_TYPE_OFFER: + if (source == SDP_REMOTE) + return webrtc->priv->last_generated_answer; + else + return webrtc->priv->last_generated_offer; + break; + case GST_WEBRTC_SDP_TYPE_PRANSWER: + case GST_WEBRTC_SDP_TYPE_ANSWER: + if (source == SDP_LOCAL) + return webrtc->priv->last_generated_answer; + else + return webrtc->priv->last_generated_offer; + case GST_WEBRTC_SDP_TYPE_ROLLBACK: + return NULL; + default: + /* other values mean memory corruption/uninitialized! */ + g_assert_not_reached (); + break; + } + + return NULL; +} + + /* http://w3c.github.io/webrtc-pc/#set-description */ static GstStructure * _set_description_task (GstWebRTCBin * webrtc, struct set_description *sd) @@ -6056,9 +6083,9 @@ _set_description_task (GstWebRTCBin * webrtc, struct set_description *sd) } } - if (!check_transceivers_not_removed (webrtc, + if (transceivers_media_num_cmp (webrtc, get_previous_description (webrtc, sd->source, sd->sdp->type), - sd->sdp)) { + sd->sdp) < 0) { g_set_error_literal (&error, GST_WEBRTC_ERROR, GST_WEBRTC_ERROR_SDP_SYNTAX_ERROR, "m=lines removed from the SDP. Processing a completely new connection " @@ -6066,6 +6093,17 @@ _set_description_task (GstWebRTCBin * webrtc, struct set_description *sd) goto out; } + if ((sd->sdp->type == GST_WEBRTC_SDP_TYPE_PRANSWER || + sd->sdp->type == GST_WEBRTC_SDP_TYPE_ANSWER) && + transceivers_media_num_cmp (webrtc, + get_last_generated_description (webrtc, sd->source, sd->sdp->type), + sd->sdp) != 0) { + g_set_error_literal (&error, GST_WEBRTC_ERROR, + GST_WEBRTC_ERROR_SDP_SYNTAX_ERROR, + "Answer doesn't have the same number of m-lines as the offer."); + goto out; + } + if (!check_locked_mlines (webrtc, sd->sdp, &error)) goto out; diff --git a/subprojects/gst-plugins-bad/tests/check/elements/webrtcbin.c b/subprojects/gst-plugins-bad/tests/check/elements/webrtcbin.c index 16abe97f6a..dfce0378c0 100644 --- a/subprojects/gst-plugins-bad/tests/check/elements/webrtcbin.c +++ b/subprojects/gst-plugins-bad/tests/check/elements/webrtcbin.c @@ -5310,6 +5310,75 @@ GST_START_TEST (test_bundle_multiple_media_rtx_payload_mapping) } GST_END_TEST; + +static void +add_media_line (struct test_webrtc *t, GstElement * element, + GstWebRTCSessionDescription * desc, gpointer user_data) +{ + GstSDPMedia *media = NULL; + const GstSDPMedia *existing_media; + GstSDPResult res; + + existing_media = gst_sdp_message_get_media (desc->sdp, 0); + + res = gst_sdp_media_copy (existing_media, &media); + fail_unless (res == GST_SDP_OK); + res = gst_sdp_message_add_media (desc->sdp, media); + fail_unless (res == GST_SDP_OK); + gst_sdp_media_free (media); +} + +static void +on_answer_set_rejected (struct test_webrtc *t, GstElement * element, + GstPromise * promise, gpointer user_data) +{ + const GstStructure *s; + GError *error = NULL; + GError *compare_error = user_data; + + s = gst_promise_get_reply (promise); + fail_unless (s != NULL); + gst_structure_get (s, "error", G_TYPE_ERROR, &error, NULL); + fail_unless (g_error_matches (error, compare_error->domain, + compare_error->code)); + fail_unless_equals_string (compare_error->message, error->message); + g_clear_error (&error); +} + +GST_START_TEST (test_invalid_add_media_in_answer) +{ + struct test_webrtc *t = create_audio_test (); + VAL_SDP_INIT (no_duplicate_payloads, on_sdp_media_no_duplicate_payloads, + NULL, NULL); + guint media_format_count[] = { 1 }; + VAL_SDP_INIT (media_formats, on_sdp_media_count_formats, + media_format_count, &no_duplicate_payloads); + VAL_SDP_INIT (count, _count_num_sdp_media, GUINT_TO_POINTER (1), + &media_formats); + const gchar *expected_offer_setup[] = { "actpass", }; + VAL_SDP_INIT (offer_setup, on_sdp_media_setup, expected_offer_setup, &count); + const gchar *expected_offer_direction[] = { "sendrecv", }; + VAL_SDP_INIT (offer, on_sdp_media_direction, expected_offer_direction, + &offer_setup); + VAL_SDP_INIT (answer, add_media_line, NULL, NULL); + GError answer_set_error = { GST_WEBRTC_ERROR, + GST_WEBRTC_ERROR_SDP_SYNTAX_ERROR, + (gchar *) "Answer doesn't have the same number of m-lines as the offer." + }; + + /* Ensure that if the answer has more m-lines than the offer, it gets + * rejected. + */ + + t->on_answer_set = on_answer_set_rejected; + t->answer_set_data = &answer_set_error; + + test_validate_sdp (t, &offer, &answer); + test_webrtc_free (t); +} + +GST_END_TEST; + static Suite * webrtcbin_suite (void) { @@ -5371,6 +5440,7 @@ webrtcbin_suite (void) tcase_add_test (tc, test_simulcast); tcase_add_test (tc, test_simulcast_fec_rtx); tcase_add_test (tc, test_bundle_multiple_media_rtx_payload_mapping); + tcase_add_test (tc, test_invalid_add_media_in_answer); if (sctpenc && sctpdec) { tcase_add_test (tc, test_data_channel_create); tcase_add_test (tc, test_data_channel_remote_notify);