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
600 stars 70 forks source link

Stringification of TrustedHTML with `null`-data needs to be specified #469

Open mbrodesser-Igalia opened 6 months ago

mbrodesser-Igalia commented 6 months ago

https://w3c.github.io/trusted-types/dist/spec/#abstract-opdef-create-a-trusted-type

mbrodesser-Igalia commented 6 months ago

The policyValue may be null.

mbrodesser-Igalia commented 6 months ago

Chrome stringifies null to "".

annevk commented 6 months ago

Why does IDL not take care of this?

mbrodesser-Igalia commented 6 months ago

Why does IDL not take care of this?

Please elaborate.

annevk commented 6 months ago

If IDL expects a string any kind of other value will have been coerced by the time you get to specification algorithms.

mbrodesser-Igalia commented 6 months ago

If IDL expects a string any kind of other value will have been coerced by the time you get to specification algorithms.

The callback defined at the IDL level allows to return null: https://w3c.github.io/trusted-types/dist/spec/#trusted-type-policy-options.

mbrodesser-Igalia commented 6 months ago

Example: https://jsfiddle.net/hbp7mzov/.

mbrodesser-Igalia commented 6 months ago

If IDL expects a string any kind of other value will have been coerced by the time you get to specification algorithms.

The callback defined at the IDL level allows to return null: https://w3c.github.io/trusted-types/dist/spec/#trusted-type-policy-options.

So IDL shouldn't take care of it?

annevk commented 6 months ago

Yeah, I misunderstood. It seems this comes down to "stringifying" not being defined in step 3 of https://w3c.github.io/trusted-types/dist/spec/#create-a-trusted-type-algorithm.

mbrodesser-Igalia commented 3 months ago

Yes, stringification of null isn't specified.

https://www.w3.org/TR/trusted-types/#trustedhtml-stringification-behavior mentions the associated date value is returned. Chrome's behavior is to return the empty string in that case. According to https://github.com/w3c/trusted-types/issues/414#issuecomment-1906622107 that's the desired behavior.

That seems acceptable, but there might be undesirable implications.

This also affects someTrustedHTML.toString(), e.g. https://jsfiddle.net/yqphfx0r/.

@petervanderbeken: WDYT?

CC @lukewarlow

mbrodesser-Igalia commented 3 months ago

Firefox currently returns "\<empty string>", one can check above example with Firefox Nightly and the pref "dom.security.trusted_types.enabled" set to true. In debug mode, an assertion is violated.

petervanderbeken commented 3 months ago

Yes, stringification of null isn't specified.

https://w3c.github.io/trusted-types/dist/spec/#create-a-trusted-type-algorithm probably needs to handle null explicitly, by setting dataString to an empty string. That's assuming https://github.com/w3c/trusted-types/issues/414#issuecomment-1906622107 is what we want. @koto should be able to confirm that.

https://www.w3.org/TR/trusted-types/#trustedhtml-stringification-behavior mentions the associated date value is returned.

Note that the associated data value is a string, so it can't contain null. I understand that the Gecko implementation currently returns an empty string, but that's just a weird by-product of having an internal "nullable" string type, which will return an empty string if it's assumed to be non-null. I think the implementation should be changed if the above spec change is done.

mbrodesser-Igalia commented 3 months ago

Yes, stringification of null isn't specified.

https://w3c.github.io/trusted-types/dist/spec/#create-a-trusted-type-algorithm probably needs to handle null explicitly, by setting dataString to an empty string. That's assuming #414 (comment) is what we want. @koto should be able to confirm that.

Given https://searchfox.org/mozilla-central/rev/a26e972e97fbec860400d80df625193f4c88d64e/testing/web-platform/tests/trusted-types/TrustedTypePolicy-createXXX.html#66, it's very likely what's desired for the spec too.

mbrodesser-Igalia commented 3 months ago

What's the use-case for letting createHTML return null instead of throwing?

CC @koto, @lukewarlow

mbrodesser-Igalia commented 3 months ago

What's the use-case for letting createHTML return null instead of throwing?

CC @koto, @lukewarlow

The report-only mode with a default policy (https://w3c.github.io/trusted-types/dist/spec/#default-policy-hdr).

mbrodesser-Igalia commented 3 months ago

It'd be clearer if createPolicy only accepted callbacks returning DOMString (not DOMString?) and a - to be added - createDefaultPolicy would allow callbacks returning DOMString?.

lukewarlow commented 3 months ago

It would be clearer but unfortunately that ship sailed about 4 years ago...

mbrodesser-Igalia commented 3 months ago

I wonder if that ship really sailed. Currently, this is only shipped in one engine.

CC @annevk

lukewarlow commented 3 months ago

That one engine gives you something like 75% market share though and it's a fairly core part of the API. Now it's possible that doesn't translate to much usage (ultimately we'd need usage stats from chrome). But it's ultimately not too hard to understand, implement and spec in its current form. And imo also not a particularly massive win given the effort to change.

annevk commented 2 months ago

This is closed, but stringifying in step 3 of https://w3c.github.io/trusted-types/dist/spec/#create-a-trusted-type-algorithm is still not defined...

lukewarlow commented 2 months ago

I'll reopen it. The spec at least says something about the null value now but yeah we need to go through and deal with bits like this and others more thorughly. I've been focusing on the larger normative bits that have been missing, but this smaller editorial work is definitely still on our TODO list.