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: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/2526>

diff --git a/subprojects/gst-plugins-bad/ext/webrtc/gstwebrtcbin.c b/subprojects/gst-plugins-bad/ext/webrtc/gstwebrtcbin.c
index df7b86c..6278bfb 100644 (file)
--- 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 16abe97..dfce037 100644 (file)
--- 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);