Skip to content

Consider something like a "Trusted Type" system for DOM manipulation. #3052

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

Closed
mikewest opened this issue Sep 19, 2017 · 6 comments
Closed
Labels
addition/proposal New features or enhancements needs implementer interest Moving the issue forward requires implementers to express interest

Comments

@mikewest
Copy link
Member

TL;DR: Some folks on Google's security team and I have sketched out https://github.com/mikewest/trusted-types. Feedback on the general notion would be very helpful.

DOM-based XSS attacks keep popping up on websites (and it's only becoming a more common vector as folks discover gadgets in popular libraries). Internally, Google has had a good deal of success at combating this kind of problem with compile-time enforcement of a set of safe HTML types that allow security reviewers to concentrate their time on the places that generate strings meant for the DOM, rather than auditing each DOM manipulation.

Our goal here is to determine whether there's something generic that we can extract from the various rule sets enforced internally.

The approach is quite vague at the moment, but breaks into three pieces:

  1. We'll introduce a set of types that correspond to various DOM APIs that convert strings to DOM / URLs / etc. These types will auto-escape the characters that would be interesting in a given context, or allow explicit unsafe creation of an unescaped string.

  2. We'll enumerate the DOM sinks we're interested in protecting, and overload each with a variant that accepts a relevant type rather than a DOMString.

  3. We'll give developers some mechanism to disable the DOMString variants to force themselves through the type system.

We've started prototyping what this might look like in Chrome, and plan to upstream tentative tests as we go (see web-platform-tests/wpt#6457, for example). But central questions like layering with libraries and etc. are up in the air at the moment, so y'all's input from the HTML side would be quite helpful.

Thanks!

@annevk annevk added addition/proposal New features or enhancements needs implementer interest Moving the issue forward requires implementers to express interest labels Sep 19, 2017
@koto
Copy link
Contributor

koto commented Oct 11, 2019

We've outlined what we think would be the HTML integration points needed for Trusted Types.

Approaches

The current draft spec implements the TT checks at the DOM sinks (JS functions) layer, and then discards the type information, such that TT become invisible to most other Web APIs and algorithms. The upside is that TT are simpler to spec & implement, the downside is that the future sinks might be introduced that skip the TT logic, and re-introduce DOM XSS-proneness. Or that we might have missed some existing algorithms that would bypass TT already. There's also a few of bypasses that require some additional custom protections (e.g. this). Let's call that tt-at-sinks approach.

@annevk proposed an alternative approach - to keep the type information intact such that it can be verified by the algorithms running later on (e.g. when script is to be prepared, check that its URL was a TrustedScriptURL. Let's call that tt-at-primitives approach.

TT-at-sinks

The exact text is currently at Integrations with HTML. Summarizing, it's:

TT-at-primitives

We think TT should throw a TypeError when a sink function is called with a non-conforming value, such that the applications can start statically verifying that e.g. HTMLScriptElement.src accepts a TrustedScriptURL and not a string (instead of getting the error being thrown later, when the script node is injected into the document and the script is prepared).

What follows is that likely some of the changes above would also be required. Some of the workarounds might be dropped though (e.g. sink changes to attribute nodes manipulation, or script text nodes).
I outlined the exact changes that would be needed for this in webappsec-trusted-types#176. First and foremost, it requires DOM to modify the Attr and Text nodes to also carry type information, that will later on be used in HTML. Specifically for HTML, it requires that:

@annevk points out that indeed this approach, while more principled and avoiding the need for bypass workarounds, is more invasive - especially for DOM.

I'll follow up with more concrete design for the TT-at-primitive changes, and the integration bugs with other specs, but it would be beneficial to get y'all feedback on this.

koto added a commit to koto/trusted-types that referenced this issue Nov 8, 2019
…#3052

As proposed by @annevk, add slots for script URL / text, populate them
when calling sink functions, and verify them when a script is prepared,
optionally running a default policy on a value read from the DOM if
it's different than the slot value.

It avoids integration points with DOM mutation algorithms, but we still
need to support script.setAttribute('src').
@koto
Copy link
Contributor

koto commented Nov 13, 2019

w3c/trusted-types#236 has an alternate implementation of tt-at-primitives for scripts, essentially adding internal slots for script elements and checking them in prepare a script.

Preview: data:text/html,<script>fetch('https://raw.githubusercontent.com/koto/trusted-types/integrationstake2/dist/spec/index.html').then(r=>r.text()).then(t=>{document.write(t);setTimeout(()=>{document.querySelector('%23enfocement-in-scripts').scrollIntoView()},1000)})</script>

Thoughts?

We'd need to do something about inline event handlers as well. For now I see that they are specced via attribute change steps. https://html.spec.whatwg.org/multipage/webappapis.html#event-handler-content-attributes. Is the current spec actually OK there? The CSP may abort setting the handler, but this is after the mutation has been committed to the DOM in https://dom.spec.whatwg.org/#concept-element-attributes-change. If I can reuse the same logic for ading TT check (that in fact would be done in CSP), than that simplifies the integration quite a bit.

@annevk
Copy link
Member

annevk commented Nov 14, 2019

I think ideally it would only modify https://html.spec.whatwg.org/multipage/webappapis.html#event-handler-idl-attributes to keep content attributes string-based.

@koto
Copy link
Contributor

koto commented Nov 14, 2019

What about the changes for script in HTML described above? Does that approach look generally OK? I wanted to start writing this in a similar fashion for other IDL attr + related algos. We can discuss content attributes separately in whatwg/dom#789.

@annevk
Copy link
Member

annevk commented Nov 15, 2019

Yeah, that looks like it matches what we discussed. I left a comment about a concern around mutation events.

koto added a commit to koto/trusted-types that referenced this issue Nov 20, 2019
…#3052

As proposed by @annevk, add slots for script URL / text, populate them
when calling sink functions, and verify them when a script is prepared,
optionally running a default policy on a value read from the DOM if
it's different than the slot value.

It avoids integration points with DOM mutation algorithms, but we still
need to support script.setAttribute('src').
koto added a commit to w3c/trusted-types that referenced this issue Nov 20, 2019
* Alternate take for script enforcement. whatwg/dom#789 and whatwg/html#3052

As proposed by @annevk, add slots for script URL / text, populate them
when calling sink functions, and verify them when a script is prepared,
optionally running a default policy on a value read from the DOM if
it's different than the slot value.

It avoids integration points with DOM mutation algorithms, but we still
need to support script.setAttribute('src').

* Fix reviewer's comments.

* Adding a note to DOM issue.
@lukewarlow
Copy link
Member

I'm going to go ahead and close this issue. Upstreaming of trusted types has already started and will continue as and when spec sections become finalised.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addition/proposal New features or enhancements needs implementer interest Moving the issue forward requires implementers to express interest
Development

No branches or pull requests

4 participants