diff --git a/pc/sdp_offer_answer.cc b/pc/sdp_offer_answer.cc index 06303de441..25383e83e5 100644 --- a/pc/sdp_offer_answer.cc +++ b/pc/sdp_offer_answer.cc @@ -120,6 +120,12 @@ const char kDefaultStreamId[] = "default"; 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 @@ RTCError ValidateSsrcGroups(const cricket::SessionDescription& description) { 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 @@ RTCError SdpOfferAnswerHandler::ValidateSessionDescription( 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. diff --git a/pc/sdp_offer_answer_unittest.cc b/pc/sdp_offer_answer_unittest.cc index 14e9557ca6..bbf7e2b092 100644 --- a/pc/sdp_offer_answer_unittest.cc +++ b/pc/sdp_offer_answer_unittest.cc @@ -993,4 +993,26 @@ TEST_F(SdpOfferAnswerTest, RejectsAnswerWithInvalidTransport) { 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