Only allow standard URLs for homepage policy

This CL adds a policy handler for the HomepageLocation policy that
filters out invalid URLs and URLs with a non-standard scheme. It used
to be possible to set Javascript as homepage, which would then execute
in the context of the current page.

This filter applies to the policy only. The user can still use
Javascript in the homepage settings if they want to.

BUG=b:112186030
TEST=Set Javascript homepage through policy, made sure it's rejected in
      chrome://policy and policy is unset in settings; unit tests.

Change-Id: I991b2de55b7edaf67413b7695da7c566e8369600
Reviewed-on: https://chromium-review.googlesource.com/c/1186643
Commit-Queue: Lutz Justen <ljusten@chromium.org>
Reviewed-by: Sergey Poromov <poromov@chromium.org>
Cr-Commit-Position: refs/heads/master@{#613049}

chrome/browser/BUILD.gn

@@ -1101,6 +1101,8 @@ jumbo_split_static_library("browser") {
     "policy/device_management_service_configuration.h",
     "policy/file_selection_dialogs_policy_handler.cc",
     "policy/file_selection_dialogs_policy_handler.h",
+    "policy/homepage_location_policy_handler.cc",
+    "policy/homepage_location_policy_handler.h",
     "policy/javascript_policy_handler.cc",
     "policy/javascript_policy_handler.h",
     "policy/managed_bookmarks_policy_handler.cc",

chrome/browser/policy/configuration_policy_handler_list_factory.cc

@@ -19,6 +19,7 @@
 #include "chrome/browser/policy/browsing_history_policy_handler.h"
 #include "chrome/browser/policy/developer_tools_policy_handler.h"
 #include "chrome/browser/policy/file_selection_dialogs_policy_handler.h"
+#include "chrome/browser/policy/homepage_location_policy_handler.h"
 #include "chrome/browser/policy/javascript_policy_handler.h"
 #include "chrome/browser/policy/managed_bookmarks_policy_handler.h"
 #include "chrome/browser/policy/network_prediction_policy_handler.h"
@@ -137,9 +138,6 @@ namespace {
 // that directly map to a single preference.
 // clang-format off
 const PolicyToPreferenceMapEntry kSimplePolicyMap[] = {
-  { key::kHomepageLocation,
-    prefs::kHomePage,
-    base::Value::Type::STRING },
   { key::kHomepageIsNewTabPage,
     prefs::kHomePageIsNewTabPage,
     base::Value::Type::BOOLEAN },
@@ -1015,6 +1013,7 @@ std::unique_ptr<ConfigurationPolicyHandlerList> BuildHandlerList(
   handlers->AddHandler(std::make_unique<GuestModePolicyHandler>());
   handlers->AddHandler(
       std::make_unique<ManagedBookmarksPolicyHandler>(chrome_schema));
+  handlers->AddHandler(std::make_unique<HomepageLocationPolicyHandler>());
   handlers->AddHandler(std::make_unique<ProxyPolicyHandler>());
   handlers->AddHandler(std::make_unique<URLBlacklistPolicyHandler>());
 

chrome/browser/policy/homepage_location_policy_handler.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 "chrome/browser/policy/homepage_location_policy_handler.h"
+
+#include <memory>
+
+#include "base/values.h"
+#include "chrome/common/pref_names.h"
+#include "components/policy/core/browser/policy_error_map.h"
+#include "components/policy/core/common/policy_map.h"
+#include "components/policy/policy_constants.h"
+#include "components/prefs/pref_value_map.h"
+#include "components/strings/grit/components_strings.h"
+#include "url/gurl.h"
+
+namespace policy {
+
+HomepageLocationPolicyHandler::HomepageLocationPolicyHandler()
+    : TypeCheckingPolicyHandler(key::kHomepageLocation,
+                                base::Value::Type::STRING) {}
+
+HomepageLocationPolicyHandler::~HomepageLocationPolicyHandler() = default;
+
+bool HomepageLocationPolicyHandler::CheckPolicySettings(
+    const PolicyMap& policies,
+    PolicyErrorMap* errors) {
+  const base::Value* value = nullptr;
+  if (!CheckAndGetValue(policies, errors, &value))
+    return false;
+  if (!value)
+    return true;
+
+  // Check whether the URL is a standard scheme to prevent e.g. Javascript.
+  GURL homepage_url(value->GetString());
+  if (!homepage_url.is_valid() || !homepage_url.IsStandard()) {
+    errors->AddError(policy_name(), IDS_POLICY_HOMEPAGE_LOCATION_ERROR);
+    return false;
+  }
+
+  return true;
+}
+
+void HomepageLocationPolicyHandler::ApplyPolicySettings(
+    const PolicyMap& policies,
+    PrefValueMap* prefs) {
+  const base::Value* policy_value = nullptr;
+  if (CheckAndGetValue(policies, nullptr, &policy_value) && policy_value)
+    prefs->SetString(prefs::kHomePage, policy_value->GetString());
+}
+
+}  // namespace policy

chrome/browser/policy/homepage_location_policy_handler.h

+// 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.
+
+#ifndef CHROME_BROWSER_POLICY_HOMEPAGE_LOCATION_POLICY_HANDLER_H_
+#define CHROME_BROWSER_POLICY_HOMEPAGE_LOCATION_POLICY_HANDLER_H_
+
+#include "base/macros.h"
+#include "components/policy/core/browser/configuration_policy_handler.h"
+
+class PrefValueMap;
+
+namespace policy {
+
+class PolicyErrorMap;
+class PolicyMap;
+
+// Handles the |kHomepageLocation| policy and sets the |kHomepage| pref. Makes
+// sure that the URL is valid and uses a standard scheme (i.e. no Javascript
+// etc.).
+class HomepageLocationPolicyHandler : public TypeCheckingPolicyHandler {
+ public:
+  HomepageLocationPolicyHandler();
+  ~HomepageLocationPolicyHandler() override;
+
+  // ConfigurationPolicyHandler:
+  bool CheckPolicySettings(const PolicyMap& policies,
+                           PolicyErrorMap* errors) override;
+
+  void ApplyPolicySettings(const PolicyMap& policies,
+                           PrefValueMap* prefs) override;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(HomepageLocationPolicyHandler);
+};
+
+}  // namespace policy
+
+#endif  // CHROME_BROWSER_POLICY_HOMEPAGE_LOCATION_POLICY_HANDLER_H_

chrome/browser/policy/homepage_location_policy_handler_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 "chrome/browser/policy/homepage_location_policy_handler.h"
+
+#include <memory>
+
+#include "base/values.h"
+#include "chrome/common/pref_names.h"
+#include "components/policy/core/browser/policy_error_map.h"
+#include "components/policy/core/common/policy_map.h"
+#include "components/policy/core/common/policy_types.h"
+#include "components/policy/policy_constants.h"
+#include "components/prefs/pref_value_map.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace policy {
+
+namespace {
+
+constexpr char kHttpUrl[] = "http://example.com";
+constexpr char kHttpsUrl[] = "https://example.com";
+constexpr char kFileUrl[] = "file:///wohnzimmertapete.html";
+constexpr char kInvalidSchemeUrl[] = "xss://crazy_hack.js";
+constexpr char kInvalidUrl[] = "example.com";
+constexpr char kJavascript[] =
+    "(function()%7Bvar%20script%20%3D%20document.createElement(%22script%22)%"
+    "3Bscript.type%3D%22text%2Fjavascript%22%3Bscript.src%3D%22https%3A%2F%"
+    "2Fwww.example.com%22%3Bdocument.head.appendChild(script)%3B%7D())%3B";
+
+}  // namespace
+
+class HomepageLocationPolicyHandlerTest : public testing::Test {
+ protected:
+  void SetPolicy(std::unique_ptr<base::Value> value) {
+    policies_.Set(key::kHomepageLocation, POLICY_LEVEL_MANDATORY,
+                  POLICY_SCOPE_USER, POLICY_SOURCE_CLOUD, std::move(value),
+                  nullptr);
+  }
+
+  bool CheckPolicy(std::unique_ptr<base::Value> value) {
+    SetPolicy(std::move(value));
+    return handler_.CheckPolicySettings(policies_, &errors_);
+  }
+
+  void ApplyPolicies() { handler_.ApplyPolicySettings(policies_, &prefs_); }
+
+  HomepageLocationPolicyHandler handler_;
+  PolicyErrorMap errors_;
+  PolicyMap policies_;
+  PrefValueMap prefs_;
+};
+
+TEST_F(HomepageLocationPolicyHandlerTest, NoPolicyDoesntExplode) {
+  EXPECT_TRUE(handler_.CheckPolicySettings(policies_, &errors_));
+  EXPECT_EQ(0U, errors_.size());
+}
+
+TEST_F(HomepageLocationPolicyHandlerTest, StandardSchemesAreAccepted) {
+  EXPECT_TRUE(CheckPolicy(std::make_unique<base::Value>(kHttpUrl)));
+  EXPECT_TRUE(CheckPolicy(std::make_unique<base::Value>(kHttpsUrl)));
+  EXPECT_TRUE(CheckPolicy(std::make_unique<base::Value>(kFileUrl)));
+  EXPECT_EQ(0U, errors_.size());
+}
+
+TEST_F(HomepageLocationPolicyHandlerTest, InvalidUrlIsRejected) {
+  EXPECT_FALSE(CheckPolicy(std::make_unique<base::Value>(kInvalidUrl)));
+  EXPECT_EQ(1U, errors_.size());
+}
+
+TEST_F(HomepageLocationPolicyHandlerTest, InvalidSchemeIsRejected) {
+  EXPECT_FALSE(CheckPolicy(std::make_unique<base::Value>(kInvalidSchemeUrl)));
+  EXPECT_EQ(1U, errors_.size());
+}
+
+TEST_F(HomepageLocationPolicyHandlerTest, JavascriptIsRejected) {
+  EXPECT_FALSE(CheckPolicy(std::make_unique<base::Value>(kJavascript)));
+  EXPECT_EQ(1U, errors_.size());
+}
+
+TEST_F(HomepageLocationPolicyHandlerTest,
+       ApplyPolicySettings_SomethingSpecified) {
+  SetPolicy(std::make_unique<base::Value>(kHttpUrl));
+  ApplyPolicies();
+  base::Value* value;
+  EXPECT_TRUE(prefs_.GetValue(prefs::kHomePage, &value));
+  ASSERT_TRUE(value->is_string());
+  EXPECT_EQ(value->GetString(), kHttpUrl);
+}
+
+TEST_F(HomepageLocationPolicyHandlerTest,
+       ApplyPolicySettings_NothingSpecified) {
+  ApplyPolicies();
+  EXPECT_FALSE(prefs_.GetValue(prefs::kHomePage, nullptr));
+}
+
+}  // namespace policy

chrome/test/BUILD.gn

@@ -3356,6 +3356,7 @@ test("unit_tests") {
       "../browser/media/webrtc/webrtc_log_uploader_unittest.cc",
       "../browser/media/webrtc/webrtc_rtp_dump_handler_unittest.cc",
       "../browser/media/webrtc/webrtc_rtp_dump_writer_unittest.cc",
+      "../browser/policy/homepage_location_policy_handler_unittest.cc",
       "../browser/policy/local_sync_policy_handler_unittest.cc",
       "../browser/renderer_context_menu/render_view_context_menu_test_util.cc",
       "../browser/renderer_context_menu/render_view_context_menu_test_util.h",

components/policy_strings.grdp

@@ -182,6 +182,9 @@
   <message name="IDS_POLICY_OFF_CWS_URL_ERROR" desc="The text displayed in the status column when the update URL points to a location other than the Chrome Webstore and the machine is not enterprise managed.">
     This computer is not detected as enterprise managed so policy can only automatically install extensions hosted on the Chrome Webstore. The Chrome Webstore update URL is "<ph name="CWS_UPDATE_URL">$1<ex>https://clients2.google.com/service/update2/crx</ex></ph>".
   </message>
+  <message name="IDS_POLICY_HOMEPAGE_LOCATION_ERROR" desc="The text displayed in the status column when the homepage URL given by the HomepageLocation policy is invalid.">
+    Invalid URL. Must be a URL with a standard scheme.
+  </message>
   <message name="IDS_POLICY_PROXY_MODE_DISABLED_ERROR" desc="The text displayed in the status column when use of a proxy is disabled but a proxy configuration is given.">
     Use of a proxy is disabled but an explicit proxy configuration is specified.
   </message>