Bug 1535049 - Disallow setting homepages to ignored urls. r=daleharvey,mixedpuppy

Differential Revision: https://phabricator.services.mozilla.com/D40393

--HG--
rename : browser/modules/test/unit/test_HomePage.js => browser/modules/test/unit/test_HomePage_ignore.js
extra : moz-landing-system : lando

browser/base/content/browser.js

@@ -4204,11 +4204,7 @@ function openHomeDialog(aURL) {
   );
 
   if (pressedVal == 0) {
-    try {
+    HomePage.set(aURL).catch(Cu.reportError);
-      HomePage.set(aURL);
-    } catch (ex) {
-      dump("Failed to set the home page.\n" + ex + "\n");
-    }
   }
 }
 

browser/components/BrowserGlue.jsm

@@ -3033,7 +3033,7 @@ BrowserGlue.prototype = {
             HomePage.reset();
           } else {
             value = updated;
-            HomePage.set(value);
+            HomePage.safeSet(value);
           }
         }
       }

browser/components/extensions/parent/ext-chrome-settings-overrides.js

@@ -21,6 +21,11 @@ ChromeUtils.defineModuleGetter(
   "ExtensionControlledPopup",
   "resource:///modules/ExtensionControlledPopup.jsm"
 );
+ChromeUtils.defineModuleGetter(
+  this,
+  "HomePage",
+  "resource:///modules/HomePage.jsm"
+);
 
 const DEFAULT_SEARCH_STORE_TYPE = "default_search";
 const DEFAULT_SEARCH_SETTING_NAME = "defaultSearch";
@@ -226,8 +231,11 @@ this.chrome_settings_overrides = class extends ExtensionAPI {
     await ExtensionSettingsStore.initialize();
 
     let homepageUrl = manifest.chrome_settings_overrides.homepage;
+    const ignoreHomePageUrl =
+      homepageUrl && (await HomePage.shouldIgnore(homepageUrl));
 
-    if (homepageUrl) {
+    // If this is a page we ignore, just skip the homepage setting completely.
+    if (homepageUrl && !ignoreHomePageUrl) {
       let inControl;
       if (
         extension.startupReason == "ADDON_INSTALL" ||

browser/components/extensions/test/browser/browser_ext_chrome_settings_overrides_home.js

@@ -13,6 +13,11 @@ ChromeUtils.defineModuleGetter(
   "ExtensionSettingsStore",
   "resource://gre/modules/ExtensionSettingsStore.jsm"
 );
+ChromeUtils.defineModuleGetter(
+  this,
+  "HomePage",
+  "resource:///modules/HomePage.jsm"
+);
 
 // Named this way so they correspond to the extensions
 const HOME_URI_2 = "http://example.com/";
@@ -24,6 +29,8 @@ const CONTROLLED_BY_OTHER = "controlled_by_other_extensions";
 const NOT_CONTROLLABLE = "not_controllable";
 
 const HOMEPAGE_URL_PREF = "browser.startup.homepage";
+const HOMEPAGE_EXTENSION_CONTROLLED =
+  "browser.startup.homepage_override.extensionControlled";
 
 const getHomePageURL = () => {
   return Services.prefs.getStringPref(HOMEPAGE_URL_PREF);
@@ -610,3 +617,31 @@ add_task(async function test_overriding_home_page_incognito_external() {
   await extension.unload();
   await BrowserTestUtils.closeWindow(win);
 });
+
+add_task(async function test_overriding_with_ignored_url() {
+  // Manually poke into the ignore list a value to be ignored.
+  HomePage._ignoreList.push("ignore=me");
+  Services.prefs.setBoolPref(HOMEPAGE_EXTENSION_CONTROLLED, false);
+
+  let extension = ExtensionTestUtils.loadExtension({
+    manifest: {
+      chrome_settings_overrides: { homepage: "https://example.com/?ignore=me" },
+      name: "extension",
+    },
+    useAddonManager: "temporary",
+  });
+
+  await extension.startup();
+
+  ok(HomePage.isDefault, "Should still have the default homepage");
+  is(
+    Services.prefs.getBoolPref(
+      "browser.startup.homepage_override.extensionControlled"
+    ),
+    false,
+    "Should not be extension controlled."
+  );
+
+  await extension.unload();
+  HomePage._ignoreList.pop();
+});

browser/components/extensions/test/xpcshell/test_ext_chrome_settings_overrides_update.js

@@ -5,7 +5,11 @@
 const { AddonTestUtils } = ChromeUtils.import(
   "resource://testing-common/AddonTestUtils.jsm"
 );
-const { HomePage } = ChromeUtils.import("resource:///modules/HomePage.jsm");
+XPCOMUtils.defineLazyModuleGetters(this, {
+  HomePage: "resource:///modules/HomePage.jsm",
+  RemoteSettings: "resource://services-settings/remote-settings.js",
+  sinon: "resource://testing-common/Sinon.jsm",
+});
 
 AddonTestUtils.init(this);
 AddonTestUtils.overrideCertDB();
@@ -17,8 +21,20 @@ AddonTestUtils.createAppInfo(
   "42"
 );
 
+async function setupRemoteSettings() {
+  const settings = await RemoteSettings("hijack-blocklists");
+  sinon.stub(settings, "get").returns([
+    {
+      id: "homepage-urls",
+      matches: ["ignore=me"],
+      _status: "synced",
+    },
+  ]);
+}
+
 add_task(async function setup() {
   await AddonTestUtils.promiseStartupManager();
+  await setupRemoteSettings();
 });
 
 add_task(async function test_overrides_update_removal() {

browser/components/preferences/in-content/home.js

@@ -314,7 +314,7 @@ var gHomePane = {
         break;
       case this.HOME_MODE_BLANK:
         if (HomePage.get() !== "about:blank") {
-          HomePage.set("about:blank");
+          HomePage.safeSet("about:blank");
         } else {
           this._renderCustomSettings({ shouldShow: false });
         }
@@ -370,7 +370,7 @@ var gHomePane = {
 
     // FIXME Bug 244192: using dangerous "|" joiner!
     if (tabs.length) {
-      HomePage.set(tabs.map(getTabURI).join("|"));
+      HomePage.set(tabs.map(getTabURI).join("|")).catch(Cu.reportError);
     }
 
     Services.telemetry.scalarAdd("preferences.use_current_page", 1);
@@ -382,7 +382,7 @@ var gHomePane = {
     }
     if (rv.urls && rv.names) {
       // XXX still using dangerous "|" joiner!
-      HomePage.set(rv.urls.join("|"));
+      HomePage.set(rv.urls.join("|")).catch(Cu.reportError);
     }
   },
 
@@ -429,7 +429,7 @@ var gHomePane = {
 
   onCustomHomePageChange(event) {
     const value = event.target.value || HomePage.getDefault();
-    HomePage.set(value);
+    HomePage.set(value).catch(Cu.reportError);
   },
 
   /**

browser/components/preferences/in-content/main.js

@@ -1077,7 +1077,7 @@ var gMainPane = {
     if (value) {
       // We need to restore the blank homepage setting in our other pref
       if (startupPref.value === this.STARTUP_PREF_BLANK) {
-        HomePage.set("about:blank");
+        HomePage.safeSet("about:blank");
       }
       newValue = this.STARTUP_PREF_RESTORE_SESSION;
       let warnOnQuitPref = Preferences.get("browser.sessionstore.warnOnQuit");

browser/components/preferences/in-content/tests/browser_homepages_filter_aboutpreferences.js

@@ -20,13 +20,16 @@ add_task(async function testSetHomepageUseCurrent() {
   useCurrent.click();
 
   is(gBrowser.tabs.length, 3, "Three tabs should be open");
+  await TestUtils.waitForCondition(
+    () => HomePage.get() == "about:blank|about:home"
+  );
   is(
     HomePage.get(),
     "about:blank|about:home",
     "about:blank and about:home should be the only homepages set"
   );
 
-  HomePage.set(oldHomepage);
+  HomePage.safeSet(oldHomepage);
   BrowserTestUtils.removeTab(gBrowser.selectedTab);
   BrowserTestUtils.removeTab(gBrowser.selectedTab);
 });

browser/components/preferences/in-content/tests/browser_homepages_use_bookmark.js

@@ -44,8 +44,10 @@ add_task(async function testSetHomepageFromBookmark() {
   dialog.document.getElementById("bookmarks").selectItems([bm.guid]);
   dialog.document.documentElement.getButton("accept").click();
 
+  await TestUtils.waitForCondition(() => HomePage.get() == TEST_URL1);
+
   Assert.equal(
-    Services.prefs.getCharPref("browser.startup.homepage"),
+    HomePage.get(),
     TEST_URL1,
     "Should have set the homepage to the same as the bookmark."
   );
@@ -74,8 +76,12 @@ add_task(async function testSetHomepageFromTopLevelFolder() {
     .selectItems([PlacesUtils.bookmarks.menuGuid]);
   dialog.document.documentElement.getButton("accept").click();
 
+  await TestUtils.waitForCondition(
+    () => HomePage.get() == `${TEST_URL1}|${TEST_URL2}`
+  );
+
   Assert.equal(
-    Services.prefs.getCharPref("browser.startup.homepage"),
+    HomePage.get(),
     `${TEST_URL1}|${TEST_URL2}`,
     "Should have set the homepage to the same as the bookmark."
   );

browser/modules/HomePage.jsm

@@ -167,7 +167,30 @@ let HomePage = {
    *   The new value to set the preference to. This could be a single url, or a
    *   `|` separated list of URLs.
    */
-  set(value) {
+  async set(value) {
+    await this.init();
+
+    if (await this.shouldIgnore(value)) {
+      Cu.reportError(
+        `Ignoring homepage setting for ${value} as it is on the ignore list.`
+      );
+      return false;
+    }
+    Services.prefs.setStringPref(kPrefName, value);
+    return true;
+  },
+
+  /**
+   * Sets the homepage preference to a new page. This is an synchronous version
+   * that should only be used when we know the source is safe as it bypasses the
+   * ignore list, e.g. when setting directly to about:blank or a value not
+   * supplied externally.
+   *
+   * @param {string} value
+   *   The new value to set the preference to. This could be a single url, or a
+   *   `|` separated list of URLs.
+   */
+  safeSet(value) {
     Services.prefs.setStringPref(kPrefName, value);
   },
 
@@ -186,6 +209,21 @@ let HomePage = {
     Services.prefs.setStringPref(kPrefName, kDefaultHomePage);
   },
 
+  /**
+   * Determines if a url should be ignored according to the ignore list.
+   *
+   * @param {string} url
+   *   A string that is the url or urls to be ignored.
+   * @returns {boolean}
+   *   True if the url should be ignored.
+   */
+  async shouldIgnore(url) {
+    await this.init();
+
+    const lowerURL = url.toLowerCase();
+    return this._ignoreList.some(code => lowerURL.includes(code));
+  },
+
   /**
    * Handles updates of the ignore list, checking the existing preference and
    * correcting it as necessary.
@@ -193,7 +231,7 @@ let HomePage = {
    * @param {Object} eventData
    *   The event data as received from RemoteSettings.
    */
-  _handleIgnoreListUpdated({ data: { current } }) {
+  async _handleIgnoreListUpdated({ data: { current } }) {
     for (const entry of current) {
       if (entry.id == kHomePageIgnoreListId) {
         this._ignoreList = [...entry.matches];

browser/modules/test/unit/test_HomePage.js

@@ -42,7 +42,7 @@ add_task(function test_HomePage() {
     "Homepage should not be overriden by default."
   );
   let newvalue = "about:blank|about:newtab";
-  HomePage.set(newvalue);
+  HomePage.safeSet(newvalue);
   Assert.ok(HomePage.overridden, "Homepage should be overriden after set()");
   Assert.equal(HomePage.get(), newvalue, "Homepage should be ${newvalue}");
   Assert.notEqual(
@@ -89,49 +89,3 @@ add_task(function test_recoverEmptyHomepage() {
   Assert.equal(HomePage.get(), HomePage.getDefault(), "Recover is default");
   Assert.ok(!HomePage.overridden, "Recover should have set default");
 });
-
-add_task(async function test_initWithIgnoredPageCausesReset() {
-  HomePage.set("http://bad/?ignore=me");
-  Assert.ok(HomePage.overridden, "Should have overriden the homepage");
-
-  await HomePage.init();
-
-  Assert.ok(
-    !HomePage.overridden,
-    "Should no longer be overriding the homepage."
-  );
-  Assert.equal(
-    HomePage.get(),
-    HomePage.getDefault(),
-    "Should have reset to the default preference"
-  );
-});
-
-add_task(async function test_updateIgnoreListCausesReset() {
-  HomePage.set("http://bad/?new=ignore");
-  Assert.ok(HomePage.overridden, "Should have overriden the homepage");
-
-  // Simulate an ignore list update.
-  await RemoteSettings("hijack-blocklists").emit("sync", {
-    data: {
-      current: [
-        {
-          id: HOMEPAGE_IGNORELIST,
-          schema: 1553857697843,
-          last_modified: 1553859483588,
-          matches: ["ignore=me", "new=ignore"],
-        },
-      ],
-    },
-  });
-
-  Assert.ok(
-    !HomePage.overridden,
-    "Should no longer be overriding the homepage."
-  );
-  Assert.equal(
-    HomePage.get(),
-    HomePage.getDefault(),
-    "Should have reset to the default preference"
-  );
-});

browser/modules/test/unit/test_HomePage_ignore.js

@@ -0,0 +1,104 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/
+ */
+
+"use strict";
+
+const { XPCOMUtils } = ChromeUtils.import(
+  "resource://gre/modules/XPCOMUtils.jsm"
+);
+
+XPCOMUtils.defineLazyModuleGetters(this, {
+  HomePage: "resource:///modules/HomePage.jsm",
+  Services: "resource://gre/modules/Services.jsm",
+  RemoteSettings: "resource://services-settings/remote-settings.js",
+  sinon: "resource://testing-common/Sinon.jsm",
+});
+
+const HOMEPAGE_IGNORELIST = "homepage-urls";
+
+/**
+ * Provides a basic set of remote settings for use in tests.
+ */
+async function setupRemoteSettings() {
+  const settings = await RemoteSettings("hijack-blocklists");
+  sinon.stub(settings, "get").returns([
+    {
+      id: HOMEPAGE_IGNORELIST,
+      matches: ["ignore=me"],
+      _status: "synced",
+    },
+  ]);
+}
+
+add_task(async function setup() {
+  await setupRemoteSettings();
+});
+
+add_task(async function test_initWithIgnoredPageCausesReset() {
+  // Set the preference direct as the set() would block us.
+  Services.prefs.setStringPref(
+    "browser.startup.homepage",
+    "http://bad/?ignore=me"
+  );
+  Assert.ok(HomePage.overridden, "Should have overriden the homepage");
+
+  await HomePage.init();
+
+  Assert.ok(
+    !HomePage.overridden,
+    "Should no longer be overriding the homepage."
+  );
+  Assert.equal(
+    HomePage.get(),
+    HomePage.getDefault(),
+    "Should have reset to the default preference"
+  );
+});
+
+add_task(async function test_updateIgnoreListCausesReset() {
+  Services.prefs.setStringPref(
+    "browser.startup.homepage",
+    "http://bad/?new=ignore"
+  );
+  Assert.ok(HomePage.overridden, "Should have overriden the homepage");
+
+  // Simulate an ignore list update.
+  await RemoteSettings("hijack-blocklists").emit("sync", {
+    data: {
+      current: [
+        {
+          id: HOMEPAGE_IGNORELIST,
+          schema: 1553857697843,
+          last_modified: 1553859483588,
+          matches: ["ignore=me", "new=ignore"],
+        },
+      ],
+    },
+  });
+
+  Assert.ok(
+    !HomePage.overridden,
+    "Should no longer be overriding the homepage."
+  );
+  Assert.equal(
+    HomePage.get(),
+    HomePage.getDefault(),
+    "Should have reset to the default preference"
+  );
+});
+
+add_task(async function test_setIgnoredUrl() {
+  Assert.ok(!HomePage.overriden, "Should not be overriding the homepage");
+
+  await HomePage.set("http://bad/?ignore=me");
+
+  Assert.equal(
+    HomePage.get(),
+    HomePage.getDefault(),
+    "Should still have the default homepage."
+  );
+  Assert.ok(!HomePage.overriden, "Should not be overriding the homepage.");
+});
+
+// Also need an integration mochitest with an extension (if we can).

browser/modules/test/unit/xpcshell.ini

@@ -5,6 +5,7 @@ skip-if = toolkit == 'android'
 
 [test_E10SUtils_nested_URIs.js]
 [test_HomePage.js]
+[test_HomePage_ignore.js]
 [test_LiveBookmarkMigrator.js]
 [test_Sanitizer_interrupted.js]
 [test_SitePermissions.js]