From 90f1e1e0d7f7b88eb5bd849bbe6a75c6778c3bc8 Mon Sep 17 00:00:00 2001 From: deadbeef Date: Fri, 10 Feb 2017 12:35:05 -0800 Subject: [PATCH] Fixing SDP parsing crash due to invalid port numbers. BUG=chromium:677029 Review-Url: https://codereview.webrtc.org/2675273003 Cr-Commit-Position: refs/heads/master@{#16541} --- webrtc/base/socketaddress.h | 4 ++++ webrtc/pc/webrtcsdp.cc | 12 ++++++++++++ webrtc/pc/webrtcsdp_unittest.cc | 22 ++++++++++++++++++++++ 3 files changed, 38 insertions(+) diff --git a/webrtc/base/socketaddress.h b/webrtc/base/socketaddress.h index 175d7a9d12..bcff3900b8 100644 --- a/webrtc/base/socketaddress.h +++ b/webrtc/base/socketaddress.h @@ -32,13 +32,16 @@ class SocketAddress { // Creates the address with the given host and port. Host may be a // literal IP string or a hostname to be resolved later. + // DCHECKs that port is in valid range (0 to 2^16-1). SocketAddress(const std::string& hostname, int port); // Creates the address with the given IP and port. // IP is given as an integer in host byte order. V4 only, to be deprecated. + // DCHECKs that port is in valid range (0 to 2^16-1). SocketAddress(uint32_t ip_as_host_order_integer, int port); // Creates the address with the given IP and port. + // DCHECKs that port is in valid range (0 to 2^16-1). SocketAddress(const IPAddress& ip, int port); // Creates a copy of the given address. @@ -77,6 +80,7 @@ class SocketAddress { void SetResolvedIP(const IPAddress& ip); // Changes the port of this address to the given one. + // DCHECKs that port is in valid range (0 to 2^16-1). void SetPort(int port); // Returns the hostname. diff --git a/webrtc/pc/webrtcsdp.cc b/webrtc/pc/webrtcsdp.cc index d5110fc854..93b77274bf 100644 --- a/webrtc/pc/webrtcsdp.cc +++ b/webrtc/pc/webrtcsdp.cc @@ -79,6 +79,8 @@ namespace cricket { class SessionDescription; } +// TODO(deadbeef): Switch to using anonymous namespace rather than declaring +// everything "static". namespace webrtc { // Line type @@ -790,6 +792,10 @@ static void GetCandidatesByMindex(const SessionDescriptionInterface& desci, } } +static bool IsValidPort(int port) { + return port >= 0 && port <= 65535; +} + std::string SdpSerialize(const JsepSessionDescription& jdesc, bool unified_plan_sdp) { const cricket::SessionDescription* desc = jdesc.description(); @@ -1026,6 +1032,9 @@ bool ParseCandidate(const std::string& message, Candidate* candidate, if (!GetValueFromString(first_line, fields[5], &port, error)) { return false; } + if (!IsValidPort(port)) { + return ParseFailed(first_line, "Invalid port number.", error); + } SocketAddress address(connection_address, port); cricket::ProtocolType protocol; @@ -1072,6 +1081,9 @@ bool ParseCandidate(const std::string& message, Candidate* candidate, first_line, fields[++current_position], &port, error)) { return false; } + if (!IsValidPort(port)) { + return ParseFailed(first_line, "Invalid port number.", error); + } related_address.SetPort(port); ++current_position; } diff --git a/webrtc/pc/webrtcsdp_unittest.cc b/webrtc/pc/webrtcsdp_unittest.cc index 81a5aaf4fe..75116e8e33 100644 --- a/webrtc/pc/webrtcsdp_unittest.cc +++ b/webrtc/pc/webrtcsdp_unittest.cc @@ -3424,3 +3424,25 @@ TEST_F(WebRtcSdpTest, IceCredentialsInCandidateStringIgnored) { EXPECT_EQ("ufrag_voice", c.username()); EXPECT_EQ("pwd_voice", c.password()); } + +// Test that SDP with an invalid port number in "a=candidate" lines is +// rejected, without crashing. +// Regression test for: +// https://bugs.chromium.org/p/chromium/issues/detail?id=677029 +TEST_F(WebRtcSdpTest, DeserializeInvalidPortInCandidateAttribute) { + static const char kSdpWithInvalidCandidatePort[] = + "v=0\r\n" + "o=- 18446744069414584320 18446462598732840960 IN IP4 127.0.0.1\r\n" + "s=-\r\n" + "t=0 0\r\n" + "m=audio 9 RTP/SAVPF 111\r\n" + "c=IN IP4 0.0.0.0\r\n" + "a=rtcp:9 IN IP4 0.0.0.0\r\n" + "a=ice-ufrag:ufrag_voice\r\na=ice-pwd:pwd_voice\r\n" + "a=rtpmap:111 opus/48000/2\r\n" + "a=candidate:a0+B/1 1 udp 2130706432 192.168.1.5 12345678 typ host " + "generation 2 raddr 192.168.1.1 rport 87654321\r\n"; + + JsepSessionDescription jdesc_output(kDummyString); + EXPECT_FALSE(SdpDeserialize(kSdpWithInvalidCandidatePort, &jdesc_output)); +}