Fix integer underflow in ParseHandshakeHeader in websocket_handshake_handler.cc

BUG=48330
TEST=WebSocketHandshakeResponseHandlerTest.BadResponse and BadResponse2 don't crash and passes

Review URL: http://codereview.chromium.org/2870047

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@51652 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 44 insertions, 9 deletions

diff --git a/net/websockets/websocket_handshake_handler.cc b/net/websockets/websocket_handshake_handler.cc
index 647241c..5278151 100644
--- a/net/websockets/websocket_handshake_handler.cc
+++ b/net/websockets/websocket_handshake_handler.cc
@@ -27,9 +27,15 @@ void ParseHandshakeHeader(
   }
   // |status_line| includes \r\n.
   *status_line = std::string(handshake_message, i + 2);
-  // |handshake_message| includes tailing \r\n\r\n.
-  // |headers| doesn't include 2nd \r\n.
-  *headers = std::string(handshake_message + i + 2, len - (i + 2) - 2);
+
+  int header_len = len - (i + 2) - 2;
+  if (header_len > 0) {
+    // |handshake_message| includes tailing \r\n\r\n.
+    // |headers| doesn't include 2nd \r\n.
+    *headers = std::string(handshake_message + i + 2, header_len);
+  } else {
+    *headers = "";
+  }
 }
 
 void FetchHeaders(const std::string& headers,
@@ -300,6 +306,10 @@ size_t WebSocketHandshakeResponseHandler::ParseRawResponse(
                        original_header_length_,
                        &status_line_,
                        &headers_);
+  int header_size = status_line_.size() + headers_.size();
+  DCHECK_GE(original_header_length_, header_size);
+  header_separator_ = std::string(original_.data() + header_size,
+                                  original_header_length_ - header_size);
   key_ = std::string(original_.data() + original_header_length_,
                      kResponseKeySize);
 
@@ -405,10 +415,10 @@ void WebSocketHandshakeResponseHandler::RemoveHeaders(
 std::string WebSocketHandshakeResponseHandler::GetResponse() {
   DCHECK(HasResponse());
   DCHECK(status_line_.size() > 0);
-  DCHECK(headers_.size() > 0);
+  // headers_ might be empty for wrong response from server.
   DCHECK_EQ(kResponseKeySize, key_.size());
 
-  return status_line_ + headers_ + "\r\n" + key_;
+  return status_line_ + headers_ + header_separator_ + key_;
 }
 
 }  // namespace net
diff --git a/net/websockets/websocket_handshake_handler.h b/net/websockets/websocket_handshake_handler.h
index 3c11e00..cf5700b 100644
--- a/net/websockets/websocket_handshake_handler.h
+++ b/net/websockets/websocket_handshake_handler.h
@@ -105,6 +105,7 @@ class WebSocketHandshakeResponseHandler {
   int original_header_length_;
   std::string status_line_;
   std::string headers_;
+  std::string header_separator_;
   std::string key_;
 
   DISALLOW_COPY_AND_ASSIGN(WebSocketHandshakeResponseHandler);
diff --git a/net/websockets/websocket_handshake_handler_unittest.cc b/net/websockets/websocket_handshake_handler_unittest.cc
index 4204d6a..65e0712 100644
--- a/net/websockets/websocket_handshake_handler_unittest.cc
+++ b/net/websockets/websocket_handshake_handler_unittest.cc
@@ -106,8 +106,10 @@ TEST(WebSocketHandshakeResponseHandlerTest, SimpleResponse) {
       "\r\n"
       "8jKS'y:G*Co,Wxa-";
 
-  EXPECT_TRUE(handler.ParseRawResponse(kHandshakeResponseMessage,
-                                       strlen(kHandshakeResponseMessage)));
+  EXPECT_EQ(strlen(kHandshakeResponseMessage),
+            handler.ParseRawResponse(kHandshakeResponseMessage,
+                                     strlen(kHandshakeResponseMessage)));
+  EXPECT_TRUE(handler.HasResponse());
 
   handler.RemoveHeaders(kCookieHeaders, arraysize(kCookieHeaders));
 
@@ -129,8 +131,10 @@ TEST(WebSocketHandshakeResponseHandlerTest, ReplaceResponseCookies) {
       "\r\n"
       "8jKS'y:G*Co,Wxa-";
 
-  EXPECT_TRUE(handler.ParseRawResponse(kHandshakeResponseMessage,
-                                       strlen(kHandshakeResponseMessage)));
+  EXPECT_EQ(strlen(kHandshakeResponseMessage),
+            handler.ParseRawResponse(kHandshakeResponseMessage,
+                                     strlen(kHandshakeResponseMessage)));
+  EXPECT_TRUE(handler.HasResponse());
   std::vector<std::string> cookies;
   handler.GetHeaders(kSetCookieHeaders, arraysize(kSetCookieHeaders), &cookies);
   ASSERT_EQ(2U, cookies.size());
@@ -151,6 +155,26 @@ TEST(WebSocketHandshakeResponseHandlerTest, ReplaceResponseCookies) {
   EXPECT_EQ(kHandshakeResponseExpectedMessage, handler.GetResponse());
 }
 
+TEST(WebSocketHandshakeResponseHandlerTest, BadResponse) {
+  WebSocketHandshakeResponseHandler handler;
+
+  static const char* kBadMessage = "\n\n\r\net-Location: w";
+  EXPECT_EQ(strlen(kBadMessage),
+            handler.ParseRawResponse(kBadMessage, strlen(kBadMessage)));
+  EXPECT_TRUE(handler.HasResponse());
+  EXPECT_EQ(kBadMessage, handler.GetResponse());
+}
+
+TEST(WebSocketHandshakeResponseHandlerTest, BadResponse2) {
+  WebSocketHandshakeResponseHandler handler;
+
+  static const char* kBadMessage = "\n\r\n\r\net-Location: w";
+  EXPECT_EQ(strlen(kBadMessage),
+            handler.ParseRawResponse(kBadMessage, strlen(kBadMessage)));
+  EXPECT_TRUE(handler.HasResponse());
+  EXPECT_EQ(kBadMessage, handler.GetResponse());
+}
+
 TEST(WebSocketHandshakeHandlerTest, HttpRequestResponse) {
   WebSocketHandshakeRequestHandler request_handler;