Bug 1417172 P1 Validate ClientSource creation path across IPC boundary. r=baku

dom/clients/manager/ClientManagerParent.cpp

@@ -101,6 +101,15 @@ ClientManagerParent::DeallocPClientSourceParent(PClientSourceParent* aActor)
   return true;
 }
 
+IPCResult
+ClientManagerParent::RecvPClientSourceConstructor(PClientSourceParent* aActor,
+                                                  const ClientSourceConstructorArgs& aArgs)
+{
+  ClientSourceParent* actor = static_cast<ClientSourceParent*>(aActor);
+  actor->Init();
+  return IPC_OK();
+}
+
 ClientManagerParent::ClientManagerParent()
   : mService(ClientManagerService::GetOrCreateInstance())
 {

dom/clients/manager/ClientManagerParent.h

@@ -56,6 +56,10 @@ class ClientManagerParent final : public PClientManagerParent
   bool
   DeallocPClientSourceParent(PClientSourceParent* aActor) override;
 
+  mozilla::ipc::IPCResult
+  RecvPClientSourceConstructor(PClientSourceParent* aActor,
+                               const ClientSourceConstructorArgs& aArgs) override;
+
 public:
   ClientManagerParent();
   ~ClientManagerParent();

dom/clients/manager/ClientManagerService.cpp

@@ -83,24 +83,33 @@ ClientManagerService::GetOrCreateInstance()
   return ref.forget();
 }
 
-void
+bool
 ClientManagerService::AddSource(ClientSourceParent* aSource)
 {
   AssertIsOnBackgroundThread();
   MOZ_ASSERT(aSource);
   auto entry = mSourceTable.LookupForAdd(aSource->Info().Id());
-  MOZ_DIAGNOSTIC_ASSERT(!entry);
+  // Do not permit overwriting an existing ClientSource with the same
+  // UUID.  This would allow a spoofed ClientParentSource actor to
+  // intercept postMessage() intended for the real actor.
+  if (NS_WARN_IF(!!entry)) {
+    return false;
+  }
   entry.OrInsert([&] { return aSource; });
+  return true;
 }
 
-void
+bool
 ClientManagerService::RemoveSource(ClientSourceParent* aSource)
 {
   AssertIsOnBackgroundThread();
   MOZ_ASSERT(aSource);
   auto entry = mSourceTable.Lookup(aSource->Info().Id());
-  MOZ_DIAGNOSTIC_ASSERT(entry);
+  if (NS_WARN_IF(!entry)) {
+    return false;
+  }
   entry.Remove();
+  return true;
 }
 
 ClientSourceParent*

dom/clients/manager/ClientManagerService.h

@@ -30,10 +30,10 @@ public:
   static already_AddRefed<ClientManagerService>
   GetOrCreateInstance();
 
-  void
+  bool
   AddSource(ClientSourceParent* aSource);
 
-  void
+  bool
   RemoveSource(ClientSourceParent* aSource);
 
   ClientSourceParent*

dom/clients/manager/ClientSource.cpp

@@ -9,6 +9,7 @@
 #include "ClientManager.h"
 #include "ClientManagerChild.h"
 #include "ClientSourceChild.h"
+#include "ClientValidation.h"
 #include "mozilla/dom/ClientIPCTypes.h"
 
 namespace mozilla {
@@ -47,6 +48,15 @@ ClientSource::Activate(PClientManagerChild* aActor)
     return;
   }
 
+  // Fast fail if we don't understand this particular kind of PrincipalInfo.
+  // This can happen since we use MozURL for validation which does not handle
+  // some of the more obscure internal principal/url combinations.  Normal
+  // content pages will pass this check.
+  if (NS_WARN_IF(!ClientIsValidPrincipalInfo(mClientInfo.PrincipalInfo()))) {
+    Shutdown();
+    return;
+  }
+
   ClientSourceConstructorArgs args(mClientInfo.Id(), mClientInfo.Type(),
                                    mClientInfo.PrincipalInfo(),
                                    mClientInfo.CreationTime());

dom/clients/manager/ClientSourceParent.cpp

@@ -9,15 +9,79 @@
 #include "ClientHandleParent.h"
 #include "ClientManagerService.h"
 #include "ClientSourceOpParent.h"
+#include "ClientValidation.h"
 #include "mozilla/dom/ClientIPCTypes.h"
+#include "mozilla/dom/ContentParent.h"
+#include "mozilla/dom/PClientManagerParent.h"
+#include "mozilla/ipc/BackgroundParent.h"
+#include "mozilla/SystemGroup.h"
 #include "mozilla/Unused.h"
 
 namespace mozilla {
 namespace dom {
 
+using mozilla::ipc::BackgroundParent;
 using mozilla::ipc::IPCResult;
 using mozilla::ipc::PrincipalInfo;
 
+namespace {
+
+// It would be nice to use a lambda instead of this class, but we cannot
+// move capture in lambdas yet and ContentParent cannot be AddRef'd off
+// the main thread.
+class KillContentParentRunnable final : public Runnable
+{
+  RefPtr<ContentParent> mContentParent;
+
+public:
+  explicit KillContentParentRunnable(RefPtr<ContentParent>&& aContentParent)
+    : Runnable("KillContentParentRunnable")
+    , mContentParent(Move(aContentParent))
+  {
+    MOZ_ASSERT(mContentParent);
+  }
+
+  NS_IMETHOD
+  Run()
+  {
+    MOZ_ASSERT(NS_IsMainThread());
+    mContentParent->KillHard("invalid ClientSourceParent actor");
+    mContentParent = nullptr;
+    return NS_OK;
+  }
+};
+
+} // anonymous namespace
+
+void
+ClientSourceParent::KillInvalidChild()
+{
+  // Try to get the content process before we destroy the actor below.
+  RefPtr<ContentParent> process =
+    BackgroundParent::GetContentParent(Manager()->Manager());
+
+  // First, immediately teardown the ClientSource actor.  No matter what
+  // we want to start this process as soon as possible.
+  Unused << ClientSourceParent::Send__delete__(this);
+
+  // If we are running in non-e10s, then there is nothing else to do here.
+  // There is no child process and we don't want to crash the entire browser
+  // in release builds.  In general, though, this should not happen in non-e10s
+  // so we do assert this condition.
+  if (!process) {
+    MOZ_DIAGNOSTIC_ASSERT(false, "invalid ClientSourceParent in non-e10s");
+    return;
+  }
+
+  // In e10s mode we also want to kill the child process.  Validation failures
+  // typically mean someone sent us bogus data over the IPC link.  We can't
+  // trust that process any more.  We have to do this on the main thread, so
+  // there is a small window of time before we kill the process.  This is why
+  // we start the actor destruction immediately above.
+  nsCOMPtr<nsIRunnable> r = new KillContentParentRunnable(Move(process));
+  MOZ_ALWAYS_SUCCEEDS(SystemGroup::Dispatch(TaskCategory::Other, r.forget()));
+}
+
 IPCResult
 ClientSourceParent::RecvTeardown()
 {
@@ -28,7 +92,8 @@ ClientSourceParent::RecvTeardown()
 void
 ClientSourceParent::ActorDestroy(ActorDestroyReason aReason)
 {
-  mService->RemoveSource(this);
+  DebugOnly<bool> removed = mService->RemoveSource(this);
+  MOZ_ASSERT(removed);
 
   nsTArray<ClientHandleParent*> handleList(mHandleList);
   for (ClientHandleParent* handle : handleList) {
@@ -57,7 +122,6 @@ ClientSourceParent::ClientSourceParent(const ClientSourceConstructorArgs& aArgs)
   : mClientInfo(aArgs.id(), aArgs.type(), aArgs.principalInfo(), aArgs.creationTime())
   , mService(ClientManagerService::GetOrCreateInstance())
 {
-  mService->AddSource(this);
 }
 
 ClientSourceParent::~ClientSourceParent()
@@ -65,6 +129,26 @@ ClientSourceParent::~ClientSourceParent()
   MOZ_DIAGNOSTIC_ASSERT(mHandleList.IsEmpty());
 }
 
+void
+ClientSourceParent::Init()
+{
+  // Ensure the principal is reasonable before adding ourself to the service.
+  // Since we validate the principal on the child side as well, any failure
+  // here is treated as fatal.
+  if (NS_WARN_IF(!ClientIsValidPrincipalInfo(mClientInfo.PrincipalInfo()))) {
+    KillInvalidChild();
+    return;
+  }
+
+  // Its possible for AddSource() to fail if there is already an entry for
+  // our UUID.  This should not normally happen, but could if someone is
+  // spoofing IPC messages.
+  if (NS_WARN_IF(!mService->AddSource(this))) {
+    KillInvalidChild();
+    return;
+  }
+}
+
 const ClientInfo&
 ClientSourceParent::Info() const
 {

dom/clients/manager/ClientSourceParent.h

@@ -21,6 +21,9 @@ class ClientSourceParent final : public PClientSourceParent
   RefPtr<ClientManagerService> mService;
   nsTArray<ClientHandleParent*> mHandleList;
 
+  void
+  KillInvalidChild();
+
   // PClientSourceParent
   IPCResult
   RecvTeardown() override;
@@ -38,6 +41,9 @@ public:
   explicit ClientSourceParent(const ClientSourceConstructorArgs& aArgs);
   ~ClientSourceParent();
 
+  void
+  Init();
+
   const ClientInfo&
   Info() const;
 

dom/clients/manager/ClientValidation.cpp

@@ -0,0 +1,78 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=8 sts=2 et sw=2 tw=80: */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#include "ClientValidation.h"
+
+#include "mozilla/net/MozURL.h"
+
+namespace mozilla {
+namespace dom {
+
+using mozilla::ipc::ContentPrincipalInfo;
+using mozilla::ipc::PrincipalInfo;
+using mozilla::net::MozURL;
+
+bool
+ClientIsValidPrincipalInfo(const PrincipalInfo& aPrincipalInfo)
+{
+  // Ideally we would verify that the source process has permission to
+  // create a window or worker with the given principal, but we don't
+  // currently have any such restriction in place.  Instead, at least
+  // verify the PrincipalInfo is an expected type and has a parsable
+  // origin/spec.
+  switch (aPrincipalInfo.type()) {
+    // Any system and null principal is acceptable.
+    case PrincipalInfo::TSystemPrincipalInfo:
+    case PrincipalInfo::TNullPrincipalInfo:
+    {
+      return true;
+    }
+
+    // Validate content principals to ensure that the origin and spec are sane.
+    case PrincipalInfo::TContentPrincipalInfo:
+    {
+      const ContentPrincipalInfo& content =
+       aPrincipalInfo.get_ContentPrincipalInfo();
+
+      // Verify the principal spec parses.
+      RefPtr<MozURL> specURL;
+      nsresult rv = MozURL::Init(getter_AddRefs(specURL), content.spec());
+      NS_ENSURE_SUCCESS(rv, false);
+
+      // Verify the principal originNoSuffix parses.
+      RefPtr<MozURL> originURL;
+      rv = MozURL::Init(getter_AddRefs(originURL),
+                        content.originNoSuffix().get_nsCString());
+      NS_ENSURE_SUCCESS(rv, false);
+
+      nsAutoCString originOrigin;
+      rv = originURL->GetOrigin(originOrigin);
+      NS_ENSURE_SUCCESS(rv, false);
+
+      nsAutoCString specOrigin;
+      rv = specURL->GetOrigin(specOrigin);
+      NS_ENSURE_SUCCESS(rv, false);
+
+      // For now require Clients to have a principal where both its
+      // originNoSuffix and spec have the same origin.  This will
+      // exclude a variety of unusual combinations within the browser
+      // but its adequate for the features need to support right now.
+      // If necessary we could expand this function to handle more
+      // cases in the future.
+      return specOrigin == originOrigin;
+    }
+    default:
+    {
+      break;
+    }
+  }
+
+  // Windows and workers should not have expanded URLs, etc.
+  return false;
+}
+
+} // namespace dom
+} // namespace mozilla

dom/clients/manager/ClientValidation.h

@@ -0,0 +1,23 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=8 sts=2 et sw=2 tw=80: */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+#ifndef _mozilla_dom_ClientValidation_h
+#define _mozilla_dom_ClientValidation_h
+
+namespace mozilla {
+
+namespace ipc {
+class PrincipalInfo;
+} // namespace ipc
+
+namespace dom {
+
+bool
+ClientIsValidPrincipalInfo(const mozilla::ipc::PrincipalInfo& aPrincipalInfo);
+
+} // namespace dom
+} // namespace mozilla
+
+#endif // _mozilla_dom_ClientValidation_h

dom/clients/manager/moz.build

@@ -42,6 +42,7 @@ UNIFIED_SOURCES += [
   'ClientSourceOpParent.cpp',
   'ClientSourceParent.cpp',
   'ClientState.cpp',
+  'ClientValidation.cpp',
 ]
 
 IPDL_SOURCES += [