commit bd44bb01843d36b3390fd24561531c5d98031be0
author hnsl <hnsl@webrtc.org> Mon Dec 12 11:14:30 2016
committer Commit bot <commit-bot@chromium.org> Mon Dec 12 11:14:34 2016
tree 9b09a6a7bf8b853fd6b8a0259c6b5f4288dcfadc
parent b010b8fe6819e4a987c975876f455a3db3abad85

Fix out of bound reads in ParseIceServerUrl() for various input.

BUG=webrtc:6835

Review-Url: https://codereview.webrtc.org/2556783002
Cr-Commit-Position: refs/heads/master@{#15544}

webrtc/api/peerconnection.cc

diff --git a/webrtc/api/peerconnection.cc b/webrtc/api/peerconnection.cc
index 49f9030..692ca2e 100644
--- a/webrtc/api/peerconnection.cc
+++ b/webrtc/api/peerconnection.cc
@@ -231,21 +231,22 @@
   std::vector<std::string> tokens;
   cricket::ProtocolType turn_transport_type = cricket::PROTO_UDP;
   RTC_DCHECK(!url.empty());
-  rtc::tokenize(url, '?', &tokens);
+  rtc::tokenize_with_empty_tokens(url, '?', &tokens);
   std::string uri_without_transport = tokens[0];
   // Let's look into transport= param, if it exists.
   if (tokens.size() == kTurnTransportTokensNum) {  // ?transport= is present.
     std::string uri_transport_param = tokens[1];
-    rtc::tokenize(uri_transport_param, '=', &tokens);
-    if (tokens[0] == kTransport) {
-      // As per above grammar transport param will be consist of lower case
-      // letters.
-      if (!cricket::StringToProto(tokens[1].c_str(), &turn_transport_type) ||
-          (turn_transport_type != cricket::PROTO_UDP &&
-           turn_transport_type != cricket::PROTO_TCP)) {
-        LOG(LS_WARNING) << "Transport param should always be udp or tcp.";
-        return false;
-      }
+    rtc::tokenize_with_empty_tokens(uri_transport_param, '=', &tokens);
+    if (tokens[0] != kTransport) {
+      LOG(LS_WARNING) << "Invalid transport parameter key.";
+      return false;
+    }
+    if (tokens.size() < 2 ||
+        !cricket::StringToProto(tokens[1].c_str(), &turn_transport_type) ||
+        (turn_transport_type != cricket::PROTO_UDP &&
+         turn_transport_type != cricket::PROTO_TCP)) {
+      LOG(LS_WARNING) << "Transport param should always be udp or tcp.";
+      return false;
     }
   }
 

webrtc/api/peerconnection_unittest.cc

diff --git a/webrtc/api/peerconnection_unittest.cc b/webrtc/api/peerconnection_unittest.cc
index 301fed5..48accb5 100644
--- a/webrtc/api/peerconnection_unittest.cc
+++ b/webrtc/api/peerconnection_unittest.cc
@@ -2749,6 +2749,9 @@
   turn_servers_.clear();
 
   EXPECT_FALSE(ParseUrl("turn:hostname?transport=invalid"));
+  EXPECT_FALSE(ParseUrl("turn:hostname?transport="));
+  EXPECT_FALSE(ParseUrl("turn:hostname?="));
+  EXPECT_FALSE(ParseUrl("?"));
 }
 
 // Test parsing ICE username contained in URL.