trusted-types icon indicating copy to clipboard operation
trusted-types copied to clipboard

Are all injection sinks covered by the spec?

Open mbrodesser-Igalia opened this issue 1 year ago • 20 comments

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 avatar Jan 08 '24 14:01 mbrodesser-Igalia

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.

mbrodesser-Igalia avatar Jan 08 '24 14:01 mbrodesser-Igalia

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.

koto avatar Jan 15 '24 09:01 koto

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.

mbrodesser-Igalia avatar Jan 15 '24 10:01 mbrodesser-Igalia

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.

koto avatar Jan 15 '24 10:01 koto

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.

mbrodesser-Igalia avatar Jan 15 '24 11:01 mbrodesser-Igalia

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.

mozfreddyb avatar Jan 15 '24 13:01 mozfreddyb

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

lukewarlow avatar Jan 15 '24 14:01 lukewarlow

@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

mbrodesser-Igalia avatar Jan 16 '24 09:01 mbrodesser-Igalia

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.

annevk avatar Jan 16 '24 09:01 annevk

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 avatar Jan 16 '24 10:01 mbrodesser-Igalia

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.

mbrodesser-Igalia avatar Jan 16 '24 10:01 mbrodesser-Igalia

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 avatar Jan 16 '24 10:01 lukewarlow

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 avatar Jan 16 '24 11:01 lukewarlow

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 avatar Jan 16 '24 11:01 lukewarlow

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 avatar Jan 16 '24 11:01 lukewarlow

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

lukewarlow avatar Jan 16 '24 11:01 lukewarlow

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?

mbrodesser-Igalia avatar Jan 23 '24 09:01 mbrodesser-Igalia

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)

mozfreddyb avatar Jan 23 '24 10:01 mozfreddyb

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?

mbrodesser-Igalia avatar Jan 23 '24 11:01 mbrodesser-Igalia

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...

mozfreddyb avatar Jan 23 '24 14:01 mozfreddyb