Fix out-of-bounds write in SetupCodec due to excessive RIDs In GetSendEncodingsFromRemoteDescription, limit the number of processed simulcast layers to kMaxSimulcastStreams. Added bounds checking in SetupCodec to avoid writing past the end of the simulcastStream array. Bug: chromium:486536241 Change-Id: Ie5bbe2bd3362cb4b319ec5be67c6288da8ad953a Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/458720 Reviewed-by: Harald Alvestrand <hta@webrtc.org> Reviewed-by: Philipp Hancke <philipp.hancke@googlemail.com> Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org> Cr-Commit-Position: refs/heads/main@{#47230}
diff --git a/modules/video_coding/video_codec_initializer.cc b/modules/video_coding/video_codec_initializer.cc index 6d329a1..7b3579d 100644 --- a/modules/video_coding/video_codec_initializer.cc +++ b/modules/video_coding/video_codec_initializer.cc
@@ -80,8 +80,8 @@ config.legacy_conference_mode; video_codec.SetFrameDropEnabled(config.frame_drop_enabled); - video_codec.numberOfSimulcastStreams = - static_cast<unsigned char>(streams.size()); + video_codec.numberOfSimulcastStreams = static_cast<unsigned char>( + std::min(streams.size(), static_cast<size_t>(kMaxSimulcastStreams))); video_codec.minBitrate = streams[0].min_bitrate_bps / 1000; bool codec_active = false; // Active configuration might not be fully copied to `streams` for SVC yet. @@ -104,7 +104,9 @@ int max_framerate = 0; std::optional<ScalabilityMode> scalability_mode = streams[0].scalability_mode; - for (size_t i = 0; i < streams.size(); ++i) { + const size_t num_streams = + std::min(streams.size(), static_cast<size_t>(kMaxSimulcastStreams)); + for (size_t i = 0; i < num_streams; ++i) { SimulcastStream* sim_stream = &video_codec.simulcastStream[i]; RTC_DCHECK_GT(streams[i].width, 0); RTC_DCHECK_GT(streams[i].height, 0);
diff --git a/pc/sdp_offer_answer.cc b/pc/sdp_offer_answer.cc index 3f5c83f..cb2ed5c 100644 --- a/pc/sdp_offer_answer.cc +++ b/pc/sdp_offer_answer.cc
@@ -706,6 +706,12 @@ // This is a remote description, the parameters we are after should appear // as receive streams. for (const auto& alternatives : simulcast.receive_layers()) { + if (result.size() >= kMaxSimulcastStreams) { + RTC_LOG(LS_WARNING) + << "Excessive simulcast layers in remote description. Clamping to " + << kMaxSimulcastStreams; + break; + } RTC_DCHECK(!alternatives.empty()); // There is currently no way to specify or choose from alternatives. // We will always use the first alternative, which is the most preferred.
diff --git a/pc/sdp_offer_answer_unittest.cc b/pc/sdp_offer_answer_unittest.cc index 14d81a1..c216c89 100644 --- a/pc/sdp_offer_answer_unittest.cc +++ b/pc/sdp_offer_answer_unittest.cc
@@ -823,6 +823,46 @@ EXPECT_TRUE(pc->SetLocalDescription(std::move(answer))); } +TEST_F(SdpOfferAnswerTest, SimulcastOfferWithExcessiveRidsClamped) { + auto pc = CreatePeerConnection(); + + std::string sdp = + "v=0\r\n" + "o=- 4131505339648218884 3 IN IP4 127.0.0.1\r\n" + "s=-\r\n" + "t=0 0\r\n" + "a=ice-ufrag:zGWFZ+fVXDeN6UoI/136\r\n" + "a=ice-pwd:9AUNgUqRNI5LSIrC1qFD2iTR\r\n" + "a=fingerprint:sha-256 " + "AD:52:52:E0:B1:37:34:21:0E:15:8E:B7:56:56:7B:B4:39:0E:6D:1C:F5:84:A7:EE:" + "B5:27:3E:30:B1:7D:69:42\r\n" + "a=setup:passive\r\n" + "m=video 9 UDP/TLS/RTP/SAVPF 96\r\n" + "c=IN IP4 0.0.0.0\r\n" + "a=rtcp:9 IN IP4 0.0.0.0\r\n" + "a=mid:0\r\n" + "a=extmap:9 urn:ietf:params:rtp-hdrext:sdes:mid\r\n" + "a=extmap:10 urn:ietf:params:rtp-hdrext:sdes:rtp-stream-id\r\n" + "a=recvonly\r\n" + "a=rtcp-mux\r\n" + "a=rtcp-rsize\r\n" + "a=rtpmap:96 VP8/90000\r\n"; + + for (int i = 1; i <= 9; ++i) { + sdp += "a=rid:" + std::to_string(i) + " recv\r\n"; + } + sdp += "a=simulcast:recv 1;2;3;4;5;6;7;8;9\r\n"; + + std::unique_ptr<SessionDescriptionInterface> offer = + CreateSessionDescription(SdpType::kOffer, sdp); + EXPECT_TRUE(pc->SetRemoteDescription(std::move(offer))); + + auto transceiver = pc->pc()->GetTransceivers()[0]; + // Verify that the number of send encodings is clamped to 3 + // (kMaxSimulcastStreams) + EXPECT_THAT(transceiver->sender()->GetParameters().encodings, SizeIs(3)); +} + TEST_F(SdpOfferAnswerTest, ExpectAllSsrcsSpecifiedInSsrcGroupFid) { auto pc = CreatePeerConnection(); std::string sdp =