Implement argument validation for chrome.cast.channel.{open,send}

chrome/browser/extensions/api/cast_channel/cast_channel_api.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/extensions/api/cast_channel/cast_channel_api.h"
 
 #include <limits>
+#include <string>
 
 #include "base/json/json_writer.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/values.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/extensions/api/cast_channel/cast_socket.h"
 #include "chrome/browser/net/chrome_net_log.h"
 #include "content/public/browser/browser_thread.h"
 #include "extensions/browser/event_router.h"
@@ skipping 35 matching lines @@
   channel_info->channel_id = socket.id();
   channel_info->url = socket.CastUrl();
   const net::IPEndPoint& ip_endpoint = socket.ip_endpoint();
   channel_info->connect_info.ip_address = ip_endpoint.ToStringWithoutPort();
   channel_info->connect_info.port = ip_endpoint.port();
   channel_info->connect_info.auth = socket.channel_auth();
   channel_info->ready_state = socket.ready_state();
   channel_info->error_state = socket.error_state();
 }
 
+bool IsValidConnectInfoPort(const ConnectInfo& connect_info) {
+  return connect_info.port > 0 && connect_info.port <
+    std::numeric_limits<unsigned short>::max();
+}
+
+bool IsValidConnectInfoAuth(const ConnectInfo& connect_info) {
+  return connect_info.auth == cast_channel::CHANNEL_AUTH_TYPE_SSL_VERIFIED ||
+    connect_info.auth == cast_channel::CHANNEL_AUTH_TYPE_SSL;
+}
+
+bool IsValidConnectInfoIpAddress(const ConnectInfo& connect_info) {
+  net::IPAddressNumber ip_address;
+  return net::ParseIPLiteralToNumber(connect_info.ip_address, &ip_address);
+}
+
 }  // namespace
 
 CastChannelAPI::CastChannelAPI(content::BrowserContext* context)
     : browser_context_(context) {
   DCHECK(browser_context_);
 }
 
 // static
 CastChannelAPI* CastChannelAPI::Get(content::BrowserContext* context) {
   return BrowserContextKeyedAPIFactory<CastChannelAPI>::Get(context);
@@ skipping 164 matching lines @@
   VLOG(2) << "IP: " << ip_address_str << " Port: " << port_str;
   int port;
   if (!base::StringToInt(port_str, &port))
     return false;
   connect_info->ip_address = ip_address_str;
   connect_info->port = port;
   connect_info->auth = auth_required ?
     cast_channel::CHANNEL_AUTH_TYPE_SSL_VERIFIED :
     cast_channel::CHANNEL_AUTH_TYPE_SSL;
   return true;
-};
+}
 
 net::IPEndPoint* CastChannelOpenFunction::ParseConnectInfo(
     const ConnectInfo& connect_info) {
   net::IPAddressNumber ip_address;
-  if (!net::ParseIPLiteralToNumber(connect_info.ip_address, &ip_address)) {
-    return NULL;
-  }
-  if (connect_info.port < 0 || connect_info.port >
-      std::numeric_limits<unsigned short>::max()) {
-    return NULL;
-  }
-  if (connect_info.auth != cast_channel::CHANNEL_AUTH_TYPE_SSL_VERIFIED &&
-      connect_info.auth != cast_channel::CHANNEL_AUTH_TYPE_SSL) {
-    return NULL;
-  }
+  CHECK(net::ParseIPLiteralToNumber(connect_info.ip_address, &ip_address));
   return new net::IPEndPoint(ip_address, connect_info.port);
 }
 
 bool CastChannelOpenFunction::PrePrepare() {
   api_ = CastChannelAPI::Get(browser_context());
   return CastChannelAsyncApiFunction::PrePrepare();
 }
 
 bool CastChannelOpenFunction::Prepare() {
   params_ = Open::Params::Create(*args_);
   EXTENSION_FUNCTION_VALIDATE(params_.get());
   // The connect_info parameter may be a string URL like cast:// or casts:// or
   // a ConnectInfo object.
   std::string cast_url;
+  bool is_connect_info_url = false;
+  bool is_connect_info_dict = false;
   switch (params_->connect_info->GetType()) {
     case base::Value::TYPE_STRING:
+      is_connect_info_url = true;
       CHECK(params_->connect_info->GetAsString(&cast_url));
       connect_info_.reset(new ConnectInfo);
       if (!ParseChannelUrl(GURL(cast_url), connect_info_.get())) {
         connect_info_.reset();
       }
       break;
     case base::Value::TYPE_DICTIONARY:
+      is_connect_info_dict = true;
       connect_info_ = ConnectInfo::FromValue(*(params_->connect_info));
       break;
     default:
       break;
   }
-  if (connect_info_.get()) {
-    channel_auth_ = connect_info_->auth;
-    ip_endpoint_.reset(ParseConnectInfo(*connect_info_));
-    return ip_endpoint_.get() != NULL;
+
+  DCHECK(!is_connect_info_dict || !is_connect_info_url);
+  if (!connect_info_.get()) {
+    if (!is_connect_info_dict && !is_connect_info_url) {
+      SetError("Invalid connect_info (unknown type)");

[Wez, 2014/07/17 21:16:47]

This can just go in the default: case, surely?

[mark a. foltz, 2014/07/17 22:23:40]

Done.

+    } else if (is_connect_info_dict) {
+      SetError("connect_info.auth is required");

[Wez, 2014/07/17 21:16:47]

I was actually thinking we could just get rid of the AUTH NONE failure mode in
FromValue() entirely, since it'll be caught by the IsValidConnectInfoAuth(),
below, won't it?

[mark a. foltz, 2014/07/17 22:23:40]

FromValue() is generated code that can't be changed.  It is intended to verify
the enum is set.  I think it's clearer to explicitly check the value as well.

+    } else {
+      SetError("Invalid connect_info (invalid Cast URL " + cast_url + ")");

[Wez, 2014/07/17 21:16:47]

This could go in the TYPE_STRING failure block, if FromValue() is fixed to
never-fail.

[mark a. foltz, 2014/07/17 22:23:40]

Done.

+    }
+    return false;
   }
-  return false;
+
+  if (!IsValidConnectInfoPort(*connect_info_)) {
+    SetError("Invalid connect_info (invalid port)");
+  } else if (!IsValidConnectInfoAuth(*connect_info_)) {
+    SetError("Invalid connect_info (invalid auth)");
+  } else if (!IsValidConnectInfoIpAddress(*connect_info_)) {
+    SetError("Invalid connect_info (invalid IP address)");
+  }
+  if (!GetError().empty()) {
+    return false;
+  }
+  channel_auth_ = connect_info_->auth;
+  ip_endpoint_.reset(ParseConnectInfo(*connect_info_));
+  return true;
 }
 
 void CastChannelOpenFunction::AsyncWorkStart() {
   DCHECK(api_);
   DCHECK(ip_endpoint_.get());
   scoped_ptr<CastSocket> socket = api_->CreateCastSocket(
       extension_->id(), *ip_endpoint_, channel_auth_);
   new_channel_id_ = AddSocket(socket.release());
   GetSocket(new_channel_id_)->Connect(
       base::Bind(&CastChannelOpenFunction::OnOpen, this));
 }
 
 void CastChannelOpenFunction::OnOpen(int result) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   SetResultFromSocket(new_channel_id_);
   AsyncWorkCompleted();
 }
 
 CastChannelSendFunction::CastChannelSendFunction() { }
 
 CastChannelSendFunction::~CastChannelSendFunction() { }
 
 bool CastChannelSendFunction::Prepare() {
   params_ = Send::Params::Create(*args_);
   EXTENSION_FUNCTION_VALIDATE(params_.get());
+  if (params_->message.namespace_.empty()) {
+    SetError("message_info.namespace_ is required");
+    return false;
+  }
+  if (params_->message.source_id.empty()) {
+    SetError("message_info.source_id is required");
+    return false;
+  }
+  if (params_->message.destination_id.empty()) {
+    SetError("message_info.destination_id is required");
+    return false;
+  }
+  switch (params_->message.data->GetType()) {
+    case base::Value::TYPE_STRING:
+    case base::Value::TYPE_BINARY:
+      break;
+    default:
+      SetError("Invalid type of message_info.data");
+      return false;
+  }
   return true;
 }
 
 void CastChannelSendFunction::AsyncWorkStart() {
   CastSocket* socket = GetSocketOrCompleteWithError(
       params_->channel.channel_id);
   if (socket)
     socket->SendMessage(params_->message,
                         base::Bind(&CastChannelSendFunction::OnSend, this));
 }
@@ skipping 32 matching lines @@
     SetResultFromError(cast_channel::CHANNEL_ERROR_SOCKET_ERROR);
   } else {
     int channel_id = params_->channel.channel_id;
     SetResultFromSocket(channel_id);
     RemoveSocket(channel_id);
   }
   AsyncWorkCompleted();
 }
 
 }  // namespace extensions