rugk / website-dark-mode-switcher

This is a (Firefox) add-on (WebExtension) that lets you invert the website's color scheme by inverting/changing the prefers-color-scheme media feature of CSS.
https://addons.mozilla.org/firefox/addon/dark-mode-website-switcher/?src=external-github-top
Other
61 stars 4 forks source link

Corss-origin stylesheets without crossorigin="anonymous" attribute not supported (e.g. Hardware.info) #31

Closed DrIT2016 closed 2 years ago

DrIT2016 commented 3 years ago

Affected website: https://nl.hardware.info/

Bug description

Will not go into dark mode site provides on firefox. When Win10 OS is set to dark and you open this site in edge f.e. it will show their developed dark mode (bleuish in this case). Can it be also made to work with this extension on Firefox?

rugk commented 3 years ago

Thanks for the report. Can reproduce the issue with Firefox 85/Linux/Fedora 33. They indeed have a dark mode (just implemented via @media (prefers-color-scheme:dark)). Don't know what the issue here could be, but if someone has more information, feel free to comment – or send a PR of course. :smiley:

FYI: Please mind to fill out the issue template you choose – completly, when you open an issue the next time. This makes it easier to troubleshoot issues or so.

tartpvule commented 3 years ago

@rugk

The CSS on that site is hosted on assets.hwigroup.net, whereas the base domain is nl.hardware.info. It is a SecurityError in the getCssForMediaQuery function. The "workaround" there is bogus; it just skips the stylesheet.

Technical details: Referencing https://developer.mozilla.org/en-US/docs/Web/HTML/Element/link, quote:

If the attribute is not present, the resource is fetched without a CORS request (i.e. without sending the Origin HTTP header), preventing its non-tainted usage.

In the case of this website, there is no "crossorigin" attribute on that <link> element; so even though the server actually responds with the header access-control-allow-origin: *, "non-tainted" usages, of which .cssRules is one, are thus disallowed.

Possible workarounds I see:

  1. Do a privileged fetch. From what I remember, this is what the "Dark Reader" extension does. I do not recommend this approach, as there is nothing prohibiting the server from detecting this fact and then do something nefarious, like returning something completely different.
  2. Use webRequest.filterResponseData to add crossorigin="anonymous" to the<link> element, this will send an Origin header with the stylesheet request (fingerprintable without a specific webRequest handler). For this case, this is all that is needed, but for uncooperative sites, where the server does not bless us with access-control-allow-origin: *, a webRequest handler to add that header is necessary. But this leads to a breakdown of the Same-Origin Policy security, as the page script will now be able to read cssRules, too. So, I do not recommend this.
  3. Use webRequest.filterResponseData to sniff the content of all (non-CORS-allowed) stylesheets to build a cache that the content script can get. Naive implementation of this is a resource hog, so complex optimizations will be needed. Not to mention that you will then need a safe parser to parse the raw CSS. If someone is going to do it, this is my recommended approach, as it requires no new server-observable changes.

P.S. If someone wants to play with number 2, use my "Custom CORS Control" extension with the rule: { "stylesheet": { "nl.hardware.info": { "assets.hwigroup.net": { "*": { "ACAO": "allow" } } } } }, then use the Developer Console to add "crossorigin" attribute and switch "href" around.

rugk commented 3 years ago

Hi, uhm I remembered having written sth. here already, but apparently it got lost.


So if anyone is wondering, if I get it correctly, we are talking about this issue/part in the code: https://github.com/rugk/website-dark-mode-switcher/blob/7c311125b670ed603983f5b56cac26dae36edb6d/src/content_scripts/applyColorSchemeCss.js#L118-L126

Now thanks @tartpvule for your investigation into the issue and finding out what we have done.

2 and 3. are really quite invasive for the website and also require a lot of development to intercept all requsts. (also, it could have negative performance effects.) And for 3., please oh no, I don't want a custom CSS parser.

Following the links there this solution looks like the privileged fetch approach. And this here is the bug report in the Mozilla issue tracker (feel free to upvote it).

tartpvule commented 3 years ago

2 and 3. are really quite invasive for the website and also require a lot of development to intercept all requsts. (also, it could have negative performance effects.) And for 3., please oh no, I don't want a custom CSS parser.

Following the links there this solution looks like the privileged fetch approach. And this here is the bug report in the Mozilla issue tracker (feel free to upvote it).

After investigating this further...

The missing piece is actually Constructable Stylesheets, which, according to this page, is behind layout.css.constructable-stylesheets.enabled. Ironically, Chromium already shipped this. Due to the fact that Firefox has not actually shipped it in Release, I don't think it's a good idea to commit to that approach yet.

If you're being risky, you can actually create a <link> element in the background page (!) and access cssRules there, but that opens a whole new can of (security) worms. I hope no one is THAT adventurous.

This is number 3 with "the CSS parser" being the browser itself. The idea is: do that loop like normal, but once we (the content script) encounter a SecurityError, we request a copy sniffed from filterResponseData, construct a CSSStyleSheet, then use cssRules from that.

Of course, this has some gotchas, for example:

Note that, if we care about security, we cannot mutate the DOM tree in anyway, including injecting our stylesheets as <style> or similar, as that can be observed by a page's hostile code.

Not to mention, if/when Firefox introduces the ability for extensions to directly control prefers-color-scheme, all these work will not be needed (and thus be thrown out).

This is very difficult to get right IMHO. I don't think I'm going to try implementing this any time soon. Anyone with the expertise?

rugk commented 3 years ago

I totally agree, it's likely not worth implementing this right now.

Due to the fact that Firefox has not actually shipped it in Release, I don't think it's a good idea to commit to that approach yet.

So yeah, when Firefox adds this, maybe then there is a better time to look at it.

Thus, marking it as blocked now.

rugk commented 2 years ago

Hi, I have great news for you! According to my testing, the latest v2.0 release fixes this issue due to the way it now uses a completely new API to do "dark-mode-changing" "properly". For more information, please see the release notes. If the issue should not be fixed, please let me know.