1393022 - WebExtensions: access to cross-origin document.styleSheets[…].cssRules from content scripts should honour "permissions" from manifest.json

Status: ASSIGNED

(WebExtensions :: General, enhancement, P5)

Description

[Khvoinitsky Mikhail [:m_khvoinitsky]]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:55.0) Gecko/20100101 Firefox/55.0
Build ID: 20170816212843

Steps to reproduce:

manifest.json:
...
  "content_scripts": [
    {
      "matches": ["<all_urls>"],
      "js": [
        "content-script.js"
      ],
      ...
    }
  ],
  "permissions": [
    "<all_urls>",
    ...
  ]
...

content-script.js:
for (let sheet of document.styleSheets)
    sheet.cssRules // do something with them


Actual results:

sheet.cssRules throws DOMException {code : 18, message : "The operation is insecure.", name : "SecurityError"} for sheet from foreign origin relative to page origin.


Expected results:

document.styleSheets access should honour requested WebExtensions permissions, in the example, due to "<all_urls>" access has been requested, full access should be granted no matter which origin stylesheet has.

Comment 1

[Niklas Gollenstede]

As I answered [here](https://discourse.mozilla.org/t/webextensions-porting-access-to-cross-origin-document-stylesheets-cssrules/18359/2), this is actually desired behavior.

The host permission only affects XMLHttpRequests and fetch, not other page APIs:
https://developer.mozilla.org/en-US/Add-ons/WebExtensions/manifest.json/permissions#Host_permissions

Comment 2

[Khvoinitsky Mikhail [:m_khvoinitsky]]

As I answered in the discourse thread, the host permission should affect not only XMLHttpRequests and fetch, but also  document.styleSheets[…] and probably others. Otherwise, workarounds may be slow, buggy and even introduce severe security vulnerabilities.

Comment 3

[Caitlin Neiman [:caitmuenster]]

Hi Mikhail, this has been added to the agenda for the WebExtensions APIs triage on September 26. Would you be able to join us? 

Agenda: https://docs.google.com/document/d/1pw5y-GHwDLPV9bYK4HWCiZtslqFtAeL3G9bC4ZDbdjs/edit#

Wiki: https://wiki.mozilla.org/Add-ons/Contribute/Triage#2017

Comment 4

[Khvoinitsky Mikhail [:m_khvoinitsky]]

Hi. Yes, I will.

Comment 5

[Caitlin Neiman [:caitmuenster]]

The group in the triage meeting agreed that this is something we would like to do as long as it can be done safely. 

Kris, Luca suggested that you might be able to offer input on how to implement this safely.

Comment 6

[Kris Maglione [:kmag]]

This can be done, but it's tricky.

CSS rules are normally only available if the caller principal subsumes the principal of the stylesheet URL:

http://searchfox.org/mozilla-central/rev/f6dc0e40b51a37c34e1683865395e72e7fca592c/layout/style/StyleSheet.cpp#697-699

If we want to allow WebExtensions to access CSS rules for origins they have access to, we need to extend that logic check against the extension's allowedOrigins, too. Since bug 1322235 and bug 1396449, we should be able to do this fairly performantly, but still not for free.

The most obvious solution would be to just extend the principal subsumption check for extension principals to take allowedOrigins into account. That would apply *everywhere* subsumption checks are used, though, including cross-compartment wrappers (bug 1214373, bug 1319201). In some of those places, the overhead could add up pretty fast.

The other option is to just special case the CSS subsumption checks (probably with a method like SubsumesConsideringExtensionOrigins), and then decide where else we'd want to apply the same logic.

Either way, it requires some discussion. And bholley should probably weigh in, both as the CAPS module owner and the Stylo lead.

Comment 7

[Kris Maglione [:kmag]]

Forgot to add needinfo.

(In reply to Kris Maglione [:kmag] (long backlog; ping on IRC if you're blocked) from comment #6)
> This can be done, but it's tricky.
> 
> CSS rules are normally only available if the caller principal subsumes the
> principal of the stylesheet URL:
> 
> http://searchfox.org/mozilla-central/rev/
> f6dc0e40b51a37c34e1683865395e72e7fca592c/layout/style/StyleSheet.cpp#697-699
> 
> If we want to allow WebExtensions to access CSS rules for origins they have
> access to, we need to extend that logic check against the extension's
> allowedOrigins, too. Since bug 1322235 and bug 1396449, we should be able to
> do this fairly performantly, but still not for free.
> 
> The most obvious solution would be to just extend the principal subsumption
> check for extension principals to take allowedOrigins into account. That
> would apply *everywhere* subsumption checks are used, though, including
> cross-compartment wrappers (bug 1214373, bug 1319201). In some of those
> places, the overhead could add up pretty fast.
> 
> The other option is to just special case the CSS subsumption checks
> (probably with a method like SubsumesConsideringExtensionOrigins), and then
> decide where else we'd want to apply the same logic.
> 
> Either way, it requires some discussion. And bholley should probably weigh
> in, both as the CAPS module owner and the Stylo lead.

Comment 8

[Bobby Holley (:bholley)]

This discussion sounds a lot like the discussion that went into bug 1180921. Shouldn't we leverage that same machinery here?

Comment 9

[Kris Maglione [:kmag]]

(In reply to Bobby Holley (:bholley) (busy with Stylo) from comment #8)
> This discussion sounds a lot like the discussion that went into bug 1180921.
> Shouldn't we leverage that same machinery here?

Yup. The machinery really isn't the problem. It's more a question of how far we want to extend cross-origin support for content scripts. Should we keep adding piecemeal exceptions every time someone wants access to a new origin-restricted attribute, or should we just extend subsumption checks for content script principals to the extension's allowed origins?

The second option seems the easiest and most consistent, but also commits us to supporting cross-origin iframe access for the foreseeable future. The first option means a bunch of scattered special cases, and some inconsistent handling of origin checks, but also gives us more control over exactly what we want to support.

Comment 10

[Bobby Holley (:bholley)]

(In reply to Kris Maglione [:kmag] (long backlog; ping on IRC if you're blocked) from comment #9)
> (In reply to Bobby Holley (:bholley) (busy with Stylo) from comment #8)
> > This discussion sounds a lot like the discussion that went into bug 1180921.
> > Shouldn't we leverage that same machinery here?
> 
> Yup. The machinery really isn't the problem. It's more a question of how far
> we want to extend cross-origin support for content scripts. Should we keep
> adding piecemeal exceptions every time someone wants access to a new
> origin-restricted attribute, or should we just extend subsumption checks for
> content script principals to the extension's allowed origins?
> 
> The second option seems the easiest and most consistent, but also commits us
> to supporting cross-origin iframe access for the foreseeable future. The
> first option means a bunch of scattered special cases, and some inconsistent
> handling of origin checks, but also gives us more control over exactly what
> we want to support.

I think the conclusion we came to in bug 1180921 is that it isn't really appropriate to encode the web extension pattern matching into the core security checks. So I think we should do it piecemeal, similar to what we did for XHR. If we want to be less reactive, we could audit the Blink source and see where they added checks for this, and add support for the same stuff on our end.

Comment 11

[Kris Maglione [:kmag]]

OK, that sounds fine to me, then.

Comment 12

[Firefox Bug Husbandry Bot]

Bulk move of bugs per https://bugzilla.mozilla.org/show_bug.cgi?id=1483958

Comment 13

[Thomas Wisniewski [:twisniewski]]

Attachment: 1393022.diff

Would this essentially be enough as the basis for a patch here? (it is letting me access cssRules as expected).

I'd like to get this working to help us experiment more with site patches for webcompat, like testing re-writing -webkit-scrollbar into the new standard form. Just being able to access the rules from the content script would be far less expensive than having to re-fetch stylesheets (and using window.eval to sneak the code past the CSP is... something I'd rather avoid as well).

Comment 14

[Khvoinitsky Mikhail [:m_khvoinitsky]]

Any update on this?

Comment 15

[Giorgio Maone [:ma1]]

Attachment: WebExtension demonstrating how cross-site stylesheet can be accessed by content scripts

Not sure if intended or not, but it seems that a Firefox extensions with "<all_urls>" permissions can read any cross-site stylesheet, provided that it's either imported through the @import directive or a <link> element with the @crossOrigin attribute set to "anonymous".
No need for the server to cooperate, i.e this works on Firefox even if the response contains no Access-Control-Allow-Origin header, as demonstrated by the attached extension.
This does NOT work for Chromium, though, which requires the aforementioned header to be either "*" or include the origin of the content page.

Comment 16

[Luca Greco [:rpl] [:luca] [:lgreco]]

Hi Shane,
Thomas asked me if we can provide our opinion about this issue and in particular the approach he proposed in comment 13.

As brief a summary:

• This would be helpful for the webcompat team work (it allows Thomas' "Testcase Reducer" addon to work on more sites) and Thomas would be willing to help on this, after we have confirmed we want to proceed.
• It looks that we marked it design-decision-approved at the time, but given that it was "quite some time ago" I wanted to ask you explicitly if we want to add it to the bugs to discuss in our next triage (or in our next design decision triage meeting first).
• From a quick look to the previous comments Thomas comment 13 seems to match the requirements ("providing to the content script access to stylesheet related to origin that are allowed to the extension based on its host permissions" as mentioned in comment 6 , and "doing it for this specific scenario and not for everything at once" as mentioned in comment 11)
• Another engineer from the team maintaining those internals should be also providing their perspective (either right after confirmed we are ok or as part review and signing off phases)

Comment 17

[Matthew Herbst]

Hello. Chiming in as the maintainer of react-to-print, a. library that allows a React user to print any sub-component (DOM Element). I recently had a bug report that seems to be caused by this issue, with Firefox blocking access to sheet.cssRules, though I am not using WebExtensions. The pseudo version of what the library does is this, which is basically the library finding all styles on the page and copying them to be inserted into an iframe that is used for the actual printing:

const printWindow = document.createElement("iframe");
const domDoc = printWindow.contentDocument || printWindow.contentWindow?.document;
const headEls = document.querySelectorAll("style, link[rel='stylesheet']");

for (let i = 0, headElsLen = headEls.length; i < headElsLen; ++i) {
  const node = headEls[i];
  if (node.tagName === "STYLE") {
    const newHeadEl = domDoc.createElement(node.tagName);
    const sheet = (node as HTMLStyleElement).sheet as CSSStyleSheet;

    if (sheet) {
      let styleCSS = "";

      // Throws: `Uncaught DOMException: CSSStyleSheet.cssRules getter: Not allowed to access cross-origin stylesheet`
      for (let j = 0, cssLen = sheet.cssRules.length; j < cssLen; ++j) {
        if (typeof sheet.cssRules[j].cssText === "string") {
          styleCSS += `${sheet.cssRules[j].cssText}\r\n`;
        }
      }
      
      newHeadEl.setAttribute("id", `react-to-print-${i}`);
      newHeadEl.appendChild(domDoc.createTextNode(styleCSS));
      domDoc.head.appendChild(newHeadEl);
    }
  }
}

If this is not the right thread for me to be commenting on please let me know and I can do so elsewhere or create a new issue. Thank you.

Comment 18

[Shane Caraveo (:mixedpuppy)]

Rob, I've no issue with this in general, though if it's not something generally allowed (ie other browsers) I think we should consider whether it is restricted to privileged addons. Do you have insight into what chrome does here and why they took whatever approach they took?

Comment 19

[Rob Wu [:robwu]]

(In reply to Matthew Herbst from comment #17)

Hello. Chiming in as the maintainer of react-to-print, a. library that allows a React user to print any sub-component (DOM Element). I recently had a bug report that seems to be caused by this issue, with Firefox blocking access to sheet.cssRules, though I am not using WebExtensions.
(...)
If this is not the right thread for me to be commenting on please let me know and I can do so elsewhere or create a new issue. Thank you.

Your report is unrelated to extensions, and intended behavior. The stylesheet examples that you've linked from https://github.com/gregnb/react-to-print/issues/429#issuecomment-974979427 are cross-origin stylesheets. Cross-origin stylesheets cannot be read by scripts because that would be a security issue.

In your github comment you're also referring to X-Content-Type-Options: nosniff; this feature is associated with CORB (cross-origin read blocking), and a stronger security restriction, in preventing not only JS code from reading the stylesheet, but even preventing the process from attempting to load it at all.

CORB provides a bridge to the expectations for behaviors in content scripts (which relates to Shane's question):

(In reply to Shane Caraveo (:mixedpuppy) from comment #18)

Rob, I've no issue with this in general, though if it's not something generally allowed (ie other browsers) I think we should consider whether it is restricted to privileged addons. Do you have insight into what chrome does here and why they took whatever approach they took?

In Chrome's early extension framework design, host permissions unlocked the ability to perform cross-origin read/fetch requests from content scripts. This feature was removed in order to support the enforcement of CORB in the context of Site isolation (https://www.chromium.org/Home/chromium-security/extension-content-script-fetches). We're planning to do the same in Firefox (bug 1578405).

In general we should try to refrain from using permissions to relax security restrictions in content scripts, especially if it hinders our ability to enforce security with Site isolation.
One could however make a case for reading .cssRules, because even with Site isolation, a valid stylesheet is already read/parsed in the process.
On the other hand, the ability to read .styleSheets[...].cssRules could inadvertently be abused by web pages.

If we want to support this feature at all, I think we should require host permissions AND a new extension permission. Or introduce this feature in the chrome.dom namespace. Anyone interested in this feature in a cross-browser way could file a request at https://github.com/w3c/webextensions.

I ran the following test case to confirm that Chrome currently refuses to offer access to .cssRules in content scripts:

1. Start Chromium with an extension that has a content script at <all_urls>.
2. Visit https://example.com
3. Insert an external stylesheet via the devtools:

   s = document.createElement('link');
   s.type = "text/css";
   s.rel = "stylesheet";
   s.href = "https://robwu.nl/style.css";
   document.head.prepend(s);
   

4. Switch the devtools context to the content script and run document.styleSheets[0].cssRules to access the rules.

Result:

• Uncaught DOMException: Failed to read the 'cssRules' property from 'CSSStyleSheet': Cannot access rules.

Comment 20

[Thomas Wisniewski [:twisniewski]]

@Rob: would it be better/easier to just support Chrome's chrome.devtools.inspectedWindow.getResources method to get at all of the CSS stylesheets? Then we could presumably get at their text content with the related getContent and parse that using a <style> element to get at the rules.

Comment 21

[Rob Wu [:robwu]]

(In reply to Thomas Wisniewski [:twisniewski] from comment #20)

@Rob: would it be better/easier to just support Chrome's chrome.devtools.inspectedWindow.getResources method to get at all of the CSS stylesheets?

Easier? Not likely. Possible slightly more feasible if there is already something that keeps track of the loaded resources.

Then we could presumably get at their text content with the related getContent and parse that using a <style> element to get at the rules.

... which could result in more network activity when the stylesheet references external resources, e.g. external stylesheets with @import.

If you're already willing to perform additional network activity, then the work-around mentioned by Giorgio in comment 15 could work. It is basically forcing the request to be CORS-enabled, and triggers the code path that allows the extension's host permissions to be effective. I guess via https://searchfox.org/mozilla-central/rev/65d4d3399afa79c8de5a0cc11752d2ba7c31edc1/dom/security/nsContentSecurityManager.cpp#1241-1243 + https://searchfox.org/mozilla-central/rev/65d4d3399afa79c8de5a0cc11752d2ba7c31edc1/netwerk/protocol/http/nsCORSListenerProxy.cpp#903-904

Thomas/Giorgio - what are your use cases, and why are getComputedStyle + browser.tabs.insertCSS insufficient?
If there is a clear need for this API we could raise an issue at the WECG to solicit additional feedback.

Comment 22

[Thomas Wisniewski [:twisniewski]]

My use case is that the Testcase Reducer [1] addon currently cannot always grab the styles for iframes, and this causes it to break in many cases. Basically it needs to gather all of the CSS text which applies to whatever parts of the page it is reducing, not just the computed styles.

I suspect that I can use Giorgio's workaround (I'm already using it, just without the <anonymous> trick), but I was hoping to avoid having to use it entirely (to not trigger any page scripts looking for DOM changes and the like).

[1] https://addons.mozilla.org/en-US/firefox/addon/testcase-reducer/