[extensions] Fix several broken assumptions in TCP sockets API

This CL addresses three independent issues:

  1. Undefined behavior when double-connecting a socket
     (hitting an unexpected DCHECK)

  2. Undefined behavior when reading from a disconnected socket.
     (hitting an unexpected DCHECK)

  3. Crash when reading from a socket with connection in progress.

BUG=582650
TEST=extensive fuzzing with extension attached to linked bug, no more crashes

Review URL: https://codereview.chromium.org/1767993002

Cr-Commit-Position: refs/heads/master@{#379498}

1 files changed, 15 insertions, 3 deletions

diff --git a/extensions/browser/api/socket/tcp_socket.cc b/extensions/browser/api/socket/tcp_socket.cc
index 2a8e718..a69ed02 100644
--- a/extensions/browser/api/socket/tcp_socket.cc
+++ b/extensions/browser/api/socket/tcp_socket.cc
@@ -86,6 +86,12 @@ void TCPSocket::Connect(const net::AddressList& address,
     callback.Run(net::ERR_CONNECTION_FAILED);
     return;
   }
+
+  if (is_connected_) {
+    callback.Run(net::ERR_SOCKET_IS_CONNECTED);
+    return;
+  }
+
   DCHECK(!server_socket_.get());
   socket_mode_ = CLIENT;
   connect_callback_ = callback;
@@ -130,7 +136,9 @@ void TCPSocket::Read(int count, const ReadCompletionCallback& callback) {
     return;
   }
 
-  if (!read_callback_.is_null()) {
+  if (!read_callback_.is_null() || !connect_callback_.is_null()) {
+    // It's illegal to read a net::TCPSocket while a pending Connect or Read is
+    // already in progress.
     callback.Run(net::ERR_IO_PENDING, NULL);
     return;
   }
@@ -140,7 +148,7 @@ void TCPSocket::Read(int count, const ReadCompletionCallback& callback) {
     return;
   }
 
-  if (!socket_.get()) {
+  if (!socket_.get() || !is_connected_) {
     callback.Run(net::ERR_SOCKET_NOT_CONNECTED, NULL);
     return;
   }
@@ -277,8 +285,12 @@ void TCPSocket::OnConnectComplete(int result) {
   DCHECK(!connect_callback_.is_null());
   DCHECK(!is_connected_);
   is_connected_ = result == net::OK;
-  connect_callback_.Run(result);
+
+  // The completion callback may re-enter TCPSocket, e.g. to Read(); therefore
+  // we reset |connect_callback_| before calling it.
+  CompletionCallback connect_callback = connect_callback_;
   connect_callback_.Reset();
+  connect_callback.Run(result);
 }
 
 void TCPSocket::OnReadComplete(scoped_refptr<net::IOBuffer> io_buffer,