From fbbfc02698f5c286f993fb383e2bb59ad172056d Mon Sep 17 00:00:00 2001 From: Philipp Hancke Date: Fri, 31 Jul 2020 08:30:50 +0200 Subject: [PATCH] sdp: reject sdp with malformed b= lines BUG=webrtc:3782 Change-Id: I3d137b0b74565f7e85bbc6b453e73731a94c2b04 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/179360 Commit-Queue: Philipp Hancke Reviewed-by: Taylor Cr-Commit-Position: refs/heads/master@{#31838} --- pc/webrtc_sdp.cc | 22 +++++++++++----------- pc/webrtc_sdp_unittest.cc | 25 +++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 11 deletions(-) diff --git a/pc/webrtc_sdp.cc b/pc/webrtc_sdp.cc index aa98e480ef..7a2aeae3a4 100644 --- a/pc/webrtc_sdp.cc +++ b/pc/webrtc_sdp.cc @@ -2992,17 +2992,17 @@ bool ParseContent(const std::string& message, if (IsLineType(line, kLineTypeSessionBandwidth)) { std::string bandwidth; std::string bandwidth_type; - if (HasAttribute(line, kApplicationSpecificBandwidth)) { - if (!GetValue(line, kApplicationSpecificBandwidth, &bandwidth, error)) { - return false; - } - bandwidth_type = kApplicationSpecificBandwidth; - } else if (HasAttribute(line, kTransportSpecificBandwidth)) { - if (!GetValue(line, kTransportSpecificBandwidth, &bandwidth, error)) { - return false; - } - bandwidth_type = kTransportSpecificBandwidth; - } else { + if (!rtc::tokenize_first(line.substr(kLinePrefixLength), + kSdpDelimiterColonChar, &bandwidth_type, + &bandwidth)) { + return ParseFailed( + line, + "b= syntax error, does not match b=:.", + error); + } + if (!(bandwidth_type == kApplicationSpecificBandwidth || + bandwidth_type == kTransportSpecificBandwidth)) { + // Ignore unknown bandwidth types. continue; } int b = 0; diff --git a/pc/webrtc_sdp_unittest.cc b/pc/webrtc_sdp_unittest.cc index 2a4c36d61f..9f81129663 100644 --- a/pc/webrtc_sdp_unittest.cc +++ b/pc/webrtc_sdp_unittest.cc @@ -2645,6 +2645,24 @@ TEST_F(WebRtcSdpTest, DeserializeSessionDescriptionWithTiasBandwidth) { EXPECT_TRUE(CompareSessionDescription(jdesc_, jdesc_with_bandwidth)); } +TEST_F(WebRtcSdpTest, + DeserializeSessionDescriptionWithUnknownBandwidthModifier) { + JsepSessionDescription jdesc_with_bandwidth(kDummyType); + std::string sdp_with_bandwidth = kSdpFullString; + InjectAfter("a=mid:video_content_name\r\na=sendrecv\r\n", + "b=unknown:100000\r\n", &sdp_with_bandwidth); + InjectAfter("a=mid:audio_content_name\r\na=sendrecv\r\n", + "b=unknown:50000\r\n", &sdp_with_bandwidth); + EXPECT_TRUE(SdpDeserialize(sdp_with_bandwidth, &jdesc_with_bandwidth)); + VideoContentDescription* vcd = GetFirstVideoContentDescription(&desc_); + vcd->set_bandwidth(-1); + AudioContentDescription* acd = GetFirstAudioContentDescription(&desc_); + acd->set_bandwidth(-1); + ASSERT_TRUE(jdesc_.Initialize(desc_.Clone(), jdesc_.session_id(), + jdesc_.session_version())); + EXPECT_TRUE(CompareSessionDescription(jdesc_, jdesc_with_bandwidth)); +} + TEST_F(WebRtcSdpTest, DeserializeSessionDescriptionWithIceOptions) { JsepSessionDescription jdesc_with_ice_options(kDummyType); std::string sdp_with_ice_options = kSdpFullString; @@ -3369,6 +3387,13 @@ TEST_F(WebRtcSdpTest, DeserializeSdpWithInvalidAttributeValue) { // bandwidth ExpectParseFailureWithNewLines("a=mid:video_content_name\r\n", "b=AS:badvalue\r\n", "b=AS:badvalue"); + ExpectParseFailureWithNewLines("a=mid:video_content_name\r\n", "b=AS\r\n", + "b=AS"); + ExpectParseFailureWithNewLines("a=mid:video_content_name\r\n", "b=AS:\r\n", + "b=AS:"); + ExpectParseFailureWithNewLines("a=mid:video_content_name\r\n", + "b=AS:12:34\r\n", "b=AS:12:34"); + // rtcp-fb ExpectParseFailureWithNewLines("a=mid:video_content_name\r\n", "a=rtcp-fb:badvalue nack\r\n",