From bd44bb01843d36b3390fd24561531c5d98031be0 Mon Sep 17 00:00:00 2001
From: hnsl <hnsl@webrtc.org>
Date: Mon, 12 Dec 2016 03:14:30 -0800
Subject: [PATCH] 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          | 23 ++++++++++++-----------
 webrtc/api/peerconnection_unittest.cc |  3 +++
 2 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/webrtc/api/peerconnection.cc b/webrtc/api/peerconnection.cc
index 49f90307d0..692ca2e253 100644
--- a/webrtc/api/peerconnection.cc
+++ b/webrtc/api/peerconnection.cc
@@ -231,21 +231,22 @@ bool ParseIceServerUrl(const PeerConnectionInterface::IceServer& server,
   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;
     }
   }
 
diff --git a/webrtc/api/peerconnection_unittest.cc b/webrtc/api/peerconnection_unittest.cc
index 301fed55db..48accb5bd9 100644
--- a/webrtc/api/peerconnection_unittest.cc
+++ b/webrtc/api/peerconnection_unittest.cc
@@ -2749,6 +2749,9 @@ TEST_F(IceServerParsingTest, ParseTransport) {
   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.