Apply Meta CSP to about:sessionrestore and about:welcomeback
Categories
(Core :: DOM: Security, defect, P3)
Tracking
()
Tracking | Status | |
---|---|---|
firefox70 | --- | fixed |
People
(Reporter: ckerschb, Assigned: ckerschb)
References
Details
(Whiteboard: [domsecurity-backlog1])
Attachments
(2 files, 1 obsolete file)
12.26 KB,
patch
|
Details | Diff | Splinter Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review |
No description provided.
Assignee | ||
Updated•6 years ago
|
Assignee | ||
Updated•6 years ago
|
Assignee | ||
Comment 2•5 years ago
|
||
Hey Gijs, I am trying to apply a CSP to about:welcomeback and about:sessionrestore. We have patch them simultaneously because both include aboutSessionRestore.js. So far that seems to work, but now I am running into problems with an inline script in tree.js [1]
If we don't allow 'unsafe-inline' then this script would be blocked. Obviously we do not want to have unsafe-inline in the CSP, because the whole point of applying a CSP is to stop unwanted inline script to run :-)
We could hash the contents of [1] and und use that hash in the CSP, so the CSP would look something like:
default-src chrome: resource: 'sha256-siVR8vAcqP06h2ppeNwqgjr0yZ6yned4X2VF84j4GmI='
but that would mean we have to update the hash whenever we touch the inline script [1]. Obviously we could document that clearly, but I am not sure if there is a better option this very moment.
Any thoughts?
[1] https://searchfox.org/mozilla-central/source/toolkit/content/widgets/tree.js#588
Comment 3•5 years ago
•
|
||
Comment on attachment 9077613 [details] [diff] [review] bug_1497209_csp_about_sessionrestore.patch Review of attachment 9077613 [details] [diff] [review]: ----------------------------------------------------------------- The tree.js script can add its handlers using `addEventListener` instead, I think? You can store the fragment result of the parse into a local variable, and then do something like: ```js let fragment = ...; let handledElements = fragment.querySelectorAll("scrollbar,scrollcorner"); let stopAndPrevent = e => { e.stopPropagation(); e.preventDefault(); }; let stopProp = e => e.stopPropagation(); for (let el of handledElements) { el.addEventListener("click", stopAndPrevent); el.addEventListener("contextmenu", stopAndPrevent); el.addEventListener("dblclick", stopProp); el.addEventListener("command", stopProp); } this.shadowRoot.appendChild(fragment); ``` I haven't tested this, but I think this should work? ::: browser/themes/shared/aboutSessionRestore.css @@ +26,5 @@ > } > > +#sessionData { > + display: none; > +} Instead of doing this in the CSS, I'd just add the `hidden="true"` attribute to the markup
Assignee | ||
Comment 4•5 years ago
|
||
Gijs, thanks for your suggestions - the attached patch seems to work. At least in terms of 'not getting any CSP errors'.
I am facing a different problem however. When loading about:sessionrestore
and hitting 'Start New Session' which is the button with 'id='errorTryAgain'' nothing happens. Any ideas what might be wrong here, or alternatively how I can debug that?
Additionally, if you have any suggestions of what I can try to manually test that everything is fine here, please let me know.
Comment 5•5 years ago
|
||
When you load the URL manually there's no session data. So the best way to test would probably be to start the browser normally, open some tabs, and then force-crash the browser, which should get you about:sessionrestore on startup.
However, the case you're running into now is also interesting, because you hit this early return: https://searchfox.org/mozilla-central/rev/1097f59d0bc17f6f8f805325c2f607e60cf0d26b/browser/components/sessionstore/content/aboutSessionRestore.js#54-58
and you added the listeners for the buttons below that, so we never add an event listener to the button, so nothing happens when the button is clicked.
We should make sure that all the listeners are added before we ever return from onLoad
.
Comment 6•5 years ago
|
||
For reference: I used the in-browser devtools - they have a little [event] bubble next to nodes in the inspector, so you can see which ones have event listeners attached, and you can click it to get a list of the event listeners and open them in the debugger etc. . This makes it fairly easy to see there's no command
event listener on the button. Then the question is why not... so I set a breakpoint in the JS debugger pane on the first line of onLoad
and refreshed the page.
Assignee | ||
Comment 7•5 years ago
|
||
Pushed by mozilla@christophkerschbaumer.com: https://hg.mozilla.org/integration/autoland/rev/15c45a5c2337 Apply Meta CSP to about:sessionrestore and about:welcomeback. r=Gijs,vporof
Comment 9•5 years ago
|
||
bugherder |
Updated•5 years ago
|
Description
•