should null & undefined for sinks requiring TT be a passthrough ?

[dejang]

Consider the following scenario:

setTimeout(null, 1);

In a TT environment this would require null to be TrustedScript value. Say we define a default policy to handle such cases and automatically sign those values. We will notice that upon entering createScript hook null & undefined is automatically coerced to a string. Simply returning the value will cause setTimeout and setInterval to execute without throwing an error when, in fact, the API should throw an error notifying you about the invalid argument type and how you can remedy the situation.

Maybe I am not that familiar with all the ways in which TrustedTypes can be used to handle such scenarios but based on the knowledge I have right now it seems to me that one solution would be to allow null & undefined to be a passthrough such that native APIs can throw the relevant error instead of being masked behind a TT validation error, or not thrown at all. The APIs also have different behavior for those values.

Consider this:

eval(null); // nothing
eval(undefined); // nothing
scriptElement.textContent = null; // nothing
scriptElement.textContent = undefined; // nothing
scriptElement.innerHTML = null; //nothing
scriptElement.innerHTML = undefined; // coerced to 'undefined'

setTimeout, setInterval, eval, textContent, innerHTML - when on a Script tag - all hit policy.createScript and they all have different behaviors. They either accept null (eval), throw an error (setTimeout), or coerce to string. Thus, handling this at the policy level to accept something is not a good option since it can start masking legitimate errors in the code.

[lukewarlow]

Should the policy be given the raw value instead of coercing to a string? At least taking nullable strings would help with this issue?

cc @koto @mbrodesser-Igalia

[koto]

Coercing to a string was intentional, though the main case was to prevent passing objects to policy functions, as they then could enable several bypasses (e.g. objects that stringify to different values each time to bypass pre-sanitization checks). Stringifying removes that attack surface from the API. Though, admittedly, this is less of a concern for other primitive types like null/undefined.

Returning null or undefined (vs '') from default policy functions triggers a CSP violation, so allowing nulls/undefined on input for a passthrough default policy would be a semantic change that actually has big breakage potential. OTOH we never found a valid enough reason to support nulls and undefineds in TT-related sinks, and such decision simplified the API. In any case where a true null/undefined value reaches the sink, there is an equivalent statement that achieves the same, and in many times the call is a no-op.

[mbrodesser-Igalia]

Consider the following scenario:

setTimeout(null, 1);

In a TT environment this would require null to be TrustedScript value. Say we define a default policy to handle such cases and automatically sign those values. We will notice that upon entering createScript hook null & undefined is automatically coerced to a string. Simply returning the value will cause setTimeout and setInterval to execute without throwing an error when, in fact, the API should throw an error notifying you about the invalid argument type and how you can remedy the situation.

Maybe I am not that familiar with all the ways in which TrustedTypes can be used to handle such scenarios but based on the knowledge I have right now it seems to me that one solution would be to allow null & undefined to be a passthrough such that native APIs can throw the relevant error instead of being masked behind a TT validation error, or not thrown at all. The APIs also have different behavior for those values.

Consider this:

eval(null); // nothing
eval(undefined); // nothing
scriptElement.textContent = null; // nothing
scriptElement.textContent = undefined; // nothing
scriptElement.innerHTML = null; //nothing
scriptElement.innerHTML = undefined; // coerced to 'undefined'

setTimeout, setInterval, eval, textContent, innerHTML - when on a Script tag - all hit policy.createScript and they all have different behaviors. They either accept null (eval), throw an error (setTimeout), or coerce to string. Thus, handling this at the policy level to accept something is not a good option since it can start masking legitimate errors in the code.

setTimeout doesn't throw, see e.g. https://jsfiddle.net/u2fvp536/.

[mbrodesser-Igalia]

Returning null or undefined (vs '') from default policy functions triggers a CSP violation,

Why?

Be aware that for values null and undefined, the default policy might return 'null' and 'undefined', not ''. See e.g. https://jsfiddle.net/gsuhj956/1/.

so allowing nulls/undefined on input for a passthrough default policy would be a semantic change that actually has big breakage potential.

[koto]

Returning null or undefined (vs '') from default policy functions triggers a CSP violation,

Why?

#414 has some context, but indeed:

• If the default policy is called implicitly by the spec mechanism (e.g. sinks are called with a string value), and the functions passed to the createPolicy 2nd argument return null/undefined, this will cause the CSP violation.
• that doesn't happen ever when the policy createXYZ functions are called explicitly, from JS code, regardless whether the policy is named default or not. These exposed functions always return a Trusted Type, or throw.

[mbrodesser-Igalia]

Returning null or undefined (vs '') from default policy functions triggers a CSP violation,

Why?

#414 has some context, but indeed:

* If the default policy is called _implicitly_ by the spec mechanism (e.g. sinks are called with a string value), and the functions passed to the `createPolicy` 2nd argument return `null`/`undefined`, this will cause the CSP violation.

* that doesn't happen ever when the policy `createXYZ` functions are called explicitly, from JS code, regardless whether the policy is named `default` or not. These exposed functions always return a Trusted Type, or throw.

Behavior confirmed by https://jsfiddle.net/ejvc1up2/1/.