pc: fix use-after-free in AllocateSctpSids

AllocateSctpSids iterates over newly assigned channels using raw
pointers and calls OnTransportReady on each one. OnTransportReady
can synchronously fail (e.g. if sending the DCEP OPEN message
fails), which may cause the channel to close and be deleted,
leaving the controller with a dangling pointer.

This change ensures the channels are kept alive during the loop by
using scoped_refptr for the temporary collection. This fixes the
UAF for any failure mode that prevents synchronous sending of the
initialization message.

Bug: chromium:503422316
Change-Id: I1b927386fb4ed4279036835a244320aba6acf875
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/465701
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Auto-Submit: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#47503}

pc/data_channel_controller.cc

diff --git a/pc/data_channel_controller.cc b/pc/data_channel_controller.cc
index 09b62cd..8b42607 100644
--- a/pc/data_channel_controller.cc
+++ b/pc/data_channel_controller.cc
@@ -475,7 +475,7 @@
   const bool ready_to_send =
       data_channel_transport_ && data_channel_transport_->IsReadyToSend();
 
-  std::vector<SctpDataChannel*> channels_to_start;
+  std::vector<scoped_refptr<SctpDataChannel>> channels_to_start;
   std::vector<scoped_refptr<SctpDataChannel>> channels_to_close;
   for (auto it = sctp_data_channels_n_.begin();
        it != sctp_data_channels_n_.end();) {
@@ -484,7 +484,7 @@
       if (sid.has_value()) {
         (*it)->SetSctpSid_n(*sid);
         AddSctpDataStream(*sid, (*it)->priority());
-        channels_to_start.push_back((*it).get());
+        channels_to_start.push_back(*it);
       } else {
         channels_to_close.push_back(std::move(*it));
         it = sctp_data_channels_n_.erase(it);
@@ -496,7 +496,7 @@
   // Since OnTransportReady can cause sending, and sending may fail and cause
   // channel to close, do this outside the loop.
   if (ready_to_send) {
-    for (auto* channel : channels_to_start) {
+    for (auto& channel : channels_to_start) {
       RTC_LOG(LS_INFO) << "AllocateSctpSids: Id assigned, ready to send.";
       channel->OnTransportReady();
     }

pc/data_channel_controller_unittest.cc

diff --git a/pc/data_channel_controller_unittest.cc b/pc/data_channel_controller_unittest.cc
index fa04960..9089df8 100644
--- a/pc/data_channel_controller_unittest.cc
+++ b/pc/data_channel_controller_unittest.cc
@@ -267,6 +267,39 @@
   EXPECT_THAT(ch2.value()->state(), Eq(DataChannelInterface::kClosed));
 }
 
+// This test reproduces the UAF reported in b/503422316.
+// It creates a data channel, drops the external reference, and then triggers
+// AllocateSctpSids. AllocateSctpSids calls OnTransportReady, which fails
+// synchronously, causing the channel to close and be deleted while
+// AllocateSctpSids (and SctpDataChannel::UpdateState) are still on the stack.
+TEST_F(DataChannelControllerTest, AllocateSctpSidsUafRepro) {
+  NiceMock<MockDataChannelTransport> transport;
+  // Reject all SendData with "message too large"
+  EXPECT_CALL(transport, SendData(_, _, _))
+      .WillRepeatedly(
+          Return(RTCError(RTCErrorType::INVALID_RANGE, "Message too large")));
+  bool ready_to_send = false;
+  EXPECT_CALL(transport, IsReadyToSend())
+      .WillRepeatedly(ReturnPointee(&ready_to_send));
+  EXPECT_CALL(transport, DtlsRole())
+      .WillOnce(Return(std::nullopt))
+      .WillRepeatedly(Return(SSL_CLIENT));
+  ON_CALL(transport, MaxChannels).WillByDefault(Return(100));
+
+  DataChannelControllerForTest dcc(pc_.get(), &transport);
+  auto ret = dcc.InternalCreateDataChannelWithProxy(
+      "ch1", InternalDataChannelInit(DataChannelInit()));
+  ASSERT_TRUE(ret.ok());
+
+  // Drop the reference.
+  ret.MoveValue();
+
+  ready_to_send = true;
+  pc_->network_thread()->BlockingCall([&] { dcc.OnTransportConnected(); });
+
+  run_loop_.Flush();
+}
+
 TEST_F(DataChannelControllerTest, BufferedAmountIncludesFromTransport) {
   NiceMock<MockDataChannelTransport> transport;
   EXPECT_CALL(transport, buffered_amount(0)).WillOnce(Return(4711));