Extensions can inject scripts while the debugger is paused, causing .cssRules access failure
Categories
(DevTools :: Debugger, defect, P3)
Tracking
(Not tracked)
People
(Reporter: saschanaz, Unassigned)
References
(Blocks 2 open bugs)
Details
Attachments
(1 file)
(deleted),
text/html
|
Details |
- Install https://darkreader.org/
- Open the attachment
- You should see
.olga
and.frau
. Now open the DevTools and refresh. - Press F8 twice to pass the debugger statement.
Expected: You should see the same result
Actual: CSSStyleSheet.cssRules getter: Not allowed to access cross-origin stylesheet instead of .olga
.
Reporter | ||
Updated•5 years ago
|
Reporter | ||
Comment 1•5 years ago
|
||
Turns out this only happens with Dark Reader extension, I'm not sure which component this should go.
Comment 2•5 years ago
|
||
So the error does not happen without debugger? That sounds then possible a debugger issue.
Reporter | ||
Comment 3•5 years ago
|
||
So the error does not happen without debugger?
Exactly.
Comment 4•5 years ago
|
||
The priority flag is not set for this bug.
:jlast, could you have a look please?
For more information, please visit auto_nag documentation.
Comment 5•5 years ago
|
||
Harald, could you try reproducing this?
Updated•5 years ago
|
Comment 6•5 years ago
|
||
I can easily reproduce using the STR and test case from comment #0
Note that you need the Dark Reader extension installed
https://addons.mozilla.org/en-US/firefox/addon/darkreader/?src=search
The page is showing: CSSStyleSheet.cssRules getter: Not allowed to access cross-origin stylesheet
error
Brad, is this something we could fix or it's issue with the add-on?
Honza
Comment 7•5 years ago
|
||
Interesting. I can replicate. The error that we are hitting here is a SecurityError, and it's intentionally thrown. In fact, we have a test to ensure that we throw this exception under these circumstances. But of course this exception isn't being thrown when the debugger isn't involved, so there's something about the debugger that's changing the triggering principal from -- I think -- the document's principal to the debugger's principal.
Edit: Wrong! The principal is being set to the extension principal. See Logan's comment 9 for the correct explanation.
So, some solutions:
- Make the debugger use the same principal as the document. This probably breaks a bunch of other things. It's certainly very useful for calls made by the debugger to have different, elevated privileges compared to the document's privileges.
- Make the execution of calls from the debugger be trivial pass-throughs to how the code would normally execute. That probably prevents the debugger from acting as a debugger (catching exceptions, etc).
- Skip the cross-origin access check for calls made by the debugger. This is the easiest thing to do if it's actually possible. The issue is that what we're calling a SecurityError may actually be something more like an "architecture error". With the pref "fission.autostart.enabled" set, each process is only guaranteed to have access to objects from a single origin. We may not be able to give direct access to cross-origin data, regardless of the privileges of the caller.
- If option 3 isn't possible, send a cross-process data packet with the requested data to the caller with appropriate privileges.
A lot going on here. I'd like to get an opinion from Jason on what's going on with the debugger in this situation.
Comment 8•4 years ago
|
||
I'll defer to logan with respect to the debugger's principal.
Comment 9•4 years ago
|
||
Alright I've finished digging into this. This is basically an run-to-completion issue.
The normal flow without the debugger
- Page loads
- Page skips debugger; since devtools are not open.
- Script accessing stylesheets and updating HTML content. Since the stylesheets are inline in the page, they have the page content principal and can be accessed from page JS without throwing.
- The extension injects a script into the page using
chrome.tabs.executeScript
. - That extension assigns
.textContent
on the stylesheet (https://github.com/darkreader/darkreader/blob/14eb81109bf68e0116efd4f91e1f22c20ffd8ca4/src/inject/dynamic-theme/index.ts#L51), which replaces the internal stylesheet representation, and most importantly, the new stylesheet has the extension principal, meaning that page content can't access the stylesheet anymore. (https://searchfox.org/mozilla-central/rev/3d39d3b7dd1b2be30692d4541ea681614e34c786/layout/style/Loader.cpp#1566)
This means for instance that if the test script waited to update the HTML until after step 5, e.g.
setTimeout(() => {
getCSSRules(style1, log1);
getCSSRules(style2, log2);
}, 1000);
the page will get the same error, without devtools having to be open.
So now we get to the run-to-completion issue. While we are paused at the debugger;
statement, step 3 is blocked from running because we are paused, but step 4 can still run because we're still processing requests from extensions to inject scripts into the page, which means that step 4 and 5 run during step 2, and by the time we reach step 3, the stylesheet has been replaced with one that the page content isn't allowed to access.
Comment 10•4 years ago
|
||
That is really useful analysis, thank you. So a way to fix this is to ensure that the calls made from the debugger have a sufficiently elevated principal that they can access stylesheets modified by an extension. Interestingly, we have a test that ensures that this access is prevented when the extension creates the stylesheet.
Kris, should devtools be able to access rules in an inline stylesheet that are modified/overwritten by an extension? Our principal currently prohibits this, and test_ext_contentscript_triggeringPrincipal.js implies that this is intentional when the extension creates the inline stylesheet. Is this a different case that should be allowed?
Comment 11•4 years ago
|
||
It's not that devtools can't access the stylesheet, it's that the page content can't access a stylesheet injected by an extension, and devtools causes the script to be injected at a different time. I think all of the principals are correct, but chrome.tabs.executeScript
should not inject scripts while the tab is paused.
Comment 12•4 years ago
|
||
(In reply to Logan Smyth [:loganfsmyth] from comment #11)
It's not that devtools can't access the stylesheet, it's that the page content can't access a stylesheet injected by an extension, and devtools causes the script to be injected at a different time. I think all of the principals are correct, but
chrome.tabs.executeScript
should not inject scripts while the tab is paused.
Got it, but your example in comment 9 says that the error would occur without devtools if the page tried to access its inline style rules after the injection. The tab isn't paused in that case.
Comment 13•4 years ago
|
||
I was assuming it was expected behavior that page content can't access a stylesheet injected by an extension, but fair question
It is the "ensure that the calls made from the debugger have a sufficiently elevated principal" and "should devtools be able to access rules in an inline stylesheet that are modified/overwritten by an extension" points that I'm contesting because the debugger/devtools is not the issue.
Comment 14•4 years ago
|
||
Yes, I'm convinced we're agreeing. The point I'm adding is that it's not required that the devtools use the same principal as the document. We break this intentionally to fix cases like Bug 1391994, and we may want to break it again here. In my mind, the question is "does a user have an expectation that devtools would work in this case?" If so, and if it doesn't expose a security issue, we should make it work.
Comment 15•4 years ago
|
||
we may want to break it again here
This is what I'm not following. Devtools code is not the code that is throwing the exception in this case, so I'm not sure how the principal of the devtools comes into play.
Comment 16•4 years ago
|
||
(In reply to Logan Smyth [:loganfsmyth] from comment #15)
we may want to break it again here
This is what I'm not following. Devtools code is not the code that is throwing the exception in this case, so I'm not sure how the principal of the devtools comes into play.
We would have to do something like give devtools a special principal for at least the duration of the cssRules getter. Ideally, I'd like to have a devtools principal created when devtools is instantiated, and we use that principal for everything the tools do, but I haven't been able to figure out a good way to do that.
Comment 17•4 years ago
|
||
We would have to do something like give devtools a special principal for at least the duration of the cssRules getter.
I feel like we're talking past eachother. The .cssRules
access happens in Step 3 in my description, which is after all of the devtools frames have left the stack. There is nothing about that code that involved devtools.
The issue is that the debugger;
statement causes the extension-injected script to be executed earlier than it would otherwise have been, meaning that when step 3 runs, the stylesheet has already been replaced by the extension and thus has the extension principal.
Comment 18•4 years ago
|
||
(In reply to Logan Smyth [:loganfsmyth] from comment #17)
The issue is that the
debugger;
statement causes the extension-injected script to be executed earlier than it would otherwise have been, meaning that when step 3 runs, the stylesheet has already been replaced by the extension and thus has the extension principal.
And in that circumstance, if devtools had an elevated principal when invoking the cssRules getter, this would work correctly and not hit the SecurityError. To make that happen, if we change devtools to always have an elevated principal when invoking the cssRules getter, I'm asserting that causes no problems in other use cases. That's the solution I'm proposing.
Comment 19•4 years ago
|
||
if devtools had an elevated principal when invoking the cssRules getter
But it doesn't invoke cssRules
at all, a normal script does. I agree with you that if it did, giving devtools have a special principal could be useful, but that is not the issue that this bug is addressing.
Comment 20•4 years ago
|
||
(In reply to Logan Smyth [:loganfsmyth] from comment #19)
if devtools had an elevated principal when invoking the cssRules getter
But it doesn't invoke
cssRules
at all, a normal script does. I agree with you that if it did, giving devtools have a special principal could be useful, but that is not the issue that this bug is addressing.
Ah, I see. I was misunderstanding what was going on. I thought this was failing to display the rules when the Rule View was trying to inspect them. I'm sorry for introducing confusion.
Does this mean this is working as intended, barring the run-to-completion issue you identified in comment 9? Do we close this bug? Mark it as a duplicate?
Comment 21•4 years ago
|
||
No problem. I think we should leave this open to track the run to completion issue for chrome.tabs.executeScript
.
Updated•4 years ago
|
Description
•