From 1be66414df0bd836e958c9636f263b02c7d3a2a6 Mon Sep 17 00:00:00 2001
From: Philipp Hancke <philipp.hancke@googlemail.com>
Date: Mon, 18 May 2026 11:12:58 +0200
Subject: [PATCH] sdp: fix sdp munging detection edge case

Bug: chromium:512953564
Change-Id: Ia56457b98b8a71b7c641764eef5b1b82498223d6
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/472600
Reviewed-by: Markus Handell <handellm@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Philipp Hancke <philipp.hancke@googlemail.com>
Cr-Commit-Position: refs/heads/main@{#47737}
---

diff --git a/pc/sdp_munging_detector_unittest.cc b/pc/sdp_munging_detector_unittest.cc
index f266c10..813a6ad 100644
--- a/pc/sdp_munging_detector_unittest.cc
+++ b/pc/sdp_munging_detector_unittest.cc
@@ -1549,6 +1549,38 @@
       ElementsAre(Pair(SdpMungingType::kDataChannelSctpInit, 1)));
 }
 
+TEST_F(SdpMungingTest, SctpInitAndIceUfrag) {
+  auto pc = CreatePeerConnection("WebRTC-Sctp-Snap/Enabled/");
+  EXPECT_TRUE(pc->CreateDataChannel("dc"));
+  auto offer = pc->CreateOffer();
+  ASSERT_THAT(offer, Not(IsNull()));
+
+  auto& transport_infos = offer->description()->transport_infos();
+  ASSERT_EQ(transport_infos.size(), 1u);
+  transport_infos[0].description.ice_ufrag =
+      "amungediceufragthisshouldberejected";  // But is not right now.
+
+  auto& contents = offer->description()->contents();
+  ASSERT_THAT(contents, SizeIs(1));
+  auto* media_description = contents[0].media_description();
+  ASSERT_THAT(media_description, Not(IsNull()));
+  auto* sctp_description = media_description->as_sctp();
+  ASSERT_THAT(sctp_description, Not(IsNull()));
+  EXPECT_TRUE(sctp_description->sctp_init());
+
+  std::vector<uint8_t> test_value = {
+      0x01, 0x00, 0x00, 0x1e, 0xde, 0xad, 0xbe, 0xef, 0x00, 0x50,
+      0x00, 0x00, 0xff, 0xff, 0xff, 0xff, 0xde, 0xad, 0xbe, 0xef,
+      0xc0, 0x00, 0x00, 0x04, 0x80, 0x08, 0x00, 0x06, 0x82, 0xc0};
+  sctp_description->set_sctp_init(test_value);
+
+  RTCError error;
+  EXPECT_FALSE(pc->SetLocalDescription(std::move(offer), &error));
+  EXPECT_THAT(
+      metrics::Samples("WebRTC.PeerConnection.SdpMunging.Offer.Initial"),
+      ElementsAre(Pair(SdpMungingType::kDataChannelSctpInit, 1)));
+}
+
 TEST_F(SdpMungingTest, MaxMessageSize) {
   auto pc = CreatePeerConnection();
   EXPECT_TRUE(pc->CreateDataChannel("dc"));
diff --git a/pc/sdp_offer_answer.cc b/pc/sdp_offer_answer.cc
index 2af7466..a2bb704 100644
--- a/pc/sdp_offer_answer.cc
+++ b/pc/sdp_offer_answer.cc
@@ -2740,22 +2740,26 @@
       DetermineSdpMungingType(desc.get(), last_created_desc);
 
   if (!disable_sdp_munging_checks_) {
-    bool reject_error = false;
-    if (HasUfragSdpMunging(desc.get(), last_created_desc)) {
+    bool reject_with_error =
+        !IsSdpMungingAllowed(sdp_munging_type, pc_->trials());
+    if (!reject_with_error &&
+        HasUfragSdpMunging(desc.get(), last_created_desc)) {
+      // ice-ufrag munging may still trigger rejection for other reasons
+      // guarded by this variable.
       has_sdp_munged_ufrag_ = true;
+
       if (pc_->trials().IsEnabled("WebRTC-NoSdpMangleUfrag")) {
         RTC_LOG(LS_ERROR) << "Rejecting SDP because of ufrag modification";
-        reject_error = true;
+        reject_with_error = true;
       }
-    } else {
-      reject_error = !IsSdpMungingAllowed(sdp_munging_type, pc_->trials());
     }
-    SdpMungingOutcome outcome = reject_error ? SdpMungingOutcome::kRejected
-                                             : SdpMungingOutcome::kAccepted;
+    SdpMungingOutcome outcome = reject_with_error
+                                    ? SdpMungingOutcome::kRejected
+                                    : SdpMungingOutcome::kAccepted;
     RTC_HISTOGRAM_ENUMERATION("WebRTC.PeerConnection.SdpMunging.Outcome",
                               static_cast<int>(outcome),
                               static_cast<int>(SdpMungingOutcome::kMaxValue));
-    if (reject_error) {
+    if (reject_with_error) {
       observer->OnSetLocalDescriptionComplete(
           RTCError(RTCErrorType::INVALID_MODIFICATION,
                    "SDP is modified in a non-acceptable way"));

diff --git a/pc/sdp_munging_detector_unittest.cc b/pc/sdp_munging_detector_unittest.cc
index f266c10..813a6ad 100644
--- a/pc/sdp_munging_detector_unittest.cc
+++ b/pc/sdp_munging_detector_unittest.cc
@@ -1549,6 +1549,38 @@
       ElementsAre(Pair(SdpMungingType::kDataChannelSctpInit, 1)));
 }
 
+TEST_F(SdpMungingTest, SctpInitAndIceUfrag) {
+  auto pc = CreatePeerConnection("WebRTC-Sctp-Snap/Enabled/");
+  EXPECT_TRUE(pc->CreateDataChannel("dc"));
+  auto offer = pc->CreateOffer();
+  ASSERT_THAT(offer, Not(IsNull()));
+
+  auto& transport_infos = offer->description()->transport_infos();
+  ASSERT_EQ(transport_infos.size(), 1u);
+  transport_infos[0].description.ice_ufrag =
+      "amungediceufragthisshouldberejected";  // But is not right now.
+
+  auto& contents = offer->description()->contents();
+  ASSERT_THAT(contents, SizeIs(1));
+  auto* media_description = contents[0].media_description();
+  ASSERT_THAT(media_description, Not(IsNull()));
+  auto* sctp_description = media_description->as_sctp();
+  ASSERT_THAT(sctp_description, Not(IsNull()));
+  EXPECT_TRUE(sctp_description->sctp_init());
+
+  std::vector<uint8_t> test_value = {
+      0x01, 0x00, 0x00, 0x1e, 0xde, 0xad, 0xbe, 0xef, 0x00, 0x50,
+      0x00, 0x00, 0xff, 0xff, 0xff, 0xff, 0xde, 0xad, 0xbe, 0xef,
+      0xc0, 0x00, 0x00, 0x04, 0x80, 0x08, 0x00, 0x06, 0x82, 0xc0};
+  sctp_description->set_sctp_init(test_value);
+
+  RTCError error;
+  EXPECT_FALSE(pc->SetLocalDescription(std::move(offer), &error));
+  EXPECT_THAT(
+      metrics::Samples("WebRTC.PeerConnection.SdpMunging.Offer.Initial"),
+      ElementsAre(Pair(SdpMungingType::kDataChannelSctpInit, 1)));
+}
+
 TEST_F(SdpMungingTest, MaxMessageSize) {
   auto pc = CreatePeerConnection();
   EXPECT_TRUE(pc->CreateDataChannel("dc"));