From bc88c6ba98806b629d64c832f8cebeafd75e93e4 Mon Sep 17 00:00:00 2001 From: deadbeef Date: Wed, 2 Aug 2017 11:26:34 -0700 Subject: [PATCH] Reject negative values for "b=AS". It doesn't make sense to have a negative RTP session bandwidth; RFC3550 doesn't define any meaning for this. So just treat it as invalid SDP. BUG=chromium:675361 Review-Url: https://codereview.webrtc.org/2989243002 Cr-Commit-Position: refs/heads/master@{#19221} --- webrtc/pc/webrtcsdp.cc | 3 +++ webrtc/pc/webrtcsdp_unittest.cc | 15 +++++++++++++++ 2 files changed, 18 insertions(+) diff --git a/webrtc/pc/webrtcsdp.cc b/webrtc/pc/webrtcsdp.cc index d9ed4255aa..bc2ec89645 100644 --- a/webrtc/pc/webrtcsdp.cc +++ b/webrtc/pc/webrtcsdp.cc @@ -2719,6 +2719,9 @@ bool ParseContent(const std::string& message, if (!GetValueFromString(line, bandwidth, &b, error)) { return false; } + if (b < 0) { + return ParseFailed(line, "b=AS value can't be negative.", error); + } // We should never use more than the default bandwidth for RTP-based // data channels. Don't allow SDP to set the bandwidth, because // that would give JS the opportunity to "break the Internet". diff --git a/webrtc/pc/webrtcsdp_unittest.cc b/webrtc/pc/webrtcsdp_unittest.cc index 20b2b4e15d..24b196f50e 100644 --- a/webrtc/pc/webrtcsdp_unittest.cc +++ b/webrtc/pc/webrtcsdp_unittest.cc @@ -3343,6 +3343,21 @@ TEST_F(WebRtcSdpTest, DeserializeLargeBandwidthLimit) { ExpectParseFailure(std::string(kSdpWithLargeBandwidth), "foo=fail"); } +// Similar to the above, except that negative values are illegal, not just +// error-prone as large values are. +// https://bugs.chromium.org/p/chromium/issues/detail?id=675361 +TEST_F(WebRtcSdpTest, DeserializingNegativeBandwidthLimitFails) { + static const char kSdpWithNegativeBandwidth[] = + "v=0\r\n" + "o=- 18446744069414584320 18446462598732840960 IN IP4 127.0.0.1\r\n" + "s=-\r\n" + "t=0 0\r\n" + "m=video 3457 RTP/SAVPF 120\r\n" + "b=AS:-1000\r\n"; + + ExpectParseFailure(std::string(kSdpWithNegativeBandwidth), "b=AS:-1000"); +} + // Test that "ufrag"/"pwd" in the candidate line itself are ignored, and only // the "a=ice-ufrag"/"a=ice-pwd" attributes are used. // Regression test for: