[Backport] CVE-2021-21221: Insufficient validation of untrusted input in Mojo

Manual backport of patch originally reviewed on
https://chromium-review.googlesource.com/c/chromium/src/+/2808893:
Mojo: Remove some inappropriate DCHECKs

There are a few places where we DCHECK conditions that cannot be
reliably asserted since they depend on untrusted inputs. These are
replaced with logic to conditionally terminate the connection to the
offending peer process.

Fixed: 1195333
Change-Id: I0c6873bf55d6b0b1d0cbb3c2e5b256e1a57ff696
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Commit-Queue: Ken Rockot <rockot@google.com>
Cr-Commit-Position: refs/heads/master@{#870007}
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>

1 files changed, 16 insertions, 7 deletions

diff --git a/chromium/mojo/core/node_controller.cc b/chromium/mojo/core/node_controller.cc
index 823a4619efa..a297c500215 100644
--- a/chromium/mojo/core/node_controller.cc
+++ b/chromium/mojo/core/node_controller.cc
@@ -943,7 +943,11 @@ void NodeController::OnBrokerClientAdded(const ports::NodeName& from_node,
 void NodeController::OnAcceptBrokerClient(const ports::NodeName& from_node,
                                           const ports::NodeName& broker_name,
                                           PlatformHandle broker_channel) {
-  DCHECK(!GetConfiguration().is_broker_process);
+  if (GetConfiguration().is_broker_process) {
+    // The broker should never receive this message from anyone.
+    DropPeer(from_node, nullptr);
+    return;
+  }
 
   // This node should already have an inviter in bootstrap mode.
   ports::NodeName inviter_name;
@@ -954,8 +958,13 @@ void NodeController::OnAcceptBrokerClient(const ports::NodeName& from_node,
     inviter = bootstrap_inviter_channel_;
     bootstrap_inviter_channel_ = nullptr;
   }
-  DCHECK(inviter_name == from_node);
-  DCHECK(inviter);
+
+  if (inviter_name != from_node || !inviter ||
+      broker_name == ports::kInvalidNodeName) {
+    // We are not expecting this message. Assume the source is hostile.
+    DropPeer(from_node, nullptr);
+    return;
+  }
 
   base::queue<ports::NodeName> pending_broker_clients;
   std::unordered_map<ports::NodeName, OutgoingMessageQueue>
@@ -966,22 +975,22 @@ void NodeController::OnAcceptBrokerClient(const ports::NodeName& from_node,
     std::swap(pending_broker_clients, pending_broker_clients_);
     std::swap(pending_relay_messages, pending_relay_messages_);
   }
-  DCHECK(broker_name != ports::kInvalidNodeName);
 
   // It's now possible to add both the broker and the inviter as peers.
   // Note that the broker and inviter may be the same node.
   scoped_refptr<NodeChannel> broker;
   if (broker_name == inviter_name) {
-    DCHECK(!broker_channel.is_valid());
     broker = inviter;
-  } else {
-    DCHECK(broker_channel.is_valid());
+  } else if (broker_channel.is_valid()) {
     broker = NodeChannel::Create(
         this,
         ConnectionParams(PlatformChannelEndpoint(std::move(broker_channel))),
         Channel::HandlePolicy::kAcceptHandles, io_task_runner_,
         ProcessErrorCallback());
     AddPeer(broker_name, broker, true /* start_channel */);
+  } else {
+    DropPeer(from_node, nullptr);
+    return;
   }
 
   AddPeer(inviter_name, inviter, false /* start_channel */);