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

Ensure at least one representative of all classes of injection sinks is guarded with TT #419

Closed
mbrodesser-Igalia opened this issue Jan 24, 2024 · 4 comments
Milestone

Comments

@mbrodesser-Igalia
Copy link
Collaborator

Classes of injection sinks are:

  1. IDL attributes, e.g. someScript.src = someSrc.
  2. JS functions, e.g. eval(someString).
  3. DOM APIs, e.g. someScriptElement.setAttribute("src", someSrc).

This is required in order to ensure all injection sinks can be guarded with TT. Finding out later, that some classes can't be covered would violate the second goal of https://w3c.github.io/trusted-types/dist/spec/#goals: "Encourage a design in which security decisions are encapsulated within a small part of the application."

This allows shipping v1 without guarding all injection sinks. Covering the remaining sinks (#385) could be done in v2.

Feedback here is appreciated.

@mbrodesser-Igalia mbrodesser-Igalia added this to the v1 milestone Jan 24, 2024
@koto
Copy link
Member

koto commented Jan 24, 2024

This allows shipping v1 without guarding all injection sinks. Covering the remaining sinks (#385) could be done in v2.

I'm not sure I follow. Do you propose to only cover 3 sinks in total in v1 of the spec, or the Gecko implementation, or something else? The sinks listed in the spec and WPT all need to be covered in whatever implementation at the moment of shipping any version of the spec - anything else will result in observable differences in websites that already enforce TT (For example, the default policy would be called in one browser, and not called in the other for some sinks).

I'm not opposed to stopping at some point, and listing some of the rarer, or more obscure sinks as level 2, but I think level 1 should be covering what's in the spec vs 1 exemplary sink from 3 classes, as that doesn't match what the authors would expect from a security feature.

@mbrodesser-Igalia mbrodesser-Igalia changed the title Ensure one representative of all classes of injection sinks is guarded with TT Ensure at least one representative of all classes of injection sinks is guarded with TT Jan 24, 2024
@mbrodesser-Igalia
Copy link
Collaborator Author

This allows shipping v1 without guarding all injection sinks. Covering the remaining sinks (#385) could be done in v2.

I'm not sure I follow. Do you propose to only cover 3 sinks in total in v1 of the spec, or the Gecko implementation, or something else?
Something else, sorry for the imprecise wording.

The sinks listed in the spec and WPT all need to be covered in whatever implementation at the moment of shipping any version of the spec - anything else will result in observable differences in websites that already enforce TT (For example, the default policy would be called in one browser, and not called in the other for some sinks).

Agreed.

I'm not opposed to stopping at some point, and listing some of the rarer, or more obscure sinks as level 2, but I think level 1 should be covering what's in the spec vs 1 exemplary sink from 3 classes, as that doesn't match what the authors would expect from a security feature.

Agreed.

That is, all sinks which the spec currently already covers, should continue to be covered. Others, potentially like the one mentioned at #385 (comment) and in the ones in the comments following it, are candidates for v2.
Ensuring new injection sinks are covered (e.g. by something like #385 (comment)) could also be added in v2.

@lukewarlow
Copy link
Member

Does this issue need to remain open? Idk if there's anything actionable from it?

@mbrodesser-Igalia
Copy link
Collaborator Author

Does this issue need to remain open?

No.

Idk if there's anything actionable from it?

No.

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

No branches or pull requests

3 participants