Bug 1275612 - Don't allow any origins to send objects over WebChannel. r=Gijs

amccreight · amccreight · commit 04a47c08504e · 2024-09-08T15:28:50.000Z
The last actual Firefox user of this less-safe feature was removed in 2022. Thunderbird's sync server still needs it, but apparently that is a prototype that isn't really working, so they said it was okay to remove this. Differential Revision: https://phabricator.services.mozilla.com/D220646
diff --git a/browser/app/profile/firefox.js b/browser/app/profile/firefox.js
@@ -2528,10 +2528,6 @@ pref("signon.showAutoCompleteFooter", true);
 pref("signon.showAutoCompleteImport", "import");
 pref("signon.suggestImportCount", 3);
 
-// Space separated list of URLS that are allowed to send objects (instead of
-// only strings) through webchannels. Bug 1275612 tracks removing this pref and capability.
-pref("webchannel.allowObject.urlWhitelist", "https://content.cdn.mozilla.net https://install.mozilla.org");
-
 // Whether or not the browser should scan for unsubmitted
 // crash reports, and then show a notification for submitting
 // those reports.
diff --git a/browser/base/content/test/general/browser_remoteTroubleshoot.js b/browser/base/content/test/general/browser_remoteTroubleshoot.js
@@ -13,9 +13,6 @@ const TEST_URL_TAIL =
   "example.com/browser/browser/base/content/test/general/test_remoteTroubleshoot.html";
 const TEST_URI_GOOD = Services.io.newURI("https://" + TEST_URL_TAIL);
 const TEST_URI_BAD = Services.io.newURI("http://" + TEST_URL_TAIL);
-const TEST_URI_GOOD_OBJECT = Services.io.newURI(
-  "https://" + TEST_URL_TAIL + "?object"
-);
 
 // Creates a one-shot web-channel for the test data to be sent back from the test page.
 function promiseChannelResponse(channelID, originOrPermission) {
@@ -116,15 +113,4 @@ add_task(async function () {
     got.message.errno === 2,
     "should have failed with errno 2, no such channel"
   );
-
-  // Check that the page can send an object as well if it's in the whitelist
-  let webchannelWhitelistPref = "webchannel.allowObject.urlWhitelist";
-  let origWhitelist = Services.prefs.getCharPref(webchannelWhitelistPref);
-  let newWhitelist = origWhitelist + " https://example.com";
-  Services.prefs.setCharPref(webchannelWhitelistPref, newWhitelist);
-  registerCleanupFunction(() => {
-    Services.prefs.clearUserPref(webchannelWhitelistPref);
-  });
-  got = await promiseNewChannelResponse(TEST_URI_GOOD_OBJECT);
-  Assert.ok(got.message, "should have gotten some data back");
 });
diff --git a/browser/base/content/test/general/test_remoteTroubleshoot.html b/browser/base/content/test/general/test_remoteTroubleshoot.html
@@ -1,21 +1,12 @@
 <!DOCTYPE HTML>
 <html>
 <script>
-// This test is run multiple times, once with only strings allowed through the
-// WebChannel, and once with objects allowed. This function allows us to handle
-// both cases without too much pain.
-function makeDetails(object) {
-  if (window.location.search.includes("object")) {
-    return object;
-  }
-  return JSON.stringify(object);
-}
 // Add a listener for responses to our remote requests.
 window.addEventListener("WebChannelMessageToContent", function(event) {
   if (event.detail.id == "remote-troubleshooting") {
     // Send what we got back to the test.
     var backEvent = new window.CustomEvent("WebChannelMessageToChrome", {
-      detail: makeDetails({
+      detail: JSON.stringify({
         id: "test-remote-troubleshooting-backchannel",
         message: {
           message: event.detail.message,
@@ -32,7 +23,7 @@
 // Make a request for the troubleshooting data as we load.
 window.onload = function() {
   var event = new window.CustomEvent("WebChannelMessageToChrome", {
-    detail: makeDetails({
+    detail: JSON.stringify({
       id: "remote-troubleshooting",
       message: {
         command: "request",
diff --git a/toolkit/actors/WebChannelChild.sys.mjs b/toolkit/actors/WebChannelChild.sys.mjs
@@ -3,27 +3,8 @@
  * 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/. */
 
-import { XPCOMUtils } from "resource://gre/modules/XPCOMUtils.sys.mjs";
-
 import { ContentDOMReference } from "resource://gre/modules/ContentDOMReference.sys.mjs";
 
-// Preference containing the list (space separated) of origins that are
-// allowed to send non-string values through a WebChannel, mainly for
-// backwards compatability. See bug 1238128 for more information.
-const URL_WHITELIST_PREF = "webchannel.allowObject.urlWhitelist";
-
-let _cachedWhitelist = null;
-
-const CACHED_PREFS = {};
-XPCOMUtils.defineLazyPreferenceGetter(
-  CACHED_PREFS,
-  "URL_WHITELIST",
-  URL_WHITELIST_PREF,
-  "",
-  // Null this out so we update it.
-  () => (_cachedWhitelist = null)
-);
-
 export class WebChannelChild extends JSWindowActorChild {
   handleEvent(event) {
     if (event.type === "WebChannelMessageToChrome") {
@@ -39,16 +20,6 @@ export class WebChannelChild extends JSWindowActorChild {
     return undefined;
   }
 
-  _getWhitelistedPrincipals() {
-    if (!_cachedWhitelist) {
-      let urls = CACHED_PREFS.URL_WHITELIST.split(/\s+/);
-      _cachedWhitelist = urls.map(origin =>
-        Services.scriptSecurityManager.createContentPrincipalFromOrigin(origin)
-      );
-    }
-    return _cachedWhitelist;
-  }
-
   _onMessageToChrome(e) {
     // If target is window then we want the document principal, otherwise fallback to target itself.
     let principal = e.target.nodePrincipal
@@ -57,19 +28,8 @@ export class WebChannelChild extends JSWindowActorChild {
 
     if (e.detail) {
       if (typeof e.detail != "string") {
-        // Check if the principal is one of the ones that's allowed to send
-        // non-string values for e.detail.  They're whitelisted by site origin,
-        // so we compare on originNoSuffix in order to avoid other origin attributes
-        // that are not relevant here, such as containers or private browsing.
-        let objectsAllowed = this._getWhitelistedPrincipals().some(
-          whitelisted => principal.originNoSuffix == whitelisted.originNoSuffix
-        );
-        if (!objectsAllowed) {
-          console.error(
-            "WebChannelMessageToChrome sent with an object from a non-whitelisted principal"
-          );
-          return;
-        }
+        console.error("WebChannelMessageToChrome must only send strings");
+        return;
       }
 
       let eventTarget =
diff --git a/toolkit/modules/tests/browser/browser_web_channel.js b/toolkit/modules/tests/browser/browser_web_channel.js
@@ -425,14 +425,13 @@ var gTests = [
     },
   },
   {
-    desc: "WebChannel disallows non-string message from non-whitelisted origin",
+    desc: "WebChannel disallows non-string messages",
     async run() {
       /**
        * This test ensures that non-string messages can't be sent via WebChannels.
-       * We create a page (on a non-whitelisted origin) which should send us two
-       * messages immediately. The first message has an object for it's detail,
-       * and the second has a string. We check that we only get the second
-       * message.
+       * We create a page which should send us two messages immediately. The first
+       * message has an object for its detail, and the second has a string. We
+       * check that we only get the second message.
        */
       let channel = new WebChannel("objects", Services.io.newURI(HTTP_PATH));
       let testDonePromise = new Promise(resolve => {
@@ -454,51 +453,6 @@ var gTests = [
       );
     },
   },
-  {
-    desc: "WebChannel allows both string and non-string message from whitelisted origin",
-    async run() {
-      /**
-       * Same process as above, but we whitelist the origin before loading the page,
-       * and expect to get *both* messages back (each exactly once).
-       */
-      let channel = new WebChannel("objects", Services.io.newURI(HTTP_PATH));
-
-      let testDonePromise = new Promise((resolve, reject) => {
-        let sawObject = false;
-        let sawString = false;
-        channel.listen((id, message) => {
-          is(id, "objects");
-          if (message.type === "object") {
-            ok(!sawObject);
-            sawObject = true;
-          } else if (message.type === "string") {
-            ok(!sawString);
-            sawString = true;
-          } else {
-            reject(new Error(`Unknown message type: ${message.type}`));
-          }
-          if (sawObject && sawString) {
-            resolve();
-          }
-        });
-      });
-      const webchannelWhitelistPref = "webchannel.allowObject.urlWhitelist";
-      let origWhitelist = Services.prefs.getCharPref(webchannelWhitelistPref);
-      let newWhitelist = origWhitelist + " " + HTTP_PATH;
-      Services.prefs.setCharPref(webchannelWhitelistPref, newWhitelist);
-      await BrowserTestUtils.withNewTab(
-        {
-          gBrowser,
-          url: HTTP_PATH + HTTP_ENDPOINT + "?object",
-        },
-        async function () {
-          await testDonePromise;
-          Services.prefs.setCharPref(webchannelWhitelistPref, origWhitelist);
-          channel.stopListening();
-        }
-      );
-    },
-  },
   {
     desc: "WebChannel errors handling the message are delivered back to content",
     async run() {
diff --git a/tools/lint/rejected-words.yml b/tools/lint/rejected-words.yml
@@ -35,7 +35,6 @@ avoid-blacklist-and-whitelist:
         - browser/app/winlauncher/LauncherProcessWin.cpp
         - browser/base/content/browser.js
         - browser/base/content/contentTheme.js
-        - browser/base/content/test/general/browser_remoteTroubleshoot.js
         - browser/base/content/test/general/browser_tab_drag_drop_perwindow.js
         - browser/base/content/test/performance/browser_preferences_usage.js
         - browser/base/content/test/protectionsUI/browser_protectionsUI_cryptominers.js
@@ -233,7 +232,6 @@ avoid-blacklist-and-whitelist:
         - testing/raptor/browsertime/browsertime_scenario.js
         - testing/web-platform/tests/tools/manifest/tests/test_manifest.py
         - toolkit/actors/RemotePageChild.sys.mjs
-        - toolkit/actors/WebChannelChild.sys.mjs
         - toolkit/components/antitracking/docs/tracking-lists/index.md
         - toolkit/components/antitracking/test/browser/browser_socialtracking_save_image.js
         - toolkit/components/reputationservice/ApplicationReputation.cpp
@@ -269,7 +267,6 @@ avoid-blacklist-and-whitelist:
         - toolkit/content/tests/browser/browser_delay_autoplay_webAudio.js
         - toolkit/modules/PermissionsUtils.sys.mjs
         - toolkit/modules/tests/browser/browser_AsyncPrefs.js
-        - toolkit/modules/tests/browser/browser_web_channel.js
         - toolkit/modules/tests/xpcshell/test_PermissionsUtils.js
         - toolkit/modules/third_party/jsesc/jsesc.mjs
         - toolkit/modules/Troubleshoot.sys.mjs
