Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide a boundary between Components and DOM #4295

Open
naugtur opened this issue Feb 26, 2024 · 8 comments
Open

Provide a boundary between Components and DOM #4295

naugtur opened this issue Feb 26, 2024 · 8 comments

Comments

@naugtur
Copy link

naugtur commented Feb 26, 2024

I'd like to open up a conversation about supporting use-cases where an application's UI is composed from potentially untrusted code.
With the proliferation of malware in JavaScript supply chain it's a concern that we've been trying to address at LavaMoat by isolating each package into an independent scope with varying access to globals etc. based on a per-package policy (see more: links below)
We've been successful in isolating packages, but the ability to access DOM nodes, and on them ownerDocument and alike can undermine those defenses.

I an pursuing an approach where using the fact that modern components rarely need direct DOM access, we could block leaking of any DOM references into components if only a boundary can be put in place in the DOM diffing code.
In my reading of Preact codebase I was not successful in finding such boundary though. Internal and external use of DOM references doesn't seem strongly separated, so if I were to wrap or deny DOM references, it would also affect Preact internals.

I'm requesting help in identifying places where Preact would expose DOM references to components being rendered and introducing a mechanism to intercept them.

the actual feature request
I imagine it being a factory for creating a render function that accepts configuration including a function that gets called with each DOM reference being assigned/passed to the component representation (result of calling the h function) and returns what should be passed on as the DOM reference. Even if it's an empty object.

I just noticed options.vnode which might be all I need to implement it myself, but the conversation would still be useful to have.

Alternatively, but I can imagine it being too major of a refactor, we could look into separating virtual DOM and real DOM into two distinct layers that communicate via a channel that's potentially even JSON serializable. I guess this ask comes way too early before the pendulum swings from SSR to clientside only + p2p.

Context:
https://github.com/LavaMoat/LavaMoat
https://www.youtube.com/watch?v=U68zPZSc7nk

[edit]
Preact is so beautifully pluggable that I might soon have a prototype and my ask might change into asking whether it's a sound approach.

@lukewarlow
Copy link

I'm not 100% sure on the exact use case you have but https://developer.mozilla.org/en-US/docs/Web/API/Trusted_Types_API it might be worth looking into the trusted types API and integrating with that inside of Preact, allowing you more control over DOM sinks?

@naugtur
Copy link
Author

naugtur commented Feb 26, 2024 via email

@naugtur
Copy link
Author

naugtur commented Feb 29, 2024

Update!
I've made a PoC implementation of what I wanted to achieve.
https://github.com/naugtur/power-broker
It lets me decide which (if at all) DOM features are made available to each package in dependencies.
Example deployed here:
http://naugtur.pl/power-broker/

Existing hooks let me get 90% there.

I'll need a hook into this:

c.base = newVNode._dom;
to capture it before the component can capture it.

@JoviDeCroock
Copy link
Member

JoviDeCroock commented Mar 22, 2024

Hey @naugtur

Happy to see that you figured it out, if you don't mind me asking where does c.base become insecure? I'll noodle a bit on whether or not it's possible with our existing hooks to provide something.

Something you could do is wrap c.base in a defineProperty and throw when it's accessed on components you deem insecure 😅

@naugtur
Copy link
Author

naugtur commented Mar 22, 2024

I don't have a good moment in time to do that defineProperty. A component I don't want to trust can do it first. A package exciting a component might be able to give us a c that's a proxy object.

So my idea is that I need a new hook in options that would be called on the DOM node being passed to a component.
Happy to contribute that if you're open to it.

C.base is not dangerous in a threat model of a regular app that deferens against XSS and alike.
What I'm doing is different. I'm working with the assumption that one of the packages my app is composed of has turned malicious.

LavaMoat lets me give it limited access to globals etc. but getting a hold of any dom node gives it all globals back via c.base.ownerDocument... and countless other means.

@JoviDeCroock
Copy link
Member

JoviDeCroock commented Mar 22, 2024

If you don't mind me asking, why not intercept it after the fact in diffed (or options._commit but that's later) this is synchronous code so it shouldn't be an issue as there's no opportunity to read it in-between setting it to newVNode._dom (which if I understood you correctly is something you already intercept as well) and calling options.diffed

@naugtur
Copy link
Author

naugtur commented Mar 22, 2024

A malicious package could create its own setter in the earliest lifecycle step before it's assigned and capture the value before I get to censor it.

@naugtur
Copy link
Author

naugtur commented Mar 22, 2024

Also, I'm not sure if I intercept _dom. Gonna have to look into it to see how it's exposed.

All my work is linked above if you're curious enough to spend the time to look at it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants