Page 1 of 1
11.0.37 src script
Posted: Wed Aug 12, 2020 7:06 am
by skriptimaahinen
Problem with 11.0.37
Since no more beforescriptexecute safety net, the script in the next sample will not be blocked on file: protocol. Does not appear to cause problems on http:.
Code: Select all
<html>
<head>
<script src="notblocked.js"></script>
</head>
<body></body>
</html>
After doing some testing, it looks like suspending in mutationobserver or main thread does not work reliably anymore. Not sure when this started, but beforescriptexecute should be able to fix it if the listener is added immediately, not after the permissions. Was there some sort of problem with the approach of .37rc1?
Re: 11.0.37 src script
Posted: Wed Aug 12, 2020 11:23 am
by skriptimaahinen
Of course there is another problem with the above.
Code: Select all
<html onclick="console.log('CLICK')"></html>
Any event attribute in element that gets parsed before the CSP is inserted, is not blocked. For documentElement (<html>) this happens by default as it exists already when contentscripts run. However, removing and re-inserting the attributes after CSP has been applied will make them honour the CSP.
Code: Select all
let de = document.documentElement;
let atts = [...de.attributes]
atts.forEach(attr => de.removeAttribute(attr.name));
// insert CSP
atts.forEach(attr => de.setAttribute(attr.name, attr.value));
But this works efficiently only for the documentElement. Doing this e.g. in mutationobserver for other elements is likely too costly performance wise (think
https://webdriver-herald.herokuapp.com/). So can't really let the DOM get parsed any further...
Re: 11.0.37 src script
Posted: Wed Aug 12, 2020 11:46 am
by Giorgio Maone
skriptimaahinen wrote: ↑Wed Aug 12, 2020 7:06 am
Was there some sort of problem with the approach of .37rc1?
Yes,
this one.
Synchronous XHR suspends the current JS execution flow by pushing the current state into a kind of stack. When XHR is done, it's popped out of the stack unless some other flow has been pushed in the meanwhile: in this case it keeps waiting until it's on the top of the pile.
Therefore "suspended" scripts end to be executed in reversed order, causing all sorts of strange bugs.
This does not appear to be the case for Mutation Observers, which (even though they're resumed in the same way) -- don't cause the DOM to actually get messed up.
Re: 11.0.37 src script
Posted: Wed Aug 12, 2020 2:08 pm
by barbaz
Why does NoScript need to do any suspending at all in Firefox?
Re: 11.0.37 src script
Posted: Wed Aug 12, 2020 10:44 pm
by Giorgio Maone
barbaz wrote: ↑Wed Aug 12, 2020 2:08 pm
Why does NoScript need to do any suspending at all in Firefox?
Because file: and ftp: URLs bypass webRequest. Therefore they cannot be injected with a script-blocking CSP by modifying non-existant HTTP headers, and require it to be inserted as a <meta> element (which NoScript does anyway for any URL as a fallback in case something goes wrong with webRequest).
To further complicate things, Firefox (and only Firefox: Chromium doesn't) parses the <html> and the <head> elements (and the content of the latter) of file: and ftp: pages before executing extensions' content scripts. So we need to jump all kind of ropes in order to inject the right CSP at the right time and prevent execution of anything coming earlier.
Case in point, please check
latest dev build, thanks.
v 11.0.38rc1
============================================================
x Work-arounds for edge cases in synchronous page loads
bypassing webRequest (thanks skriptimaahinen)
x [L10n] Updated bn
Re: 11.0.37 src script
Posted: Thu Aug 13, 2020 6:40 am
by skriptimaahinen
Giorgio Maone wrote: ↑Wed Aug 12, 2020 11:46 am
Synchronous XHR suspends the current JS execution flow by pushing the current state into a kind of stack.
Oh, I knew about that. Should have realized it would be a problem.
barbaz wrote: ↑Wed Aug 12, 2020 2:08 pm
Why does NoScript need to do any suspending at all in Firefox?
Also "Disable restrictions for this tab" relies on this as there is no other way to get tab specific data synchronously.
I think you forgot a reload somewhere there.
Also, do you actually see a case where the beforescriptexecute is necessary when the page is stopped? The one in the DocumentCSP seems to be enough after reloading.
However, the event attributes do need to be removed even at this stage. Stop does not seem to stop them and it's quite possible to make script run e.g. on mouse move even during that small time window.
Re: 11.0.37 src script
Posted: Thu Aug 13, 2020 7:30 am
by Giorgio Maone
skriptimaahinen wrote: ↑Thu Aug 13, 2020 6:40 am
I think you forgot a reload somewhere there.
Yes I did, removing it was quite useful for debugging but I left it out accidentally.
skriptimaahinen wrote: ↑Thu Aug 13, 2020 6:40 am
Also, do you actually see a case where the beforescriptexecute is necessary when the page is stopped? The one in the DocumentCSP seems to be enough after reloading.
At this point (considering the negligible overhead) I prefer to err on the cautious side.
skriptimaahinen wrote: ↑Thu Aug 13, 2020 6:40 am
However, the event attributes do need to be removed even at this stage. Stop does not seem to stop them and it's quite possible to make script run e.g. on mouse move even during that small time window.
Done, please check rc2, thanks.
Re: 11.0.37 src script
Posted: Thu Aug 13, 2020 9:15 am
by skriptimaahinen
Looks good now. Thanks!