commit b64615a1942333e182971df6c7d2e4b17793f266
author Philipp Hancke <phancke@microsoft.com> Thu Sep 14 14:23:31 2023
committer WebRTC LUCI CQ <webrtc-scoped@luci-project-accounts.iam.gserviceaccount.com> Fri Sep 15 09:18:52 2023
tree 1d2736953d94c9aea862c0955cdadd6471e68566
parent e0083d480420c2b7318dd20b933c0773d4149a4c

sdp: reject RTP payload types in the 64-95 range w/rtcp-mux

which is forbidden by
  https://tools.ietf.org/html/rfc5761#section-4

BUG=webrtc:12197

Change-Id: I6227f01e7dcbca3f5871a2e4a8cea3c4db0b16cf
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/319120
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Philipp Hancke <phancke@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#40752}

pc/sdp_offer_answer.cc

diff --git a/pc/sdp_offer_answer.cc b/pc/sdp_offer_answer.cc
index 06303de..25383e8 100644
--- a/pc/sdp_offer_answer.cc
+++ b/pc/sdp_offer_answer.cc
@@ -120,6 +120,12 @@
 static const char kDefaultAudioSenderId[] = "defaulta0";
 static const char kDefaultVideoSenderId[] = "defaultv0";
 
+// NOTE: Duplicated from pc/used_ids.h
+static const int kLastDynamicPayloadTypeLowerRange = 63;
+
+static const int kFirstDynamicPayloadTypeUpperRange = 96;
+static const int kLastDynamicPayloadTypeUpperRange = 127;
+
 void NoteAddIceCandidateResult(int result) {
   RTC_HISTOGRAM_ENUMERATION("WebRTC.PeerConnection.AddIceCandidate", result,
                             kAddIceCandidateMax);
@@ -562,6 +568,53 @@
   return RTCError::OK();
 }
 
+RTCError ValidatePayloadTypes(const cricket::SessionDescription& description) {
+  for (const ContentInfo& content : description.contents()) {
+    if (content.type != MediaProtocolType::kRtp) {
+      continue;
+    }
+    const auto media_description = content.media_description();
+    RTC_DCHECK(media_description);
+    if (content.rejected || !media_description ||
+        !media_description->has_codecs()) {
+      continue;
+    }
+    const auto type = media_description->type();
+    if (type == cricket::MEDIA_TYPE_AUDIO) {
+      RTC_DCHECK(media_description->as_audio());
+      for (const auto& codec : media_description->as_audio()->codecs()) {
+        if (codec.id < 0 || codec.id > kLastDynamicPayloadTypeUpperRange ||
+            (media_description->rtcp_mux() &&
+             (codec.id > kLastDynamicPayloadTypeLowerRange &&
+              codec.id < kFirstDynamicPayloadTypeUpperRange))) {
+          LOG_AND_RETURN_ERROR(
+              RTCErrorType::INVALID_PARAMETER,
+              "The media section with MID='" + content.mid() +
+                  "' used an invalid payload type " + rtc::ToString(codec.id) +
+                  " for codec '" + codec.name + ", rtcp-mux:" +
+                  (media_description->rtcp_mux() ? "enabled" : "disabled"));
+        }
+      }
+    } else if (type == cricket::MEDIA_TYPE_VIDEO) {
+      RTC_DCHECK(media_description->as_video());
+      for (const auto& codec : media_description->as_video()->codecs()) {
+        if (codec.id < 0 || codec.id > kLastDynamicPayloadTypeUpperRange ||
+            (media_description->rtcp_mux() &&
+             (codec.id > kLastDynamicPayloadTypeLowerRange &&
+              codec.id < kFirstDynamicPayloadTypeUpperRange))) {
+          LOG_AND_RETURN_ERROR(
+              RTCErrorType::INVALID_PARAMETER,
+              "The media section with MID='" + content.mid() +
+                  "' used an invalid payload type " + rtc::ToString(codec.id) +
+                  " for codec '" + codec.name + ", rtcp-mux:" +
+                  (media_description->rtcp_mux() ? "enabled" : "disabled"));
+        }
+      }
+    }
+  }
+  return RTCError::OK();
+}
+
 bool IsValidOfferToReceiveMedia(int value) {
   typedef PeerConnectionInterface::RTCOfferAnswerOptions Options;
   return (value >= Options::kUndefined) &&
@@ -3587,6 +3640,11 @@
                          kBundleWithoutRtcpMux);
   }
 
+  error = ValidatePayloadTypes(*sdesc->description());
+  if (!error.ok()) {
+    return error;
+  }
+
   // TODO(skvlad): When the local rtcp-mux policy is Require, reject any
   // m-lines that do not rtcp-mux enabled.
 

pc/sdp_offer_answer_unittest.cc

diff --git a/pc/sdp_offer_answer_unittest.cc b/pc/sdp_offer_answer_unittest.cc
index 14e9557..bbf7e2b 100644
--- a/pc/sdp_offer_answer_unittest.cc
+++ b/pc/sdp_offer_answer_unittest.cc
@@ -993,4 +993,26 @@
   EXPECT_EQ(error.type(), RTCErrorType::INVALID_PARAMETER);
 }
 
+TEST_F(SdpOfferAnswerTest, SdpMungingWithInvalidPayloadTypeIsRejected) {
+  auto pc = CreatePeerConnection();
+  pc->AddAudioTrack("audio_track", {});
+
+  auto offer = pc->CreateOffer();
+  ASSERT_EQ(offer->description()->contents().size(), 1u);
+  auto* audio =
+      offer->description()->contents()[0].media_description()->as_audio();
+  ASSERT_GT(audio->codecs().size(), 0u);
+  EXPECT_TRUE(audio->rtcp_mux());
+  auto codecs = audio->codecs();
+  for (int invalid_payload_type = 64; invalid_payload_type < 96;
+       invalid_payload_type++) {
+    codecs[0].id =
+        invalid_payload_type;  // The range [64-95] is disallowed with rtcp_mux.
+    audio->set_codecs(codecs);
+    // ASSERT to avoid getting into a bad state.
+    ASSERT_FALSE(pc->SetLocalDescription(offer->Clone()));
+    ASSERT_FALSE(pc->SetRemoteDescription(offer->Clone()));
+  }
+}
+
 }  // namespace webrtc