w3c / trusted-types

A browser API to prevent DOM-Based Cross Site Scripting in modern web applications.
https://w3c.github.io/trusted-types/dist/spec/
Other
606 stars 74 forks source link

Are all injection sinks covered by the spec? #385

Open mbrodesser-Igalia opened 10 months ago

mbrodesser-Igalia commented 10 months ago

https://w3c.github.io/trusted-types/dist/spec/#introduction mentions "over 60 different injection sinks".

However, the spec contains:

12 occurrences of "HtmlString" 6 occurrences of "ScriptString" 12 occurrences of "ScriptUrlString"

Which is below 60.

https://w3c.github.io/trusted-types/dist/spec/#injection-sinks mentions The exact list of injection sinks covered by this document is defined in [§ 4 Integrations](https://w3c.github.io/trusted-types/dist/spec/#integrations)..

I web-searched for a full list of injection sinks but found none. If there's such a list, please share.

mbrodesser-Igalia commented 10 months ago

Some injection sinks are covered implicitly by one change of the spec. E.g. eval() and Function() are covered by https://w3c.github.io/trusted-types/dist/spec/#csp-eval.

koto commented 10 months ago

All injection sinks are covered, some are covered not on the IDL+[[StringContext]] layer, e.g. https://w3c.github.io/trusted-types/dist/spec/#enforcement-in-event-handler-content-attributes, https://w3c.github.io/trusted-types/dist/spec/#require-trusted-types-for-pre-navigation-check, or the eval-related sinks.

mbrodesser-Igalia commented 10 months ago

All injection sinks are covered,

Is there a trustworthy (no pun intended) list of those? Otherwise, it's easy to miss some.

some are covered not on the IDL+[[StringContext]] layer, e.g. https://w3c.github.io/trusted-types/dist/spec/#enforcement-in-event-handler-content-attributes, https://w3c.github.io/trusted-types/dist/spec/#require-trusted-types-for-pre-navigation-check, or the eval-related sinks.

koto commented 10 months ago

I don't think there's a single authoritative, exhaustive and up-to-date list of those, but the implementation following the spec should cover all of them, to the best of our knowledge.

The list could be compiled from various XSS related projects, e.g. https://github.com/w3c/trusted-types/blob/8041828e2426a00221850934ae0f9421f77e3e07/src/enforcement.js has the navigation sinks in DOM, https://github.com/google/safevalues/tree/main/src/dom has a similar list of DOM sinks, https://github.com/cure53/DOMPurify/blob/b3b441e6589f49c506b4840aea3421aa83c62f52/test/fixtures/expect.mjs has XSS vectors etc.

The closest thing in a spec is the reverse - a list of DOM elements and attributes that don't cause XSS; see subsections of https://wicg.github.io/sanitizer-api/#constants.

mbrodesser-Igalia commented 10 months ago

I don't think there's a single authoritative, exhaustive and up-to-date

Related to up-to-dateness: https://github.com/w3c/trusted-types/issues/399.

list of those, but the implementation following the spec should cover all of them, to the best of our knowledge.

The list could be compiled from various XSS related projects, e.g. https://github.com/w3c/trusted-types/blob/8041828e2426a00221850934ae0f9421f77e3e07/src/enforcement.js has the navigation sinks in DOM, https://github.com/google/safevalues/tree/main/src/dom has a similar list of DOM sinks, https://github.com/cure53/DOMPurify/blob/b3b441e6589f49c506b4840aea3421aa83c62f52/test/fixtures/expect.mjs has XSS vectors etc.

The closest thing in a spec is the reverse - a list of DOM elements and attributes that don't cause XSS; see subsections of https://wicg.github.io/sanitizer-api/#constants.

mozfreddyb commented 10 months ago

This goes into implementation-specific details, but some engines (e.g., blink) has a list of all known elements and event handler attributes, which makes it possible to properly collect and annotate in a central location.

I don't think the same is true for Gecko, but it's been a while since I last looked.

lukewarlow commented 10 months ago

See https://github.com/w3c/trusted-types/issues/403 for one missing sink (it's quite new)

mbrodesser-Igalia commented 10 months ago

@mozfreddyb: the note at https://wicg.github.io/sanitizer-api/#builtins-justification mentions the values of the constants (the allow lists) will need to be updated when new HTML elements are added to user agents. The situation for trusted types is similar: when new elements or attributes are added, they potentially need trusted types.

So when new elements or attributes are added to the spec, it seems desirable to check whether the Sanitizer API's constants require adaptation and whether those elements or attributes require trusted types. TBH, ideally both should become mandatory. E.g. a PR for a new element or attribute could contain checkboxes for both instead of only relying on reviewers thinking about it. Are there other similar requirements for new elements or attributes one may learn from?

CC @annevk

annevk commented 10 months ago

There's a comment at the top of whatwg/html's source with a checklist. That will have to be updated. But also, per-element information should be maintained in the HTML Standard directly so it's unlikely to get looked over.

mbrodesser-Igalia commented 10 months ago

There's a comment at the top of whatwg/html's source with a checklist.

That's at the top of https://raw.githubusercontent.com/whatwg/html/main/source. TIL.

That will have to be updated. But also, per-element information should be maintained in the HTML Standard directly so it's unlikely to get looked over.

Okay, trusting your experience here. Filed https://github.com/WICG/sanitizer-api/issues/206 for updating it for the Sanitizer API.

mbrodesser-Igalia commented 10 months ago

See #403 for one missing sink (it's quite new)

@lukewarlow: how did you find it?

This ticket should stay open until one can be sure that all sinks are covered.

lukewarlow commented 10 months ago

I found it by chance I happened to be losely following the sanitizer API work and remembered the unsafe variants were merged into the HTML spec recently for DSD parsing support.

lukewarlow commented 10 months ago

https://github.com/w3c/trusted-types/issues/358 - It's possible there's additional sinks that aren't covered too, here's an issue that seems to suggest another one (haven't tested to confirm)

lukewarlow commented 10 months ago

https://github.com/shhnjk/cursed_types - I've also come across this repository that claims to list some DOM XSS bypasses for trusted types. They all seem to be solvable using CSP directives and I'm not sure if any are valid bypasses for what this spec is concerned with but worth bringing to attention.

This page references https://github.com/w3c/trusted-types/issues/357 as something that might need to be fixed still?

lukewarlow commented 10 months ago

https://github.com/w3c/trusted-types/issues/232 - also mentions potential issues with dynamic imports that mentions this stage 2 Ecamscript proposal https://github.com/tc39/proposal-dynamic-import-host-adjustment Are there any updates on that?

lukewarlow commented 10 months ago

https://github.com/w3c/trusted-types/issues/359 - here's another issue that suggests there's a sink not covered.

mbrodesser-Igalia commented 9 months ago

There's a comment at the top of whatwg/html's source with a checklist. That will have to be updated. But also, per-element information should be maintained in the HTML Standard directly so it's unlikely to get looked over.

@mozfreddyb during our internal meeting last Thursday you mentioned an alternative, more robust proposal. Unfortunately I don't remember the details. In case you do, can you please write down that idea here?

mozfreddyb commented 9 months ago

The HTML spec has a list of properties that go with every element. For example the section introducing the style element, has metadata about the element (categories, content model, etc.).

It would be nice if there was an script execution property that goes with every element. This would allow a robust definition of what HTML built-in syntax can cause XSS and what doesn't. I think that is useful for a lot of other specs and probably everyone else building XSS protections for the web. (Editorial: It would be nice if the spec kept all of those listed for a reference table in the appendix of the spec. This bit would require work on the HTML spec parser, I suppose)

mbrodesser-Igalia commented 9 months ago

The HTML spec has a list of properties that go with every element. For example the section introducing the style element, has metadata about the element (categories, content model, etc.).

It would be nice if there was an script execution property that goes with every document.

Not every document, but every element?

Be aware that functions specified in ES wouldn't be covered. E.g. new Function() (https://tc39.es/ecma262/multipage/fundamental-objects.html#sec-function-constructor) eval (https://tc39.es/ecma262/multipage/global-object.html#sec-eval-x).

This would allow a robust definition of what HTML built-in syntax can cause XSS and what doesn't. I think that is useful for a lot of other specs

Mainly the Sanitizer API (https://wicg.github.io/sanitizer-api/), presumably.

and probably everyone else building XSS protections for the web.

Agreed.

(Editorial: It would be nice if the spec kept all of those listed for a reference table in the appendix of the spec. This bit would require work on the HTML spec parser, I suppose)

Does that proposal only affect the HTML spec, or other specs like the DOM Standard too?

mozfreddyb commented 9 months ago

Not every document, but every element?

Typo (and modified above): Yes a script execution property.

Be aware that functions specified in ES wouldn't be covered. E.g. new Function() (https://tc39.es/ecma262/multipage/fundamental-objects.html#sec-function-constructor) eval (https://tc39.es/ecma262/multipage/global-object.html#sec-eval-x).

Yes, that's a different language though. The HTML bits should be in the HTML spec. ECMAScript does have a hook for script execution as part of CSP's control for eval, Function constructor etc - doesn't it?

(Editorial: It would be nice if the spec kept all of those listed for a reference table in the appendix of the spec. This bit would require work on the HTML spec parser, I suppose)

Does that proposal only affect the HTML spec, or other specs like the DOM Standard too?

I'd hope the DOM standard does not create or define additional HTML elements on the fly. But WICG drafts might. Admittedly, this is a bit of a gap that we'll need to close. I'm sure people will try and ship functionality before fully upstreaming to HTML. I suppose that will remain a challenge...