Repository navigation
Add trusted types tests for setAttribute that are not sinks. - #50295
Conversation
See w3c/trusted-types#520 We add tests for: - Names that don't correspond to any event handler attribute. - Names that don't correspond to an event handler attribute on any element. - Names that don't correspond to an event handler attribute for the modified element.
annevk
left a comment
There was a problem hiding this comment.
Thank you @fred-wang, this looks great. I hope you don't mind I'm going to use this PR as a check-in on the overall semantics.
Also copying @mozfreddyb and @otherdaniel for that.
| [ | ||
| // The following names correspond to event handler content attributes, but | ||
| // not for the div element. The current version of the spec says we should | ||
| // still run the default policy on them. |
There was a problem hiding this comment.
This also seems a bit weird on reflection, but I guess it's okay?
@domenic as you have dabbled a bit in event handler attributes I wonder what you make of the situation with Trusted Types (and soon the Sanitizer API). I guess there's these four classes:
- Elements and event handler content attributes defined for them
- Elements and event handler content attributes not defined for them (a couple are only defined for the
bodyor media elements) - Elements and event handler content attributes that don't exist, but the event handler attribute does exist somewhere
- Elements and event handler content attributes that don't exist, and there's no event handler attribute either
It seems that we're currently leaning towards treating 1 and 2 as something we need to apply policy toward and 3 and 4 as cases we can ignore.
There was a problem hiding this comment.
To be really specific here, I believe 1 and 2 are consistent across browsers as being protected, 3 is something this test prohibits but implementations actually do protect against[1]. 4. is something that all browsers agree aren't protected.
[1] I'm not familiar with Firefox's implementation here but it seems to protect onreadystatechange for example.
There was a problem hiding this comment.
Regarding 3, Firefox protects "onreadystatechange" but not "onopen", so that's probably an implementation bug.
There was a problem hiding this comment.
@fred-wang Did you / Can you please file a bug to make sure this isn't lost?
There was a problem hiding this comment.
@mozfreddyb Sure I opened https://bugzilla.mozilla.org/show_bug.cgi?id=1944504 for that.
There was a problem hiding this comment.
Regarding 2, one simple refinement could be to not do the event handler check if the element is not in the HTML, SVG or MathML namespace: w3c/trusted-types#520
See w3c/trusted-types#520
Closes w3c/trusted-types#573
We add tests for: