Investigate a safe way to execute custom formatters hooks from DevTools server
Categories
(DevTools :: Framework, task)
Tracking
(firefox104 fixed)
Tracking | Status | |
---|---|---|
firefox104 | --- | fixed |
People
(Reporter: nchevobbe, Assigned: sebo)
References
(Blocks 2 open bugs)
Details
Attachments
(1 file, 1 obsolete file)
Custom formatters register methods on the content page global that needs to be executed so we can get the wanted representation of such object.
We should make sure that executing such hooks:
- wouldn't cause Security issue (i.e. getting access to privileged code)
- wouldn't cause privacy issue (e.g. disallow any requests / storing data on cookies or storage, …)
We could try to take advantage of our instant evaluation feature [1], and bail out as soon as one of those hooks would have an effect on some state of the page.
Those hooks should only consumes the logged object and return a custom representation for it, so it shouldn't be a blocker for the custom formatters authors, while providing us some guarantees.
Another idea would be to execute the hooks in some kind of Sandbox, but I'm still not sure how that could be done while still maintaining access to the logged object (and maybe the classes defined by a library that would need them?)
Reporter | ||
Comment 1•2 years ago
|
||
Note that this is only one of the aspect we'll look into, we'll probably have restrictive ways of turning this feature on (I wonder if we could maintain a list of integrity hashes (see https://developer.mozilla.org/en-US/docs/Web/Security/Subresource_Integrity) that would be valid formatters.
Or we could only accept formatters as webextensions that we would review and accept.
There might be other areas we could look into to make this as safe as possible.
Assignee | ||
Comment 2•2 years ago
|
||
As Antonin Hildebrand pointed out, the Chrome DevTools implementation for the UI can be found at https://github.com/ChromeDevTools/devtools-frontend/blob/main/front_end/ui/legacy/components/object_ui/CustomPreviewComponent.ts.
In addition to avoiding security and privacy issues plus blocking code from changing the state of the page, the presentation of those formatters within the Console needs to be considered. The Chrome DevTools implementation, for example, restricts the JsonML to a few elements, namely div, span
, ol
, li
, table
, tr
, and td
. They do not restrict the styling, as far as I can see. Allowing arbitrary layout CSS might break the display within the Console, though. I'm not totally sure how to achieve that yet, but I assume the contain
property could help here, maybe in addition with some restrictions regarding CSS properties.
Sebastian
Reporter | ||
Comment 3•2 years ago
|
||
Hello Sebo, I wanted to check how things are going for this :)
Are you blocked on anything, would you need some help ?
Assignee | ||
Comment 4•2 years ago
|
||
Hi Nicolas! Basically I was missing general feedback from your team. I'll write you a separate email regarding this.
Sebastian
Reporter | ||
Comment 5•2 years ago
|
||
Reporter | ||
Comment 6•2 years ago
|
||
So I tinkered with what we have for eager evaluation in the console, and things seem to work well!
The idea is to reuse the makeSideEffectFreeDebugger
function (devtools/server/actors/webconsole/eval-with-debugger.js#327), which already does the heavy work of checking if any stateful expression is triggered.
Then, we can use this special debugger object to wrap and call the custom formatter hooks.
You can see an early implementation in https://phabricator.services.mozilla.com/D134261
If I try to modify the window
object in a custom formatter, it is not executed and we move on to the next formatter.
I did not tested that extensively, but it looks like a good lead to dig and run more test against :)
The nice part of makeSideEffectFreeDebugger
is that it already has a "time out" mechanism, meaning that if a hook takes more than 100ms to run, we'll ignore it and move on (https://searchfox.org/mozilla-central/rev/7d17fd1fe9f0005a2fb19e5d53da4741b06a98ba/devtools/server/actors/webconsole/eval-with-debugger.js#345-353) We might want to be able to tweak that, so the duration could be a param of the function, in case we need just a bit more for the custom formatter.
Assignee | ||
Comment 7•2 years ago
|
||
The first step of parsing custom formatters for objects is checking a website for a global array called devtoolsFormatters
.
If this array exists, it is looped over to find a custom formatter that can handle the object.
In case there is one, its header
and hasBody
methods are executed using makeSideeffectFreeDebugger()
to ensure to get a result without causing any negative side effects.
Those results are then returned together with the index of the custom formatter.
This index is used in case the custom formatter's body is expandedö. In that case the body
property is also executed in a safe way and its value then returned.
Updated•2 years ago
|
Pushed by nchevobbe@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/850d9b814040 Added parsing for custom formatters. r=devtools-backward-compat-reviewers,nchevobbe
Comment 9•1 year ago
|
||
bugherder |
Updated•4 months ago
|
Description
•