[Backport] CVE-2020-6385 - Insufficient policy enforcement in storage

Manual backport of patch orginally reviewed on
https://chromium-review.googlesource.com/c/chromium/src/+/1975165:

Introduce ChildProcessSecurityPolicyImpl::Handle.

Merging to M80 branch

This change introduces a Handle object so that Mojo services can
preserve the security state beyond the lifetime of the
RenderProcessHostImpl object. This allows consistent security
checks to occur even during the period when the renderer process is
shutting down and there are still pending Mojo operations in flight.
This will be used to remove all remaining uses of
ChildProcessSecurityPolicyImpl::HasSecurityState() in follow-up CLs.

- Implements new Handle object that allows security checks to provide
consistent results after ChildProcessSecurityPolicyImpl::Remove() is
called.
- Convert blob code to use Handle instead of the HasSecurityState()
workaround.

This is an updated version of https://crrev.com/c/1534368 . Further
discussion of the history and reasons for this CL can be found there.

(cherry picked from commit 4fcbe415172be634fee82ecb300e50f67b27f0b1)

Bug: 1035399, 943887
Change-Id: I8967936cc894f8f66168abed8a8a2387bf3a5c20
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>

6 files changed, 312 insertions, 85 deletions

diff --git a/chromium/content/browser/blob_storage/blob_registry_wrapper.cc b/chromium/content/browser/blob_storage/blob_registry_wrapper.cc
index ad14032511d..1c418b1df9a 100644
--- a/chromium/content/browser/blob_storage/blob_registry_wrapper.cc
+++ b/chromium/content/browser/blob_storage/blob_registry_wrapper.cc
@@ -20,32 +20,23 @@ namespace {
 
 class BindingDelegate : public storage::BlobRegistryImpl::Delegate {
  public:
-  explicit BindingDelegate(int process_id) : process_id_(process_id) {}
+  explicit BindingDelegate(
+      ChildProcessSecurityPolicyImpl::Handle security_policy_handle)
+      : security_policy_handle_(std::move(security_policy_handle)) {}
   ~BindingDelegate() override {}
 
   bool CanReadFile(const base::FilePath& file) override {
-    ChildProcessSecurityPolicyImpl* security_policy =
-        ChildProcessSecurityPolicyImpl::GetInstance();
-    return security_policy->CanReadFile(process_id_, file);
+    return security_policy_handle_.CanReadFile(file);
   }
   bool CanReadFileSystemFile(const storage::FileSystemURL& url) override {
-    ChildProcessSecurityPolicyImpl* security_policy =
-        ChildProcessSecurityPolicyImpl::GetInstance();
-    return security_policy->CanReadFileSystemFile(process_id_, url);
+    return security_policy_handle_.CanReadFileSystemFile(url);
   }
   bool CanCommitURL(const GURL& url) override {
-    ChildProcessSecurityPolicyImpl* security_policy =
-        ChildProcessSecurityPolicyImpl::GetInstance();
-    return security_policy->CanCommitURL(process_id_, url);
-  }
-  bool IsProcessValid() override {
-    ChildProcessSecurityPolicyImpl* security_policy =
-        ChildProcessSecurityPolicyImpl::GetInstance();
-    return security_policy->HasSecurityState(process_id_);
+    return security_policy_handle_.CanCommitURL(url);
   }
 
  private:
-  const int process_id_;
+  ChildProcessSecurityPolicyImpl::Handle security_policy_handle_;
 };
 
 }  // namespace
@@ -69,8 +60,11 @@ BlobRegistryWrapper::BlobRegistryWrapper() {
 void BlobRegistryWrapper::Bind(int process_id,
                                blink::mojom::BlobRegistryRequest request) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  blob_registry_->Bind(std::move(request),
-                       std::make_unique<BindingDelegate>(process_id));
+  blob_registry_->Bind(
+      std::move(request),
+      std::make_unique<BindingDelegate>(
+          ChildProcessSecurityPolicyImpl::GetInstance()->CreateHandle(
+              process_id)));
 }
 
 BlobRegistryWrapper::~BlobRegistryWrapper() {}
diff --git a/chromium/content/browser/browser_context.cc b/chromium/content/browser/browser_context.cc
index 5fbd75f67fb..d86b478a9e5 100644
--- a/chromium/content/browser/browser_context.cc
+++ b/chromium/content/browser/browser_context.cc
@@ -572,13 +572,13 @@ void BrowserContext::NotifyWillBeDestroyed(BrowserContext* browser_context) {
     }
   }
 
-  // Clean up any isolated origins associated with this BrowserContext.  This
-  // should be safe now that all RenderProcessHosts are destroyed, since future
-  // navigations or security decisions shouldn't ever need to consult these
-  // isolated origins.
+  // Clean up any isolated origins and other security state associated with this
+  // BrowserContext.  This should be safe now that all RenderProcessHosts are
+  // destroyed, since future navigations or security decisions shouldn't ever
+  // need to consult these isolated origins and other security state.
   ChildProcessSecurityPolicyImpl* policy =
       ChildProcessSecurityPolicyImpl::GetInstance();
-  policy->RemoveIsolatedOriginsForBrowserContext(*browser_context);
+  policy->RemoveStateForBrowserContext(*browser_context);
 }
 
 void BrowserContext::EnsureResourceContextInitialized(BrowserContext* context) {
diff --git a/chromium/content/browser/child_process_security_policy_impl.cc b/chromium/content/browser/child_process_security_policy_impl.cc
index 3c18dc775e8..bd1beaa9a7b 100644
--- a/chromium/content/browser/child_process_security_policy_impl.cc
+++ b/chromium/content/browser/child_process_security_policy_impl.cc
@@ -32,6 +32,7 @@
 #include "content/public/browser/site_isolation_policy.h"
 #include "content/public/browser/storage_partition.h"
 #include "content/public/common/bindings_policy.h"
+#include "content/public/common/child_process_host.h"
 #include "content/public/common/url_constants.h"
 #include "net/base/filename_util.h"
 #include "net/base/url_util.h"
@@ -130,6 +131,88 @@ base::debug::CrashKeyString* GetRequestedOriginCrashKey() {
 
 }  // namespace
 
+ChildProcessSecurityPolicyImpl::Handle::Handle()
+    : child_id_(ChildProcessHost::kInvalidUniqueID) {}
+
+ChildProcessSecurityPolicyImpl::Handle::Handle(int child_id)
+    : child_id_(child_id) {
+  auto* policy = ChildProcessSecurityPolicyImpl::GetInstance();
+  if (!policy->AddProcessReference(child_id_))
+    child_id_ = ChildProcessHost::kInvalidUniqueID;
+}
+
+ChildProcessSecurityPolicyImpl::Handle::Handle(Handle&& rhs)
+    : child_id_(rhs.child_id_) {
+  rhs.child_id_ = ChildProcessHost::kInvalidUniqueID;
+}
+
+ChildProcessSecurityPolicyImpl::Handle::~Handle() {
+  if (child_id_ != ChildProcessHost::kInvalidUniqueID) {
+    auto* policy = ChildProcessSecurityPolicyImpl::GetInstance();
+    policy->RemoveProcessReference(child_id_);
+  }
+}
+
+ChildProcessSecurityPolicyImpl::Handle& ChildProcessSecurityPolicyImpl::Handle::
+operator=(Handle&& rhs) {
+  if (child_id_ != ChildProcessHost::kInvalidUniqueID &&
+      child_id_ != rhs.child_id_) {
+    auto* policy = ChildProcessSecurityPolicyImpl::GetInstance();
+    policy->RemoveProcessReference(child_id_);
+  }
+  child_id_ = rhs.child_id_;
+  rhs.child_id_ = ChildProcessHost::kInvalidUniqueID;
+  return *this;
+}
+
+bool ChildProcessSecurityPolicyImpl::Handle::is_valid() const {
+  return child_id_ != ChildProcessHost::kInvalidUniqueID;
+}
+
+bool ChildProcessSecurityPolicyImpl::Handle::CanCommitURL(const GURL& url) {
+  if (child_id_ == ChildProcessHost::kInvalidUniqueID)
+    return false;
+
+  auto* policy = ChildProcessSecurityPolicyImpl::GetInstance();
+  return policy->CanCommitURL(child_id_, url);
+}
+
+bool ChildProcessSecurityPolicyImpl::Handle::CanReadFile(
+    const base::FilePath& file) {
+  if (child_id_ == ChildProcessHost::kInvalidUniqueID)
+    return false;
+
+  auto* policy = ChildProcessSecurityPolicyImpl::GetInstance();
+  return policy->CanReadFile(child_id_, file);
+}
+
+bool ChildProcessSecurityPolicyImpl::Handle::CanReadFileSystemFile(
+    const storage::FileSystemURL& url) {
+  if (child_id_ == ChildProcessHost::kInvalidUniqueID)
+    return false;
+
+  auto* policy = ChildProcessSecurityPolicyImpl::GetInstance();
+  return policy->CanReadFileSystemFile(child_id_, url);
+}
+
+bool ChildProcessSecurityPolicyImpl::Handle::CanAccessDataForOrigin(
+    const GURL& url) {
+  if (child_id_ == ChildProcessHost::kInvalidUniqueID)
+    return false;
+
+  auto* policy = ChildProcessSecurityPolicyImpl::GetInstance();
+  return policy->CanAccessDataForOrigin(child_id_, url);
+}
+
+bool ChildProcessSecurityPolicyImpl::Handle::CanAccessDataForOrigin(
+    const url::Origin& origin) {
+  if (child_id_ == ChildProcessHost::kInvalidUniqueID)
+    return false;
+
+  auto* policy = ChildProcessSecurityPolicyImpl::GetInstance();
+  return policy->CanAccessDataForOrigin(child_id_, origin);
+}
+
 // The SecurityState class is used to maintain per-child process security state
 // information.
 class ChildProcessSecurityPolicyImpl::SecurityState {
@@ -356,7 +439,10 @@ class ChildProcessSecurityPolicyImpl::SecurityState {
     return BrowserOrResourceContext();
   }
 
-  void ClearBrowserContext() { browser_context_ = nullptr; }
+  void ClearBrowserContextIfMatches(const BrowserContext* browser_context) {
+      if (browser_context == browser_context_)
+        browser_context_ = nullptr;
+  }
 
  private:
   enum class CommitRequestPolicy {
@@ -526,12 +612,13 @@ void ChildProcessSecurityPolicyImpl::Add(int child_id,
   DCHECK(browser_context);
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   base::AutoLock lock(lock_);
-  if (security_state_.count(child_id) != 0) {
+  if (security_state_.find(child_id) != security_state_.end()) {
     NOTREACHED() << "Add child process at most once.";
     return;
   }
 
   security_state_[child_id] = std::make_unique<SecurityState>(browser_context);
+  CHECK(AddProcessReferenceLocked(child_id));
 }
 
 void ChildProcessSecurityPolicyImpl::Remove(int child_id) {
@@ -542,28 +629,13 @@ void ChildProcessSecurityPolicyImpl::Remove(int child_id) {
   if (state == security_state_.end())
     return;
 
-  state->second->ClearBrowserContext();
-
   // Moving the existing SecurityState object into a pending map so
   // that we can preserve permission state and avoid mutations to this
   // state after Remove() has been called.
   pending_remove_state_[child_id] = std::move(state->second);
   security_state_.erase(child_id);
 
-  // |child_id| could be inside tasks that are on the IO thread task queues. We
-  // need to keep the |pending_remove_state_| entry around until we have
-  // successfully executed a task on the IO thread. This should ensure that any
-  // pending tasks on the IO thread will have completed before we remove the
-  // entry.
-  base::PostTaskWithTraits(
-      FROM_HERE, {BrowserThread::IO},
-      base::BindOnce(
-          [](ChildProcessSecurityPolicyImpl* policy, int child_id) {
-            DCHECK_CURRENTLY_ON(BrowserThread::IO);
-            base::AutoLock lock(policy->lock_);
-            policy->pending_remove_state_.erase(child_id);
-          },
-          base::Unretained(this), child_id));
+  RemoveProcessReferenceLocked(child_id);
 }
 
 void ChildProcessSecurityPolicyImpl::RegisterWebSafeScheme(
@@ -975,13 +1047,13 @@ bool ChildProcessSecurityPolicyImpl::CanCommitURL(int child_id,
     if (base::Contains(schemes_okay_to_commit_in_any_process_, scheme))
       return true;
 
-    auto state = security_state_.find(child_id);
-    if (state == security_state_.end())
+    auto* state = GetSecurityState(child_id);
+    if (!state)
       return false;
 
     // Otherwise, we consult the child process's security state to see if it is
     // allowed to commit the URL.
-    return state->second->CanCommitURL(url);
+    return state->CanCommitURL(url);
   }
 }
 
@@ -1254,10 +1326,10 @@ bool ChildProcessSecurityPolicyImpl::CanReadRawCookies(int child_id) {
 
 bool ChildProcessSecurityPolicyImpl::ChildProcessHasPermissionsForFile(
     int child_id, const base::FilePath& file, int permissions) {
-  auto state = security_state_.find(child_id);
-  if (state == security_state_.end())
+  auto* state = GetSecurityState(child_id);
+  if (!state)
     return false;
-  return state->second->HasPermissionsForFile(file, permissions);
+  return state->HasPermissionsForFile(file, permissions);
 }
 
 bool ChildProcessSecurityPolicyImpl::CanAccessDataForOrigin(
@@ -1367,10 +1439,10 @@ bool ChildProcessSecurityPolicyImpl::HasPermissionsForFileSystem(
     int permission) {
   base::AutoLock lock(lock_);
 
-  auto state = security_state_.find(child_id);
-  if (state == security_state_.end())
+  auto* state = GetSecurityState(child_id);
+  if (!state)
     return false;
-  return state->second->HasPermissionsForFileSystem(filesystem_id, permission);
+  return state->HasPermissionsForFileSystem(filesystem_id, permission);
 }
 
 void ChildProcessSecurityPolicyImpl::RegisterFileSystemPermissionPolicy(
@@ -1492,22 +1564,33 @@ void ChildProcessSecurityPolicyImpl::AddIsolatedOrigins(
   }
 }
 
-void ChildProcessSecurityPolicyImpl::RemoveIsolatedOriginsForBrowserContext(
+void ChildProcessSecurityPolicyImpl::RemoveStateForBrowserContext(
     const BrowserContext& browser_context) {
-  base::AutoLock isolated_origins_lock(isolated_origins_lock_);
+  {
+    base::AutoLock isolated_origins_lock(isolated_origins_lock_);
+
+    for (auto& iter : isolated_origins_) {
+      base::EraseIf(iter.second,
+                    [&browser_context](const IsolatedOriginEntry& entry) {
+                      // Remove if BrowserContext matches.
+                      return (entry.browser_context() == &browser_context);
+                    });
+    }
 
-  for (auto& iter : isolated_origins_) {
-    base::EraseIf(iter.second,
-                  [&browser_context](const IsolatedOriginEntry& entry) {
-                    // Remove if BrowserContext matches.
-                    return (entry.browser_context() == &browser_context);
-                  });
+    // Also remove map entries for site URLs which no longer have any
+    // IsolatedOriginEntries remaining.
+    base::EraseIf(isolated_origins_,
+                  [](const auto& pair) { return pair.second.empty(); });
   }
 
-  // Also remove map entries for site URLs which no longer have any
-  // IsolatedOriginEntries remaining.
-  base::EraseIf(isolated_origins_,
-                [](const auto& pair) { return pair.second.empty(); });
+  {
+    base::AutoLock lock(lock_);
+    for (auto& pair : security_state_)
+      pair.second->ClearBrowserContextIfMatches(&browser_context);
+
+    for (auto& pair : pending_remove_state_)
+      pair.second->ClearBrowserContextIfMatches(&browser_context);
+  }
 }
 
 bool ChildProcessSecurityPolicyImpl::IsIsolatedOrigin(
@@ -1680,15 +1763,29 @@ ChildProcessSecurityPolicyImpl::GetSecurityState(int child_id) {
   if (itr != security_state_.end())
     return itr->second.get();
 
-  // Check to see if |child_id| is in the pending removal map since this
-  // may be a call that was already on the IO thread's task queue when the
-  // Remove() call occurred.
-  if (BrowserThread::CurrentlyOn(BrowserThread::IO)) {
-    itr = pending_remove_state_.find(child_id);
-    if (itr != pending_remove_state_.end())
-      return itr->second.get();
+  auto pending_itr = pending_remove_state_.find(child_id);
+  if (pending_itr == pending_remove_state_.end())
+    return nullptr;
+
+  // At this point the SecurityState in the map is being kept alive
+  // by a Handle object or we are waiting for the deletion task to be run on
+  // the IO thread.
+  SecurityState* pending_security_state = pending_itr->second.get();
+
+  auto count_itr = process_reference_counts_.find(child_id);
+  if (count_itr != process_reference_counts_.end()) {
+    // There must be a Handle that still holds a reference to this
+    // pending state so it is safe to return. The assumption is that the
+    // owner of this Handle is making a security check.
+    return pending_security_state;
   }
 
+  // Since we don't have an entry in |process_reference_counts_| it means
+  // that we are waiting for the deletion task posted to the IO thread to run.
+  // Only allow the state to be accessed by the IO thread in this situation.
+  if (BrowserThread::CurrentlyOn(BrowserThread::IO))
+    return pending_security_state;
+
   return nullptr;
 }
 
@@ -1707,4 +1804,59 @@ ChildProcessSecurityPolicyImpl::ParseIsolatedOrigins(
   return patterns;
 }
 
+ChildProcessSecurityPolicyImpl::Handle
+ChildProcessSecurityPolicyImpl::CreateHandle(int child_id) {
+  return Handle(child_id);
+}
+
+bool ChildProcessSecurityPolicyImpl::AddProcessReference(int child_id) {
+  base::AutoLock lock(lock_);
+  return AddProcessReferenceLocked(child_id);
+}
+
+bool ChildProcessSecurityPolicyImpl::AddProcessReferenceLocked(int child_id) {
+  // Make sure that we aren't trying to add references after the process has
+  // been destroyed.
+  if (security_state_.find(child_id) == security_state_.end())
+    return false;
+
+  ++process_reference_counts_[child_id];
+  return true;
+}
+
+void ChildProcessSecurityPolicyImpl::RemoveProcessReference(int child_id) {
+  base::AutoLock lock(lock_);
+  RemoveProcessReferenceLocked(child_id);
+}
+
+void ChildProcessSecurityPolicyImpl::RemoveProcessReferenceLocked(
+    int child_id) {
+  auto itr = process_reference_counts_.find(child_id);
+  CHECK(itr != process_reference_counts_.end());
+
+  if (itr->second > 1) {
+    itr->second--;
+    return;
+  }
+
+  DCHECK_EQ(itr->second, 1);
+  process_reference_counts_.erase(itr);
+
+  // |child_id| could be inside tasks that are on the IO thread task queues. We
+  // need to keep the |pending_remove_state_| entry around until we have
+  // successfully executed a task on the IO thread. This should ensure that any
+  // pending tasks on the IO thread will have completed before we remove the
+  // entry.
+  // TODO(acolwell): Remove this call once all objects on the IO thread have
+  // been converted to use Handles.
+  base::PostTask(FROM_HERE, {BrowserThread::IO},
+                 base::BindOnce(
+                     [](ChildProcessSecurityPolicyImpl* policy, int child_id) {
+                       DCHECK_CURRENTLY_ON(BrowserThread::IO);
+                       base::AutoLock lock(policy->lock_);
+                       policy->pending_remove_state_.erase(child_id);
+                     },
+                     base::Unretained(this), child_id));
+}
+
 }  // namespace content
diff --git a/chromium/content/browser/child_process_security_policy_impl.h b/chromium/content/browser/child_process_security_policy_impl.h
index 7df496559df..8f8ddddb35f 100644
--- a/chromium/content/browser/child_process_security_policy_impl.h
+++ b/chromium/content/browser/child_process_security_policy_impl.h
@@ -52,6 +52,68 @@ class SiteInstance;
 class CONTENT_EXPORT ChildProcessSecurityPolicyImpl
     : public ChildProcessSecurityPolicy {
  public:
+  // Handle used to access the security state for a specific process.
+  //
+  // Objects that require the security state to be preserved beyond the
+  // lifetime of the RenderProcessHostImpl should hold an instance of this
+  // object and use it to answer security policy questions. (e.g. Mojo services
+  // created by RPHI that can receive calls after RPHI destruction). This
+  // object should only be called on the UI and IO threads.
+  //
+  // Note: Some security methods, like CanAccessDataForOrigin(), require
+  // information from the BrowserContext to make its decisions. These methods
+  // will fall back to failsafe values if called after BrowserContext
+  // destruction. Callers should be prepared to gracefully handle this or
+  // ensure that they don't make any calls after BrowserContext destruction.
+  class CONTENT_EXPORT Handle {
+   public:
+    Handle();
+    Handle(Handle&&);
+    Handle(const Handle&) = delete;
+    ~Handle();
+
+    Handle& operator=(const Handle&) = delete;
+    Handle& operator=(Handle&&);
+
+    // Returns true if this object has a valid process ID.
+    // Returns false if this object was created with the default constructor,
+    // the contents of this object was transferred to another Handle via
+    // std::move(), or ChildProcessSecurityPolicyImpl::CreateHandle()
+    // created this object after the process has already been destructed.
+    bool is_valid() const;
+
+    // Whether the process is allowed to commit a document from the given URL.
+    bool CanCommitURL(const GURL& url);
+
+    // Before servicing a child process's request to upload a file to the web,
+    // the browser should call this method to determine whether the process has
+    // the capability to upload the requested file.
+    bool CanReadFile(const base::FilePath& file);
+
+    // Explicit read permissions check for FileSystemURL specified files.
+    bool CanReadFileSystemFile(const storage::FileSystemURL& url);
+
+    // Returns true if the process is permitted to read and modify the data for
+    // the origin of |url|. This is currently used to protect data such as
+    // cookies, passwords, and local storage. Does not affect cookies attached
+    // to or set by network requests.
+    //
+    // This can only return false for processes locked to a particular origin,
+    // which can happen for any origin when the --site-per-process flag is used,
+    // or for isolated origins that require a dedicated process (see
+    // AddIsolatedOrigins).
+    bool CanAccessDataForOrigin(const GURL& url);
+    bool CanAccessDataForOrigin(const url::Origin& origin);
+
+   private:
+    friend class ChildProcessSecurityPolicyImpl;
+    explicit Handle(int child_id);
+
+    // The ID of the child process that this handle is associated with or
+    // ChildProcessHost::kInvalidUniqueID if the handle is no longer valid.
+    int child_id_;
+  };
+
   // Object can only be created through GetInstance() so the constructor is
   // private.
   ~ChildProcessSecurityPolicyImpl() override;
@@ -283,14 +345,14 @@ class CONTENT_EXPORT ChildProcessSecurityPolicyImpl
   // Returns true if sending system exclusive messages is allowed.
   bool CanSendMidiSysExMessage(int child_id);
 
-  // Remove all isolated origins associated with |browser_context|.  This is
+  // Remove all isolated origins associated with |browser_context| and clear any
+  // pointers that may reference |browser_context|.  This is
   // typically used when |browser_context| is being destroyed and assumes that
   // no processes are running or will run for that profile; this makes the
   // isolated origin removal safe.  Note that |browser_context| cannot be null;
   // i.e., isolated origins that apply globally to all profiles cannot
   // currently be removed, since that is not safe to do at runtime.
-  void RemoveIsolatedOriginsForBrowserContext(
-      const BrowserContext& browser_context);
+  void RemoveStateForBrowserContext(const BrowserContext& browser_context);
 
   // Check whether |origin| requires origin-wide process isolation within
   // |isolation_context|.
@@ -330,9 +392,23 @@ class CONTENT_EXPORT ChildProcessSecurityPolicyImpl
   // implemented for the one caller doing Blob URL revocation.
   bool HasSecurityState(int child_id);
 
+  // Creates a Handle object for a specific child process ID.
+  //
+  // This handle can be used to extend the lifetime of policy state beyond
+  // the Remove() call for |child_id|. This should be used by objects that can
+  // outlive the RenderProcessHostImpl object associated with |child_id| and
+  // need to be able to make policy decisions after RPHI destruction. (e.g.
+  // Mojo services created by RPHI)
+  //
+  // Returns a valid Handle for any |child_id| that is present in
+  // |security_state_|. Otherwise it returns a Handle that returns false for
+  // all policy checks.
+  Handle CreateHandle(int child_id);
+
  private:
   friend class ChildProcessSecurityPolicyInProcessBrowserTest;
   friend class ChildProcessSecurityPolicyTest;
+  friend class ChildProcessSecurityPolicyImpl::Handle;
   FRIEND_TEST_ALL_PREFIXES(ChildProcessSecurityPolicyInProcessBrowserTest,
                            NoLeak);
   FRIEND_TEST_ALL_PREFIXES(ChildProcessSecurityPolicyTest, FilePermissions);
@@ -500,6 +576,13 @@ class CONTENT_EXPORT ChildProcessSecurityPolicyImpl
                           IsolatedOriginSource source,
                           BrowserContext* browser_context = nullptr);
 
+  bool AddProcessReference(int child_id);
+  bool AddProcessReferenceLocked(int child_id) EXCLUSIVE_LOCKS_REQUIRED(lock_);
+  void RemoveProcessReference(int child_id);
+  void RemoveProcessReferenceLocked(int child_id)
+      EXCLUSIVE_LOCKS_REQUIRED(lock_);
+
+
   // You must acquire this lock before reading or writing any members of this
   // class, except for isolated_origins_ which uses its own lock.  You must not
   // block while holding this lock.
@@ -532,6 +615,13 @@ class CONTENT_EXPORT ChildProcessSecurityPolicyImpl
 
   FileSystemPermissionPolicyMap file_system_policy_map_ GUARDED_BY(lock_);
 
+  // Contains a mapping between child process ID and the number of outstanding
+  // references that want to keep the SecurityState for each process alive.
+  // This object and Handles created by this object increment/decrement
+  // the counts in this map and only destroy a SecurityState object for a
+  // process when its count goes to zero.
+  std::map<int, int> process_reference_counts_ GUARDED_BY(lock_);
+
   // You must acquire this lock before reading or writing isolated_origins_.
   // You must not block while holding this lock.
   //
diff --git a/chromium/storage/browser/blob/blob_registry_impl.h b/chromium/storage/browser/blob/blob_registry_impl.h
index 653feea5b5d..38101562ba2 100644
--- a/chromium/storage/browser/blob/blob_registry_impl.h
+++ b/chromium/storage/browser/blob/blob_registry_impl.h
@@ -32,7 +32,6 @@ class COMPONENT_EXPORT(STORAGE_BROWSER) BlobRegistryImpl
     virtual bool CanReadFile(const base::FilePath& file) = 0;
     virtual bool CanReadFileSystemFile(const FileSystemURL& url) = 0;
     virtual bool CanCommitURL(const GURL& url) = 0;
-    virtual bool IsProcessValid() = 0;
   };
 
   BlobRegistryImpl(base::WeakPtr<BlobStorageContext> context,
diff --git a/chromium/storage/browser/blob/blob_url_store_impl.cc b/chromium/storage/browser/blob/blob_url_store_impl.cc
index 75895540f1a..030d9adf5dc 100644
--- a/chromium/storage/browser/blob/blob_url_store_impl.cc
+++ b/chromium/storage/browser/blob/blob_url_store_impl.cc
@@ -81,12 +81,8 @@ void BlobURLStoreImpl::Register(blink::mojom::BlobPtr blob,
     std::move(callback).Run();
     return;
   }
-  // Only report errors when we don't have permission to commit and
-  // the process is valid. The process check is a temporary solution to
-  // handle cases where this method is run after the
-  // process associated with |delegate_| has been destroyed.
-  // See https://crbug.com/933089 for details.
-  if (!delegate_->CanCommitURL(url) && delegate_->IsProcessValid()) {
+
+  if (!delegate_->CanCommitURL(url)) {
     mojo::ReportBadMessage(
         "Non committable URL passed to BlobURLStore::Register");
     std::move(callback).Run();
@@ -110,12 +106,8 @@ void BlobURLStoreImpl::Revoke(const GURL& url) {
     mojo::ReportBadMessage("Invalid scheme passed to BlobURLStore::Revoke");
     return;
   }
-  // Only report errors when we don't have permission to commit and
-  // the process is valid. The process check is a temporary solution to
-  // handle cases where this method is run after the
-  // process associated with |delegate_| has been destroyed.
-  // See https://crbug.com/933089 for details.
-  if (!delegate_->CanCommitURL(url) && delegate_->IsProcessValid()) {
+
+  if (!delegate_->CanCommitURL(url)) {
     mojo::ReportBadMessage(
         "Non committable URL passed to BlobURLStore::Revoke");
     return;