MetaMask / metamask-extension

:globe_with_meridians: :electric_plug: The MetaMask browser extension enables browsing Ethereum blockchain enabled websites
https://metamask.io
Other
12.03k stars 4.91k forks source link

Inpage injection fails in Firefox under some CSP settings #3133

Closed marcusmolchany closed 2 days ago

marcusmolchany commented 6 years ago

Hey, as far as I can tell, my content security policy is preventing MetaMask from injecting its scripts. This is only happening in Firefox. It works correctly in Chrome, Safari, Opera, and Brave. My script-src directive looks like this:

script-src 'self';

and I'm seeing this csp violation in the js console:

Content Security Policy: The page’s settings blocked the loading of a resource at self. Source: (function e(t,n,r){function s(o,u){if(!n ....

Unfortunately Firefox only shows a preview of the blocked script. I've tried sha256 hashing each of the scripts in the latest Metamask release and adding them to the CSP, but that did not work. If you have any ideas that would be great!

Browser: Firefox 58.0.1 Operating System: Mac OSX 10.13.2

shoenseiwaso commented 1 year ago

This is still an issue

xrchz commented 1 year ago

How is this not fixed yet? Is it also in other wallets or just Metamask?

seniorjoinu commented 11 months ago

As an additional workaround you can add this to your CSP setting: 'sha256-7WNRe20drB93fDeE6wXp+mRpZz4QnQJ0MS4oT5S5bi4='

This is a hash of the script that MetaMask injects inside the page. I acquired it by stopping my debugger inside the contentscript to copy the content of the injected script. Then, I re-created the whole process of creation and injection of the script inside Chrome's developer console on a page with the same CSP parameters. This made the console respond with the following error:

Refused to execute inline script because it violates the following Content Security Policy directive: "script-src 'self' 'unsafe-eval'. Either the 'unsafe-inline' keyword, a hash ('sha256-7WNRe20drB93fDeE6wXp+mRpZz4QnQJ0MS4oT5S5bi4='), or a nonce ('nonce-...') is required to enable inline execution.

From which I copied the hash value.

This workaround will work for as long as MetaMask team don't upgrade their injected script. Once they do so, you'll have to re-do the steps above to get a new hash value.

UPD: Found out they use different injection scripts for each OS. So you actually need to add three hashes to your CSP settings instead of one:

UPD2: Seems like MetaMask team did update their injection code. Edited all the hashes so they match the current version.

seniorjoinu commented 11 months ago

It would be nice if MetaMask team would calculate these hashes themselves and post them somewhere we could find them. Or if they don't update the injection script so frequently. Or if they fix the problem already.

Can we get someone from the team to this thread? It seems like they've abandoned it.

manuelsc commented 11 months ago

Can we get more momentum on this issue given the recent ledger connector incident https://twitter.com/bantg/status/1735283652857119140 Teams wanting to enforce stricter CSP rules but can't because of issues like these.

debilin commented 10 months ago

+1 :pray:

glitch-txs commented 9 months ago

I created an NPM pacakge that "fixes" this.

  1. Install

    npm i metamask-csp-firefox
  2. Import it on top of your app

import 'metamask-csp-firefox'

Here is the source code (at least worked for me hehe): https://github.com/glitch-txs/metamask-csp-firefox

Considerations: I added an ugly workaround for when the wallet is not installed, relying on MetaMask always returning the chain Id when requested, even when the wallet is locked. I'm not sure if this behavior is going to change in the future, is there other way to ping the wallet? @danfinlay @Gudahtt

shanejonas commented 9 months ago

@glitch-txs check out https://github.com/MetaMask/detect-provider which is doing something similar to your lib.

There's an existing bug with Firefox where it shouldn't enforce csp for injected content scripts that still hasn't been resolved: https://bugzilla.mozilla.org/show_bug.cgi?id=1267027

glitch-txs commented 9 months ago

Thanks for clarifying! So sad it's 8 years old issue in Firefox :(

davidmurdoch commented 9 months ago

I briefly looked into ways of solving this from MM's side. It's a bit tricky. Here are 2 ideas:

1. Xray vision

Firefox Extensions are unique in that Firefox allows for extensions to add properties to the page's window object via Xray vision via a helper function cloneInto. But cloneInto feature uses the structuredClone algorithm, plus some extra magic to clone functions, to copy the JS object from the content_script's realm to the page's realm. The problem here is that the MM provider makes use of Proxy, Promise, and async -- and those objects cannot be cloned. There are probably more objects used by the MM Provider that are "too exotic" for clone, I just didn't get that far to find them.

2. onBeforeSendHeaders

Another approach I toyed with is altering the CSP headers to allow our script. I toyed with the existing extension (ModHeader - this is not an endorsement) to alter the CSP headers to allow any script-src (script-src * data: blob: 'unsafe-inline' 'unsafe-eval';) and it works. We'd have to add "webRequestBlocking" to our manifest.json's permissions to make this work for our extension. We'd need to make use of the onBeforeSendHeaders event to alter the at least content-security-policy and content-security-policy-report-only response headers. I'm unsure if HTML meta tag will need to be altered (but I doubt it). Ideally we'd inject inpage.js with a nonce or integrity hash, and only add that value to the CSP directive.

glitch-txs commented 9 months ago

It would be nice to get a fix on this without altering too much the CSP as we want to keep the safe feature for the users. It's also a very old issue and Firefox doesn't seem to have intentions to fix it, so finding a way around it would be great for all wallets.

dmlayton commented 9 months ago

Ideally we'd inject inpage.js with a nonce or integrity hash, and only add that value to the CSP directive.

Yes! This is what I was surprised to find lacking when I encountered the issue. Otherwise, it's not safe, right?

github-actions[bot] commented 4 months ago

This issue has been automatically marked as stale because it has not had recent activity in the last 90 days. It will be closed in 45 days if there is no further activity. The MetaMask team intends on reviewing this issue before close, and removing the stale label if it is still a bug. We welcome new comments on this issue. We do not intend on closing issues if they report bugs that are still reproducible. Thank you for your contributions.

shoenseiwaso commented 4 months ago

Still an issue. Thanks!

FelipeCabreraB commented 2 months ago

Hi @gauthierpetetin this has been an issue since 2018, it's there any effort being made from Metamask team side to fix this in the near future?, or should we all just tell our clients to not use Metamask on Firefox when we have some csp on our app? Just to try and understand, a very straightforward answer will be very well received and really useful, thanks!

gauthierpetetin commented 1 month ago

Hi @FelipeCabreraB , we will reassess this issue in the next week and determine what our next steps are. We'll keep you informed.

gauthierpetetin commented 1 month ago

Hi @FelipeCabreraB , we've got a team discussion on this specific issue today. We're still not able to get an answer. As next step we'll try to reproduce it and assess how many dapps and users are impacted by it. We'll continue to keep you updated.

coderbizman commented 1 month ago

Any update on this? Still seems to be an issue regarding wallets.

gauthierpetetin commented 1 month ago

Hi @coderbizman , this week, we've started working on a fix for this CSP issue in Firefox, consisting in overriding CSP headers. We'll share a PR in this thread as soon as available.

gauthierpetetin commented 2 days ago

The fix is planned to be released in version 12.8.0.

glitch-txs commented 2 days ago

Are there any downsides to overrriding this?

gauthierpetetin commented 2 days ago

We believe there are no downsides, but we can't be certain. Therefore, we've added a toggle in the settings to disable CSP headers overriding if needed.