WebSQL: Validate origins of requests from child processes

Bug: 467150
Change-Id: I4d45b7aab0f8fcac5fe0d812c172d9fa0194f9b9
Reviewed-on: https://chromium-review.googlesource.com/1174920
Commit-Queue: Joshua Bell <jsbell@chromium.org>
Reviewed-by: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584162}

content/browser/renderer_host/web_database_host_impl.cc

@@ -9,6 +9,7 @@
 
 #include "base/metrics/histogram_macros.h"
 #include "base/strings/utf_string_conversions.h"
+#include "content/browser/child_process_security_policy_impl.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/common/origin_util.h"
@@ -33,15 +34,6 @@ namespace {
 const int kNumDeleteRetries = 2;
 // The delay between each retry to delete the SQLite database.
 const int kDelayDeleteRetryMs = 100;
-
-bool ValidateOrigin(const url::Origin& origin) {
-  if (origin.unique()) {
-    mojo::ReportBadMessage("Invalid Origin.");
-    return false;
-  }
-  return true;
-}
-
 }  // namespace
 
 WebDatabaseHostImpl::WebDatabaseHostImpl(
@@ -80,6 +72,9 @@ void WebDatabaseHostImpl::OpenFile(const base::string16& vfs_file_name,
                                    int32_t desired_flags,
                                    OpenFileCallback callback) {
   DCHECK(db_tracker_->task_runner()->RunsTasksInCurrentSequence());
+  if (!ValidateOrigin(vfs_file_name))
+    return;
+
   base::File file;
   const base::File* tracked_file = nullptr;
   std::string origin_identifier;
@@ -130,6 +125,9 @@ void WebDatabaseHostImpl::DeleteFile(const base::string16& vfs_file_name,
                                      bool sync_dir,
                                      DeleteFileCallback callback) {
   DCHECK(db_tracker_->task_runner()->RunsTasksInCurrentSequence());
+  if (!ValidateOrigin(vfs_file_name))
+    return;
+
   DatabaseDeleteFile(vfs_file_name, sync_dir, std::move(callback),
                      kNumDeleteRetries);
 }
@@ -138,6 +136,9 @@ void WebDatabaseHostImpl::GetFileAttributes(
     const base::string16& vfs_file_name,
     GetFileAttributesCallback callback) {
   DCHECK(db_tracker_->task_runner()->RunsTasksInCurrentSequence());
+  if (!ValidateOrigin(vfs_file_name))
+    return;
+
   int32_t attributes = -1;
   base::FilePath db_file =
       DatabaseUtil::GetFullFilePathForVfsFile(db_tracker_.get(), vfs_file_name);
@@ -150,6 +151,9 @@ void WebDatabaseHostImpl::GetFileAttributes(
 void WebDatabaseHostImpl::GetFileSize(const base::string16& vfs_file_name,
                                       GetFileSizeCallback callback) {
   DCHECK(db_tracker_->task_runner()->RunsTasksInCurrentSequence());
+  if (!ValidateOrigin(vfs_file_name))
+    return;
+
   int64_t size = 0LL;
   base::FilePath db_file =
       DatabaseUtil::GetFullFilePathForVfsFile(db_tracker_.get(), vfs_file_name);
@@ -163,6 +167,9 @@ void WebDatabaseHostImpl::SetFileSize(const base::string16& vfs_file_name,
                                       int64_t expected_size,
                                       SetFileSizeCallback callback) {
   DCHECK(db_tracker_->task_runner()->RunsTasksInCurrentSequence());
+  if (!ValidateOrigin(vfs_file_name))
+    return;
+
   bool success = false;
   base::FilePath db_file =
       DatabaseUtil::GetFullFilePathForVfsFile(db_tracker_.get(), vfs_file_name);
@@ -177,13 +184,12 @@ void WebDatabaseHostImpl::GetSpaceAvailable(
     GetSpaceAvailableCallback callback) {
   // QuotaManager is only available on the IO thread.
   DCHECK(db_tracker_->task_runner()->RunsTasksInCurrentSequence());
-  DCHECK(db_tracker_->quota_manager_proxy());
 
   if (!ValidateOrigin(origin)) {
-    std::move(callback).Run(0);
     return;
   }
 
+  DCHECK(db_tracker_->quota_manager_proxy());
   db_tracker_->quota_manager_proxy()->GetUsageAndQuota(
       db_tracker_->task_runner(), origin, blink::mojom::StorageType::kTemporary,
       base::BindOnce(
@@ -367,4 +373,37 @@ blink::mojom::WebDatabase& WebDatabaseHostImpl::GetWebDatabase() {
   return *database_provider_;
 }
 
+bool WebDatabaseHostImpl::ValidateOrigin(const url::Origin& origin) {
+  if (origin.unique()) {
+    mojo::ReportBadMessage("Invalid origin.");
+    return false;
+  }
+
+  if (!ChildProcessSecurityPolicyImpl::GetInstance()->CanAccessDataForOrigin(
+          process_id_, origin.GetURL())) {
+    mojo::ReportBadMessage("Unauthorized origin.");
+    return false;
+  }
+  return true;
+}
+
+bool WebDatabaseHostImpl::ValidateOrigin(const base::string16& vfs_file_name) {
+  std::string origin_identifier;
+  if (vfs_file_name.empty())
+    return true;
+
+  if (!DatabaseUtil::CrackVfsFileName(vfs_file_name, &origin_identifier,
+                                      nullptr, nullptr)) {
+    return true;
+  }
+
+  if (!ChildProcessSecurityPolicyImpl::GetInstance()->CanAccessDataForOrigin(
+          process_id_,
+          storage::GetOriginURLFromIdentifier(origin_identifier))) {
+    mojo::ReportBadMessage("Unauthorized origin.");
+    return false;
+  }
+  return true;
+}
+
 }  // namespace content

content/browser/renderer_host/web_database_host_impl.h

@@ -8,6 +8,7 @@
 #include <string>
 
 #include "base/strings/string16.h"
+#include "build/build_config.h"
 #include "mojo/public/cpp/bindings/binding.h"
 #include "storage/browser/database/database_tracker.h"
 #include "third_party/blink/public/platform/modules/webdatabase/web_database.mojom.h"
@@ -18,10 +19,11 @@ class Origin;
 
 namespace content {
 
-class WebDatabaseHostImpl : public blink::mojom::WebDatabaseHost,
-                            public storage::DatabaseTracker::Observer {
+class CONTENT_EXPORT WebDatabaseHostImpl
+    : public blink::mojom::WebDatabaseHost,
+      public storage::DatabaseTracker::Observer {
  public:
-  WebDatabaseHostImpl(int proess_id,
+  WebDatabaseHostImpl(int process_id,
                       scoped_refptr<storage::DatabaseTracker> db_tracker);
   ~WebDatabaseHostImpl() override;
 
@@ -30,6 +32,9 @@ class WebDatabaseHostImpl : public blink::mojom::WebDatabaseHost,
                      blink::mojom::WebDatabaseHostRequest request);
 
  private:
+  FRIEND_TEST_ALL_PREFIXES(WebDatabaseHostImplTest, BadMessagesUnauthorized);
+  FRIEND_TEST_ALL_PREFIXES(WebDatabaseHostImplTest, BadMessagesInvalid);
+
   // blink::mojom::WebDatabaseHost:
   void OpenFile(const base::string16& vfs_file_name,
                 int32_t desired_flags,
@@ -85,6 +90,17 @@ class WebDatabaseHostImpl : public blink::mojom::WebDatabaseHost,
   // exist.
   blink::mojom::WebDatabase& GetWebDatabase();
 
+  // Check that an IPC call has permission to access the passed origin. Must
+  // be called from within the context of a mojo call. Invalid calls will
+  // report a bad message, which will terminate the calling process. If this
+  // returns false, the caller should return immediately without invoking
+  // callbacks.
+  bool ValidateOrigin(const url::Origin& origin);
+
+  // As above, but for calls where the origin is embedded in a VFS filename.
+  // Empty filenames signalling a temp file are permitted.
+  bool ValidateOrigin(const base::string16& vfs_file_name);
+
   // Our render process host ID, used to bind to the correct render process.
   const int process_id_;
 

content/browser/renderer_host/web_database_host_impl_unittest.cc

+// Copyright 2018 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 "content/browser/renderer_host/web_database_host_impl.h"
+
+#include "base/strings/utf_string_conversions.h"
+#include "content/browser/child_process_security_policy_impl.h"
+#include "content/public/test/test_browser_thread_bundle.h"
+#include "mojo/core/embedder/embedder.h"
+#include "storage/common/database/database_identifier.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace content {
+
+namespace {
+base::string16 ConstructVfsFileName(const url::Origin& origin,
+                                    const base::string16& name,
+                                    const base::string16& suffix) {
+  std::string identifier = storage::GetIdentifierFromOrigin(origin);
+  return base::UTF8ToUTF16(identifier) + base::ASCIIToUTF16("/") + name +
+         base::ASCIIToUTF16("#") + suffix;
+}
+
+class BadMessageObserver {
+ public:
+  BadMessageObserver()
+      : dummy_message_(0, 0, 0, 0, nullptr), context_(&dummy_message_) {
+    mojo::core::SetDefaultProcessErrorCallback(base::BindRepeating(
+        &BadMessageObserver::ReportBadMessage, base::Unretained(this)));
+  }
+
+  ~BadMessageObserver() {
+    mojo::core::SetDefaultProcessErrorCallback(
+        mojo::core::ProcessErrorCallback());
+  }
+
+  const std::string& last_error() const { return last_error_; }
+
+ private:
+  void ReportBadMessage(const std::string& error) { last_error_ = error; }
+
+  mojo::Message dummy_message_;
+  mojo::internal::MessageDispatchContext context_;
+  std::string last_error_;
+
+  DISALLOW_COPY_AND_ASSIGN(BadMessageObserver);
+};
+}  // namespace
+
+class WebDatabaseHostImplTest : public ::testing::Test {
+ protected:
+  WebDatabaseHostImplTest() = default;
+  ~WebDatabaseHostImplTest() override = default;
+
+  void SetUp() override {
+    scoped_refptr<storage::DatabaseTracker> db_tracker =
+        base::MakeRefCounted<storage::DatabaseTracker>(
+            base::FilePath(), /*is_incognito=*/false,
+            /*special_storage_policy=*/nullptr,
+            /*quota_manager_proxy=*/nullptr);
+    db_tracker->set_task_runner_for_testing(
+        base::ThreadTaskRunnerHandle::Get());
+
+    host_ = std::make_unique<WebDatabaseHostImpl>(process_id(),
+                                                  std::move(db_tracker));
+  }
+
+  template <typename Callable>
+  void CheckUnauthorizedOrigin(const Callable& func) {
+    BadMessageObserver bad_message_observer;
+    func();
+    EXPECT_EQ("Unauthorized origin.", bad_message_observer.last_error());
+  }
+
+  template <typename Callable>
+  void CheckInvalidOrigin(const Callable& func) {
+    BadMessageObserver bad_message_observer;
+    func();
+    EXPECT_EQ("Invalid origin.", bad_message_observer.last_error());
+  }
+
+  WebDatabaseHostImpl* host() { return host_.get(); }
+  int process_id() { return kProcessId; }
+
+ private:
+  static constexpr int kProcessId = 1234;
+  TestBrowserThreadBundle thread_bundle_;
+  std::unique_ptr<WebDatabaseHostImpl> host_;
+
+  DISALLOW_COPY_AND_ASSIGN(WebDatabaseHostImplTest);
+};
+
+TEST_F(WebDatabaseHostImplTest, BadMessagesUnauthorized) {
+  const url::Origin correct_origin =
+      url::Origin::Create(GURL("http://correct.com"));
+  const url::Origin incorrect_origin =
+      url::Origin::Create(GURL("http://incorrect.net"));
+  const base::string16 db_name(base::ASCIIToUTF16("db_name"));
+  const base::string16 suffix(base::ASCIIToUTF16("suffix"));
+  const base::string16 bad_vfs_file_name =
+      ConstructVfsFileName(incorrect_origin, db_name, suffix);
+
+  auto* security_policy = ChildProcessSecurityPolicyImpl::GetInstance();
+  security_policy->Add(process_id());
+  security_policy->AddIsolatedOrigins({correct_origin, incorrect_origin});
+  security_policy->LockToOrigin(process_id(), correct_origin.GetURL());
+  ASSERT_TRUE(security_policy->CanAccessDataForOrigin(process_id(),
+                                                      correct_origin.GetURL()));
+  ASSERT_FALSE(security_policy->CanAccessDataForOrigin(
+      process_id(), incorrect_origin.GetURL()));
+
+  CheckUnauthorizedOrigin([&]() {
+    host()->OpenFile(bad_vfs_file_name,
+                     /*desired_flags=*/0, base::BindOnce([](base::File) {}));
+  });
+
+  CheckUnauthorizedOrigin([&]() {
+    host()->DeleteFile(bad_vfs_file_name,
+                       /*sync_dir=*/false, base::BindOnce([](int32_t) {}));
+  });
+
+  CheckUnauthorizedOrigin([&]() {
+    host()->GetFileAttributes(bad_vfs_file_name,
+                              base::BindOnce([](int32_t) {}));
+  });
+
+  CheckUnauthorizedOrigin([&]() {
+    host()->GetFileSize(bad_vfs_file_name, base::BindOnce([](int64_t) {}));
+  });
+
+  CheckUnauthorizedOrigin([&]() {
+    host()->SetFileSize(bad_vfs_file_name, /*expected_size=*/0,
+                        base::BindOnce([](bool) {}));
+  });
+
+  CheckUnauthorizedOrigin([&]() {
+    host()->GetSpaceAvailable(incorrect_origin, base::BindOnce([](int64_t) {}));
+  });
+
+  CheckUnauthorizedOrigin([&]() {
+    host()->Opened(incorrect_origin, db_name, base::ASCIIToUTF16("description"),
+                   /*estimated_size=*/0);
+  });
+
+  CheckUnauthorizedOrigin(
+      [&]() { host()->Modified(incorrect_origin, db_name); });
+
+  CheckUnauthorizedOrigin([&]() { host()->Closed(incorrect_origin, db_name); });
+
+  CheckUnauthorizedOrigin([&]() {
+    host()->HandleSqliteError(incorrect_origin, db_name, /*error=*/0);
+  });
+}
+
+TEST_F(WebDatabaseHostImplTest, BadMessagesInvalid) {
+  const url::Origin opaque_origin;
+  const base::string16 db_name(base::ASCIIToUTF16("db_name"));
+
+  CheckInvalidOrigin([&]() {
+    host()->GetSpaceAvailable(opaque_origin, base::BindOnce([](int64_t) {}));
+  });
+
+  CheckInvalidOrigin([&]() {
+    host()->Opened(opaque_origin, db_name, base::ASCIIToUTF16("description"),
+                   /*estimated_size=*/0);
+  });
+
+  CheckInvalidOrigin([&]() { host()->Modified(opaque_origin, db_name); });
+
+  CheckInvalidOrigin([&]() { host()->Closed(opaque_origin, db_name); });
+
+  CheckInvalidOrigin([&]() {
+    host()->HandleSqliteError(opaque_origin, db_name, /*error=*/0);
+  });
+}
+
+}  // namespace content

content/test/BUILD.gn

@@ -1533,6 +1533,7 @@ test("content_unittests") {
     "../browser/renderer_host/render_widget_host_view_mac_editcommand_helper_unittest.mm",
     "../browser/renderer_host/render_widget_host_view_mac_unittest.mm",
     "../browser/renderer_host/text_input_client_mac_unittest.mm",
+    "../browser/renderer_host/web_database_host_impl_unittest.cc",
     "../browser/resolve_proxy_msg_helper_unittest.cc",
     "../browser/scheduler/responsiveness/calculator_unittest.cc",
     "../browser/scheduler/responsiveness/watcher_unittest.cc",

storage/browser/database/database_tracker.h

@@ -179,6 +179,12 @@ class STORAGE_EXPORT DatabaseTracker
 
   base::SequencedTaskRunner* task_runner() const { return task_runner_.get(); }
 
+  // TODO(jsbell): Remove this; tests should use the normal task runner.
+  void set_task_runner_for_testing(
+      scoped_refptr<base::SequencedTaskRunner> task_runner) {
+    task_runner_ = std::move(task_runner);
+  }
+
  private:
   friend class base::RefCountedThreadSafe<DatabaseTracker>;
   friend class content::DatabaseTracker_TestHelper_Test;
@@ -272,12 +278,6 @@ class STORAGE_EXPORT DatabaseTracker
   // Returns the directory where all DB files for the given origin are stored.
   base::string16 GetOriginDirectory(const std::string& origin_identifier);
 
-  // TODO(jsbell): Remove this; tests should use the normal task runner.
-  void set_task_runner_for_testing(
-      scoped_refptr<base::SequencedTaskRunner> task_runner) {
-    task_runner_ = std::move(task_runner);
-  }
-
   bool is_initialized_ = false;
   const bool is_incognito_;
   bool force_keep_session_state_ = false;