mozilla / contain-facebook

Facebook Container isolates your Facebook activity from the rest of your web activity in order to prevent Facebook from tracking you outside of the Facebook website via third party cookies.
Mozilla Public License 2.0
974 stars 176 forks source link

Add-on for Facebook crashes the whole Firefox tab by opening a popup when Facebook Container is enabled #662

Open hckr opened 4 years ago

hckr commented 4 years ago

Actual behavior

My add-on for Facebook crashes the whole Firefox tab by trying to open a window on Facebook domain, the root cause is somewhere inside Facebook Container.

Expected behavior

It shouldn't crash :) and doesn't when Facebook Container is disabled and Firefox restarted afterwards (I don't know why this happened, but it kept crashing as described until I restarted Firefox).

Steps to reproduce

  1. Load a minimal addon which reproduces the issue:

manifest.json:

{
  "manifest_version": 2,
  "name": "Crashed by Facebook Container",
  "version": "0.0.1",
  "content_scripts": [
    {
      "matches": ["*://*.facebook.com/*"],
      "js": ["content.js"]
    }
  ]
}

content.js

const windowOpener = document.createElement("button");

windowOpener.innerText = "Clicking this will crash current Facebook tab!";

windowOpener.onclick = () =>
  window.open("https://facebook.com", "_blank", "width=360, height=240"); // removing all parameters but url also doesn't fix the issue

document.body.insertBefore(windowOpener, document.body.firstChild);

// some styles, irrelevant
windowOpener.style.color = "red";
windowOpener.style.padding = "50px 0";
windowOpener.style.width = "100%";
windowOpener.style.height = "100px";
  1. Go to facebook.com, no need to be logged in.
  2. Click on the button added by the add-on.

Notes

My hunch is something weird goes on while trying to run separate Facebook context in the newly created popup window.

hckr commented 4 years ago

Similar issue, but in Multi-Account Containers and with userscripts in Greasemonkey: https://github.com/mozilla/multi-account-containers/issues/1771

It seems the URL of opened window doesn't make any difference.

hckr commented 4 years ago

I found a workaround: using window.wrappedJSObject instead of plain window.

For example:

window.wrappedJSObject.open("https://mozilla.org");

I'm not necessarily fan of this solution/hack though.

EDIT: Unfortunately, I discovered that this workaround causes problems with messaging between windows via postMessage – responding with e.source.postMessage from popup window to the main window (e.source) fails.

maxxcrawford commented 4 years ago

I'm not able to reproduce this error in a clean install of Firefox 77.0.1 and Facebook Container.

Do you have Greasemonkey installed? If so and you disable Greasemonkey, does it resume normal behavior?

hckr commented 4 years ago

@maxxcrawford I have Greasemoneky intalled, but it doesn't seem to be an issue. I disabled all extensions but Firefox Container, closed Firefox and opened it again, then temporarily loaded the add-on I pasted code for above, opened a new tab with facebook.com, clicked the button and it still crashed.

Then, I created a new Firefox profile (in about:profiles), opened a browser window with this new empty profile, installed Facebook Container, loaded the add-on from above, opened facebook.com, clicked the button... and it still crashed.

Crash looks like this (in Polish):

obraz
maxxcrawford commented 4 years ago

I'm not understanding the issue – is this a compatibility issue between Facebook Container and your add-on/Greasemonkey?

filbo commented 4 years ago

(1) it has nothing to do with Greasemonkey, that was just the initial way people found the problem (2) it is a compatibility issue between Facebook Container and Firefox. Any extension can instantly crash a containerized tab if FBC is installed (3) rarely have I seen a clearer initial presentation of a bug

maxxcrawford commented 4 years ago

@filbo Thanks for highlighting the root issue!

hckr commented 4 years ago

@filbo Do you know what might be an actual cause of this? The same issue exists in Multi-Account Containers.

I think it might be a regression of some kind, as my add-on used to open windows and communicate with them just fine in previous months (I don't know, unfortunately, when it started happening – not more than two months ago I guess). I wasn't using Facebook Container then, but Multi-Account Containers, but there is the same bug there now and I think they are of the same nature.

I'd be happy to help if you gave me some more information, if you have any.

filbo commented 4 years ago

Sorry, no, I'm just one of the maintainers of another extension which is shot in the head by this.

Of course it is a regression of some sort. When a combination of [ browser + browser-vendor-supplied extension ] goes from 'works' to 'reliably crashes the tab', things have unambiguously moved backwards.

Suggestion: take the simple example extension you provided at the top of this report, and strip it to the bone. I think your content.js could be as simple as:

// If caller tab crashes, FBC#662 is not fixed;
// if new tab opens gracefully, it might be.
setTimeout(()=>open("https://mozilla.org"),1000);

-- although it would be better if you could do something like:

if([I don't know how to check: is FBC active?])
    // If caller tab crashes, FBC#662 is not fixed;
    // if new tab opens gracefully, it might be.
    setTimeout(()=>open("https://mozilla.org"),1000);
else
    alert('contain-facebook #662 repro requires Facebook Container active!');
maxxcrawford commented 4 years ago

I've got a working example leveraging windows.create in a background script. The onclick function sends a message with the window params from the content script (as the content script cannot call browser.X functions).

Revised barebones extension:

manifest.json

{
  "manifest_version": 2,
  "name": "Crashed by Facebook Container",
  "version": "0.0.1",
  "background": {
        "scripts": [
          "background.js"
        ]
    },
  "content_scripts": [
    {
      "matches": ["*://*.facebook.com/*"],
      "js": ["content.js"]
    }
  ]
}

content.js

const windowOpener = document.createElement("button");

windowOpener.innerText = "Clicking this will crash current Facebook tab!";

windowOpener.onclick = () => {
  browser.runtime.sendMessage({
    url: "https://facebook.com",
    type: "popup",
    width: 360,
    height: 240
  });
};

document.body.insertBefore(windowOpener, document.body.firstChild);

// some styles, irrelevant
windowOpener.style.color = "red";
windowOpener.style.padding = "50px 0";
windowOpener.style.width = "100%";
windowOpener.style.height = "100px";

background.js

browser.runtime.onMessage.addListener(openWindow);

function onCreated(windowInfo) {
  console.log(`Created window: ${windowInfo.id}`);
}

function onError(error) {
  console.log(`Error: ${error}`);
}

function openWindow(message) {
  const popupURL = "https://instagram.com";
  const creating = browser.windows.create(message);
  creating.then(onCreated, onError);
}

It doesn't answer the main issue, but I was able to open a new URL that hooks into the Containers web request watching/capturing.

hckr commented 4 years ago

I also noticed that the same happens in private window, even with "tabs with context" disabled (in this case it's not connected with Facebook Container at all). Should I also file a bug in Firefox?

maxxcrawford commented 4 years ago

@hckr Yes. This add-on cannot run in private windows. That seems like a browser bug. If, please mention this issue when filing!

hckr commented 4 years ago

I meant my sample add-on (which calls window.open) crashes private tabs ;) But I see, there's also a problem with Facebook Container.