Open Bug 1393022 Opened 4 years ago Updated 2 months ago

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

Categories

(WebExtensions :: General, enhancement, P5)

enhancement

Tracking

(Not tracked)

ASSIGNED

People

(Reporter: m_khvoinitsky, Assigned: twisniewski, NeedInfo)

Details

(Whiteboard: [design-decision-approved][dev-ux])

Attachments

(2 files)

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.
Component: Untriaged → WebExtensions: Untriaged
Product: Firefox → Toolkit
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.
Priority: -- → P5
Whiteboard: [design-decision-needed]
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
Hi. Yes, I will.
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.
Flags: needinfo?(lgreco)
Flags: needinfo?(kmaglione+bmo)
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.
Flags: needinfo?(kmaglione+bmo)
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.
Flags: needinfo?(bobbyholley)
This discussion sounds a lot like the discussion that went into bug 1180921. Shouldn't we leverage that same machinery here?
Flags: needinfo?(bobbyholley)
(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.
(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.
OK, that sounds fine to me, then.
Flags: needinfo?(lgreco)
Whiteboard: [design-decision-needed] → [design-decision-needed] [needs-follow-up]
Severity: normal → enhancement
Product: Toolkit → WebExtensions
Bulk move of bugs per https://bugzilla.mozilla.org/show_bug.cgi?id=1483958
Component: Untriaged → General
Whiteboard: [design-decision-needed] [needs-follow-up] → [design-decision-approved][dev-ux]
Attached patch 1393022.diffSplinter Review
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).
Assignee: nobody → twisniewski
Flags: needinfo?(kmaglione+bmo)
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true

Any update on this?

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.

You need to log in before you can comment on or make changes to this bug.