Bug 1759786 - Send tab should not send or receive file:// uris r=markh

Differential Revision: https://phabricator.services.mozilla.com/D141174
dothq · Mar 30, 2022 · 9ff273b · 9ff273b
1 parent 7d206f2
commit 9ff273b
Show file tree

Hide file tree

Showing 4 changed files with 102 additions and 12 deletions.
diff --git a/services/fxaccounts/FxAccountsCommands.js b/services/fxaccounts/FxAccountsCommands.js
@@ -30,6 +30,17 @@ XPCOMUtils.defineLazyModuleGetters(this, {
   CryptoWrapper: "resource://services-sync/record.js",
 });
 
+XPCOMUtils.defineLazyPreferenceGetter(
+  this,
+  "INVALID_SHAREABLE_SCHEMES",
+  "services.sync.engine.tabs.filteredSchemes",
+  "",
+  null,
+  val => {
+    return new Set(val.split("|"));
+  }
+);
+
 class FxAccountsCommands {
   constructor(fxAccountsInternal) {
     this._fxai = fxAccountsInternal;
@@ -199,6 +210,13 @@ class FxAccountsCommands {
                 sender ? sender.name : "Unknown device"
               }.`
             );
+            // This should eventually be rare to hit as all platforms will be using the same
+            // scheme filter list, but we have this here in the case other platforms
+            // haven't caught up and/or trying to send invalid uris using older versions
+            const scheme = Services.io.newURI(uri).scheme;
+            if (INVALID_SHAREABLE_SCHEMES.has(scheme)) {
+              throw new Error("Invalid scheme found for received URI.");
+            }
             tabsReceived.push({ title, uri, sender });
           } catch (e) {
             log.error(`Error while handling incoming Send Tab payload.`, e);
@@ -209,9 +227,13 @@ class FxAccountsCommands {
       }
     }
     if (tabsReceived.length) {
-      Observers.notify("fxaccounts:commands:open-uri", tabsReceived);
+      this._notifyFxATabsReceived(tabsReceived);
     }
   }
+
+  _notifyFxATabsReceived(tabsReceived) {
+    Observers.notify("fxaccounts:commands:open-uri", tabsReceived);
+  }
 }
 
 /**

diff --git a/services/fxaccounts/tests/xpcshell/test_commands.js b/services/fxaccounts/tests/xpcshell/test_commands.js
@@ -392,7 +392,7 @@ add_task(async function test_commands_handleCommands() {
   commands.sendTab.handle = (sender, data, reason) => {
     return {
       title: "testTitle",
-      uri: "testURI",
+      uri: "https://testURI",
     };
   };
   commands._fxai.device = {
@@ -408,6 +408,57 @@ add_task(async function test_commands_handleCommands() {
     .expects("_getReason")
     .once()
     .withExactArgs(pushIndexReceived, remoteMessageIndex);
+  mockCommands.expects("_notifyFxATabsReceived").once();
+  await commands._handleCommands(remoteMessages, pushIndexReceived);
+  mockCommands.verify();
+});
+
+add_task(async function test_commands_handleCommands_invalid_tab() {
+  // This test ensures that `_getReason` is being called by
+  // `_handleCommands` with the expected parameters.
+  const pushIndexReceived = 12;
+  const senderID = "6d09f6c4-89b2-41b3-a0ac-e4c2502b5485";
+  const remoteMessageIndex = 8;
+  const remoteMessages = [
+    {
+      index: remoteMessageIndex,
+      data: {
+        command: COMMAND_SENDTAB,
+        payload: {
+          encrypted: {},
+        },
+        sender: senderID,
+      },
+    },
+  ];
+
+  const fxAccounts = {
+    async withCurrentAccountState(cb) {
+      await cb({});
+    },
+  };
+  const commands = new FxAccountsCommands(fxAccounts);
+  commands.sendTab.handle = (sender, data, reason) => {
+    return {
+      title: "badUriTab",
+      uri: "file://path/to/pdf",
+    };
+  };
+  commands._fxai.device = {
+    refreshDeviceList: () => {},
+    recentDeviceList: [
+      {
+        id: senderID,
+      },
+    ],
+  };
+  const mockCommands = sinon.mock(commands);
+  mockCommands
+    .expects("_getReason")
+    .once()
+    .withExactArgs(pushIndexReceived, remoteMessageIndex);
+  // We shouldn't have tried to open a tab with an invalid uri
+  mockCommands.expects("_notifyFxATabsReceived").never();
 
   await commands._handleCommands(remoteMessages, pushIndexReceived);
   mockCommands.verify();

diff --git a/toolkit/modules/BrowserUtils.jsm b/toolkit/modules/BrowserUtils.jsm
@@ -20,6 +20,17 @@ ChromeUtils.defineModuleGetter(
   "resource://gre/modules/Region.jsm"
 );
 
+XPCOMUtils.defineLazyPreferenceGetter(
+  this,
+  "INVALID_SHAREABLE_SCHEMES",
+  "services.sync.engine.tabs.filteredSchemes",
+  "",
+  null,
+  val => {
+    return new Set(val.split("|"));
+  }
+);
+
 function stringPrefToSet(prefVal) {
   return new Set(
     prefVal
@@ -147,16 +158,9 @@ var BrowserUtils = {
     if (url.spec.length > 65535) {
       return false;
     }
-
-    let scheme = url.scheme;
-
-    return !(
-      "about" == scheme ||
-      "resource" == scheme ||
-      "chrome" == scheme ||
-      "blob" == scheme ||
-      "moz-extension" == scheme
-    );
+    // Use the same preference as synced tabs to disable what kind
+    // of tabs we can send to another device
+    return !INVALID_SHAREABLE_SCHEMES.has(url.scheme);
   },
 
   /**

diff --git a/toolkit/modules/tests/xpcshell/test_BrowserUtils.js b/toolkit/modules/tests/xpcshell/test_BrowserUtils.js
@@ -136,3 +136,16 @@ add_task(async function test_sendToDeviceEmailsSupported() {
   setLanguage(disallowedLanguage);
   Assert.ok(!BrowserUtils.sendToDeviceEmailsSupported());
 });
+
+add_task(function test_isShareableURL() {
+  // Empty shouldn't be sendable
+  Assert.ok(!BrowserUtils.isShareableURL(""));
+  // Valid
+  Assert.ok(
+    BrowserUtils.isShareableURL(Services.io.newURI("https://mozilla.org"))
+  );
+  // Invalid
+  Assert.ok(
+    !BrowserUtils.isShareableURL(Services.io.newURI("file://path/to/pdf.pdf"))
+  );
+});