1928932 - Implement Trusted Types enforcement for scripts

Status: RESOLVED FIXED

(Core :: DOM: Security, task)

Description

[Frédéric Wang Nélar]

See https://w3c.github.io/trusted-types/dist/spec/#enforcement-in-scripts.

In bug 1905706, I'm just going to make the IDL arguments Trusted Types but the spec has more things e.g. booleans for "trusted" and "changed by trusted sink" and when they are set. See also https://github.com/w3c/trusted-types/issues/525 and https://searchfox.org/mozilla-central/rev/e74b86533c2499eb6393071aeedcfb080d84e4da/testing/web-platform/tests/trusted-types/HTMLScriptElement-internal-slot.html#125,134

Comment 1

[Mirko Brodesser (:mbrodesser) -- not reading bugmails currently]

https://github.com/w3c/trusted-types/pull/533#discussion_r1835791151 might need clarification before implementing this.

Comment 2

[Frédéric Wang Nélar]

We also need a similar mechanism for SVG script element, but it's not in the spec yet: https://github.com/w3c/trusted-types/issues/483

Comment 3

[Frédéric Wang Nélar]

The tests are badly written. I submitted:

https://github.com/web-platform-tests/wpt/pull/50824
https://github.com/web-platform-tests/wpt/pull/50861

The spec is being edited at https://github.com/w3c/trusted-types/pull/581

Comment 4

[Frédéric Wang Nélar]

Luke had commented the implementation would be easier with https://github.com/w3c/trusted-types/pull/579 so I will give it a try.

Comment 5

[Frédéric Wang Nélar]

Attachment: WIP: Bug 1928932 - Part 1 - Add an mIsTrusted flag on script elements.

Spec: https://w3c.github.io/trusted-types/dist/spec/#enforcement-in-scripts
PR: https://github.com/w3c/trusted-types/pull/579/

Comment 6

[Frédéric Wang Nélar]

Attachment: 1928932-ScriptElement-mutation-observer-notifications.html

Comment 7

[Frédéric Wang Nélar]

Attachment: Bug 1928932 - Part 2 - Implement enforcements for untrusted script elements. r=smaug

Spec: https://w3c.github.io/trusted-types/dist/spec/#enforcement-in-scripts
PR: https://github.com/w3c/trusted-types/pull/579/

Comment 8

[Frédéric Wang Nélar]

Attachment: Bug 1928932 - Part 1 - Add an mIsTrusted flag on script elements. r=smaug

Spec: https://w3c.github.io/trusted-types/dist/spec/#enforcement-in-scripts
PR: https://github.com/w3c/trusted-types/pull/579/

Comment 9

[Frédéric Wang Nélar]

Attachment: Bug 1928932 - Part 3 - Transmit proper MutationEffectOnScript to mutation observers. r=smaug

Comment 10

[Frédéric Wang Nélar]

Attachment: Bug 1928932 - Part 4 - Update WPT tests for script enforcements. r=smaug

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/ce2493cdd420
https://hg.mozilla.org/integration/autoland/rev/8f6d960e567b
Part 1 - Add an mIsTrusted flag on script elements. r=smaug,credential-management-reviewers,dimi

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/a15f457eb620
https://hg.mozilla.org/integration/autoland/rev/307d09ef826c
Revert "Bug 1928932 - Part 1 - Add an mIsTrusted flag on script elements. r=smaug,credential-management-reviewers,dimi" for causing macOS build bustages on nsMenuGroupOwnerX.mm

Comment 13

[Cosmin Sabou [:CosminS]]

Backed out for causing macOS build bustages on nsMenuGroupOwnerX.mm

Push with failures

Failure log

Backout link

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/4cca1cbaa379
https://hg.mozilla.org/integration/autoland/rev/bb51e4ab0800
Part 1 - Add an mIsTrusted flag on script elements. r=smaug,credential-management-reviewers,dimi

Comment 15

[agoloman]

https://hg.mozilla.org/mozilla-central/rev/bb51e4ab0800

Comment 16

[Frédéric Wang Nélar]

Attachment: script-enforcement-for-innerText-and-textContent.html

This is a testcase trying to set innerText/textContent on a script via

HTMLElement.prototype.__lookupSetter__("innerText").call(scriptElement, ...)
Node.prototype.__lookupSetter__("textContent").call(scriptElement, ...)

In WebKit, this is always causing a TT security violation.
In Chromium, it is only causing a TT security violation if the specified text is untrusted. And if it is trusted, the script runs normally.

Comment 17

[Frédéric Wang Nélar]

(In reply to Frédéric Wang (:fredw) from comment #16)

In WebKit, this is always causing a TT security violation.
In Chromium, it is only causing a TT security violation if the specified text is untrusted. And if it is trusted, the script runs normally.

After some checks & discussions, WebKit's behavior is the one aligned with the spec here. I'm adding new tests here: https://github.com/web-platform-tests/wpt/pull/54548

Comment 18

[Frédéric Wang Nélar]

https://treeherder.mozilla.org/jobs?repo=try&revision=fc0131e0cfe705abced184caa4f6162234f49234

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/163aea67f34a
https://hg.mozilla.org/integration/autoland/rev/bec0b586fa9e
Part 2 - Implement enforcements for untrusted script elements. r=smaug
https://github.com/mozilla-firefox/firefox/commit/1f81a41d257e
https://hg.mozilla.org/integration/autoland/rev/d803b4b4eec7
Part 3 - Transmit proper MutationEffectOnScript to mutation observers. r=smaug
https://github.com/mozilla-firefox/firefox/commit/0232ead158bc
https://hg.mozilla.org/integration/autoland/rev/b64da9619185
Part 4 - Update WPT tests for script enforcements. r=smaug

Comment 20

[agoloman]

https://hg.mozilla.org/mozilla-central/rev/bec0b586fa9e
https://hg.mozilla.org/mozilla-central/rev/d803b4b4eec7
https://hg.mozilla.org/mozilla-central/rev/b64da9619185

Comment 21

[Hamish Willee]

Can I please confirm what this delivers, because it isn't simple for me to read the code.

From the title I assume that this adds support for Content-Security-Policy: require-trusted-types-for right?
Presumably enabled behind the same preference as the trusted types used in API?

Comment 22

[Frédéric Wang Nélar]

It's indeed behind the TrustedTypes flag.

Support for require-trusted-types-for 'script' was implemented in previous patches. This CSP rule basically just enables Trusted Types checks so that things like

let script = document.createElement("script");
script.text = "/* arbitrary JS code */";

would cause a security violation (or run the default policy on the JS string, if such a policy defined) when we try to set the script text.

Other APIs that can be used to set the script's text, but they don't necessarily run a the Trusted Type check. The patch in this bug implements adds such a check when the script is about to be executed i.e.

let script = document.createElement("script");
script.appendChild(document.createTextNode("/* arbitrary JS code */"));
document.body.appendChild(script);

would cause a security violation (or again, run the default policy) when we append the script to the document.

Comment 23

[Hamish Willee]

Thanks Fred.

So in the first case I can avoid a security violation by passing a TrustedScript or having a default policy - right?

let script = document.createElement("script");
script.text = someTrustedType; // A TrustedScript

In the second case for which this patch exists is there a way OTHER than using a default policy to pass a trusted type to the document.createTextNode()?

My assumption is "no" based on https://w3c.github.io/trusted-types/dist/spec/#enforcement-in-scripts - which seems to indicate that the recommended way to set the content is the one shown above.

Comment 24

[Frédéric Wang Nélar]

(In reply to Hamish Willee from comment #23)

Thanks Fred.

So in the first case I can avoid a security violation by passing a TrustedScript or having a default policy - right?

let script = document.createElement("script");
script.text = someTrustedType; // A TrustedScript

In the second case for which this patch exists is there a way OTHER than using a default policy to pass a trusted type to the document.createTextNode()?

My assumption is "no" based on https://w3c.github.io/trusted-types/dist/spec/#enforcement-in-scripts - which seems to indicate that the recommended way to set the content is the one shown above.

You are right, and there is no way to pass a TrustedScript in the second case, this is mainly to prevent attacker to bypass the trusted type check via the many alternative means of setting the script text. However, someone could still do something like

script.text = mypolicy.createScript(script.text) // create a TrustedScript

to ensure script text is trusted before appending the script. But I'm not sure that's really useful in practice.

And just to be clear, there are many ways to set the script text (see https://wpt.fyi/results/trusted-types/script-enforcement-001.html for example) and multiple of them can be combined. The TT check is only run when the script is about to run (so when document.body.appendChild(script); is called in the example) or immediately if you use a TrustedScript sink like script.text, script.textContent or script.innerText.

(Note sure it's worth documenting, but one subtle thing is that script.innerHTML is a TrustedHTML sink so it will run the TT check to make sure the content is trusted HTML but that does not guarantee it is a TrustedScript)