1907849 - (trusted types) Audit test coverage, making sure that the remaining annotations are timeouts, but not failures

Status: RESOLVED FIXED

(Core :: DOM: Security, task, P3)

Comment 1

[Frédéric Wang Nélar]

Trying to see if the CRASH annotations are still valid: https://treeherder.mozilla.org/jobs?repo=try&revision=2858277c9d51cb52ca69baf3bf062b3f5dc86203

Comment 2

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

Attachment: Bug 1907849: part 1) Add WPTs for setting event handlers with `setAttributeNS`. r=smaug

Adresses https://github.com/w3c/trusted-types/issues/448.

Comment 3

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

Attachment: Bug 1907849: part 2) Transform the test which is commented out in <Element-setAttributeNS.html> to a valid one which is executed. r=smaug

Regarding the removed TODO: checking one non-null namespace should
suffice, since other non-null namespaces likely take the same codepath.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/aa989819968b
part 1) Add WPTs for setting event handlers with `setAttributeNS`. r=smaug
https://hg.mozilla.org/integration/autoland/rev/bc406a35457d
part 2) Transform the test which is commented out in <Element-setAttributeNS.html> to a valid one which is executed. r=smaug

Comment 5

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/49621 for changes under testing/web-platform/tests

Comment 6

[amarc]

https://hg.mozilla.org/mozilla-central/rev/aa989819968b
https://hg.mozilla.org/mozilla-central/rev/bc406a35457d

Comment 7

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 8

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

Attachment: Bug 1907849: part 3) Enrich documentation of <https://www.w3.org/TR/trusted-types/#validate-attribute-mutation>. r=smaug

Before it was hard to understand.

Comment 9

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

Attachment: Bug 1907849: part 3) Enrich documentation of <Element-setAttribute-respects-Elements-node-documents-globals-CSP.html>. r=smaug

Before it was hard to understand.

Comment 10

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

Attachment: Bug 1907849: part 4) Rename <Element-setAttribute-respects-Elements-node-documents-globals-CSP.html>. r=smaug

Another test for an adoption from a TT realm is added in another part.
The renaming makes their relationship clearer.

Comment 11

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

Attachment: Bug 1907849: part 5) Add WPT <Element-setAttribute-respects-Elements-node-documents-globals-CSP-after-adoption-from-TT-realm.html>. r=smaug

Replaces https://github.com/web-platform-tests/wpt/pull/46432/.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/882367ece188
part 3) Enrich documentation of <Element-setAttribute-respects-Elements-node-documents-globals-CSP.html>. r=smaug
https://hg.mozilla.org/integration/autoland/rev/a9bf391403fd
part 4) Rename <Element-setAttribute-respects-Elements-node-documents-globals-CSP.html>. r=smaug
https://hg.mozilla.org/integration/autoland/rev/5a48fdcefee7
part 5) Add WPT <Element-setAttribute-respects-Elements-node-documents-globals-CSP-after-adoption-from-TT-realm.html>. r=smaug

Comment 13

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/49671 for changes under testing/web-platform/tests

Comment 14

[amarc]

https://hg.mozilla.org/mozilla-central/rev/882367ece188
https://hg.mozilla.org/mozilla-central/rev/a9bf391403fd
https://hg.mozilla.org/mozilla-central/rev/5a48fdcefee7

Comment 15

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 16

[Frédéric Wang Nélar]

It seems we have done fixed quite a lot of failures since October, compare for example:

• https://hg.mozilla.org/mozilla-central/file/33e502b0f9b19d37b250b7e1d04b3618ec758d6f/testing/web-platform/meta/trusted-types
• https://hg.mozilla.org/mozilla-central/file/a861e5955acebded52e664b0ca290e040fa933fb/testing/web-platform/meta/trusted-types

We need to go over the list of remaining failures and analyze them.

Comment 17

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

Attachment: Bug 1907849: part 7) Add Worker tests to <DOMWindowTimers-setTimeout-setInterval.html>. r=smaug

Comment 18

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

Attachment: Bug 1907849: part 6) Link from some worker tests to WPT's documentation. r=smaug

Makes clearer what's meant.

Comment 19

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

Attachment: Bug 1907849: part 8) Add WPTs for trusted types when constructing Workers from WorkerGlobalScope. r=smaug

Comment 20

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

Attachment: Bug 1907849: part 9) Extend WPT to check sink value of "Document parseHTMLUnsafe". r=smaug

One step towards fixing https://github.com/w3c/trusted-types/issues/494.

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/75f53a8a4a02
part 6) Link from some worker tests to WPT's documentation. r=smaug

Comment 22

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/49735 for changes under testing/web-platform/tests

Comment 23

[Cristina Horotan [:chorotan]]

https://hg.mozilla.org/mozilla-central/rev/75f53a8a4a02

Comment 24

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 25

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/64153e8a6666
part 7) Add Worker tests to <DOMWindowTimers-setTimeout-setInterval.html>. r=smaug
https://hg.mozilla.org/integration/autoland/rev/1615038844f1
part 8) Add WPTs for trusted types when constructing Workers from WorkerGlobalScope. r=smaug
https://hg.mozilla.org/integration/autoland/rev/cb3e58c8b7ff
part 9) Extend WPT to check sink value of "Document parseHTMLUnsafe". r=smaug

Comment 26

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/49756 for changes under testing/web-platform/tests

Comment 27

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

Update:

• Fixed:
  • https://github.com/w3c/trusted-types/issues/448
  • https://github.com/w3c/trusted-types/issues/447
  • https://github.com/w3c/trusted-types/issues/426
  • https://github.com/w3c/trusted-types/issues/425
  • https://github.com/w3c/trusted-types/issues/567
• Remaining to be fixed:
  • https://github.com/w3c/trusted-types/issues/526
  • https://github.com/w3c/trusted-types/issues/513
  • https://github.com/w3c/trusted-types/issues/494
  • https://github.com/w3c/trusted-types/issues/568
  • https://hg.mozilla.org/mozilla-central/file/d15f84756dc8feca1636cce4a0d164d0f2afd637/testing/web-platform/meta/trusted-types
  • https://bugzilla.mozilla.org/show_bug.cgi?id=1937764

Comment 28

[amarc]

Backed out for causing wpt fails @ block-string-assignment-to-Document-parseHTMLUnsafe.html

• Backout link
• Push with failures
• Failure Log
• Failure line:

TEST-UNEXPECTED-FAIL | /trusted-types/block-string-assignment-to-Document-parseHTMLUnsafe.html | 'Document.parseHTMLUnsafe(string)' assigned via default policy (successful HTML transformation). - assert_equals: expected "Document parseHTMLUnsafe" but got "Document parseHTMLUnsafe "

Comment 29

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

(In reply to amarc from comment #28)

Backed out for causing wpt fails @ block-string-assignment-to-Document-parseHTMLUnsafe.html

• Backout link
• Push with failures
• Failure Log
• Failure line:

TEST-UNEXPECTED-FAIL | /trusted-types/block-string-assignment-to-Document-parseHTMLUnsafe.html | 'Document.parseHTMLUnsafe(string)' assigned via default policy (successful HTML transformation). - assert_equals: expected "Document parseHTMLUnsafe" but got "Document parseHTMLUnsafe "

It's a typo in the actual code (https://searchfox.org/mozilla-central/rev/df850fa290fe962c2c5ae8b63d0943ce768e3cc4/dom/base/Document.cpp#19796). Will fix it in a separate ticket.

Comment 30

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f7666d84884d
part 7) Add Worker tests to <DOMWindowTimers-setTimeout-setInterval.html>. r=smaug
https://hg.mozilla.org/integration/autoland/rev/33f7437132ed
part 8) Add WPTs for trusted types when constructing Workers from WorkerGlobalScope. r=smaug

Comment 31

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

Attachment: Bug 1907849: part 10) Add WPTs for checking `Document.write`'s and `Document.writeln`'s sink values passed to the default policy. r=smaug

Comment 32

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

Attachment: Bug 1907849: part 11) Add WPTs for checking `setHTMLUnsafe`'s trusted types' sink string of the default policy. r=smaug

The strings in the production code contain a superfluous whitespace.

No separate issues are filed for fixing those as all failing tests will
be audited according to
https://bugzilla.mozilla.org/show_bug.cgi?id=1907849#c27. That is,
auditing at all test meta files.

Comment 33

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

Attachment: Bug 1907849: part 12) Add WPTs for checking `outerHTML`'s trusted types' sink string of the default policy. r=smaug

Comment 34

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

Attachment: Bug 1907849: part 13) Add WPTs for checking `innerHTML`'s trusted types' sink string of the default policy. r=smaug

A test for Element innerHTML already exists at
https://searchfox.org/mozilla-central/rev/df850fa290fe962c2c5ae8b63d0943ce768e3cc4/testing/web-platform/tests/trusted-types/default-policy.html#43,74,84.

Comment 35

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

Attachment: Bug 1907849: part 14) Add WPTs for checking `parseFromString`'s trusted types' sink string of the default policy. r=smaug

Comment 36

[amarc]

https://hg.mozilla.org/mozilla-central/rev/f7666d84884d
https://hg.mozilla.org/mozilla-central/rev/33f7437132ed

Comment 37

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 38

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c006cb26e155
part 9) Extend WPT to check sink value of "Document parseHTMLUnsafe". r=smaug
https://hg.mozilla.org/integration/autoland/rev/669822cfa2fb
part 10) Add WPTs for checking `Document.write`'s and `Document.writeln`'s sink values passed to the default policy. r=smaug
https://hg.mozilla.org/integration/autoland/rev/f21fc5b44dfc
part 11) Add WPTs for checking `setHTMLUnsafe`'s trusted types' sink string of the default policy. r=smaug
https://hg.mozilla.org/integration/autoland/rev/4d109782995e
part 12) Add WPTs for checking `outerHTML`'s trusted types' sink string of the default policy. r=smaug
https://hg.mozilla.org/integration/autoland/rev/390c5456d7df
part 13) Add WPTs for checking `innerHTML`'s trusted types' sink string of the default policy. r=smaug
https://hg.mozilla.org/integration/autoland/rev/4b9f0161c5ae
part 14) Add WPTs for checking `parseFromString`'s trusted types' sink string of the default policy. r=smaug

Comment 39

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/49859 for changes under testing/web-platform/tests

Comment 40

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

Attachment: Bug 1907849: part 15) Add check for Range's createContextualFragment's sink value. r=smaug

Comment 41

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

Attachment: Bug 1907849: part 16) Add check in WPT for Element's insertAdjacentHTML's sink value. r=smaug

Comment 42

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

Attachment: Bug 1907849: part 16) Add WPT check for Document's parseHTMLUnsafe's sink value. r=smaug

Comment 43

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

Attachment: Bug 1907849: part 17.1) Move JS code from <block-string-assignment-to-DOMWindowTimers-setTimeout-setInterval.html>. r=smaug

Preparation for reusing it in a following part.

Comment 44

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

Attachment: Bug 1907849: part 17.2) Add Worker/SharedWorker tests to <block-string-assignment-to-DOMWindowTimers-setTimeout-setInterval.html>. r=smaug

Fixes https://github.com/w3c/trusted-types/issues/568.

Comment 45

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/c006cb26e155
https://hg.mozilla.org/mozilla-central/rev/669822cfa2fb
https://hg.mozilla.org/mozilla-central/rev/f21fc5b44dfc
https://hg.mozilla.org/mozilla-central/rev/4d109782995e
https://hg.mozilla.org/mozilla-central/rev/390c5456d7df
https://hg.mozilla.org/mozilla-central/rev/4b9f0161c5ae

Comment 46

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 47

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c4b0f5515c7d
part 15) Add check for Range's createContextualFragment's sink value. r=smaug
https://hg.mozilla.org/integration/autoland/rev/5304ae5946ed
part 16) Add check in WPT for Element's insertAdjacentHTML's sink value. r=smaug

Comment 48

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/49863 for changes under testing/web-platform/tests

Comment 49

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

Attachment: Bug 1907849: part 18) Add sink tests for "get Trusted Types-compliant attribute value". r=smaug

"get Trusted Types-compliant attribute value" is called from
setAttribute*, for links see the test.

setAttribute and setAttributeNS presumably take the same code-paths,
so only testing setAttributeNS here.

For event handlers only one test is added, because other event handlers
presumably take the same code-paths.

Fixes https://github.com/w3c/trusted-types/issues/570.

Comment 50

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

Attachment: Bug 1907849: part 19) Add simple Worker test with unused policy. r=smaug

It timeouts in Chrome and Gecko. Removing creating the policy lets it
pass.

Comment 51

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

Update of the currently known remaining work for this ticket:

• https://github.com/w3c/trusted-types/issues/526
• https://github.com/w3c/trusted-types/issues/513
• https://github.com/w3c/trusted-types/issues/494:
  • The most but potentially not all sinks should now be covered with tests; relevant patches under review: part 17 and part 18 of this Bugzilla ticket.
  • Determining an complete list of all sinks is not trivial, for two reasons. Gecko might not cover all sinks currently and the different specs have pending PRs (e.g. https://github.com/whatwg/dom/pull/1268).
  • All sinks which currently support Trusted Types in Gecko can be determined via https://searchfox.org/mozilla-central/search?q=%5CbTrustedHTML%5Cb%7C%5CbTrustedScript%5Cb%7CTrustedScriptURL%7C%5CbTrustedType%5Cb&path=*.webidl&case=false&regexp=true.
• https://github.com/w3c/trusted-types/issues/568: fixed when the patch of part 17 of this Bugzilla ticket lands.
• https://searchfox.org/mozilla-central/search?q=&path=meta%2Ftrusted-types%2F*.ini&case=false&regexp=false
• https://bugzilla.mozilla.org/show_bug.cgi?id=1937764
• https://github.com/w3c/trusted-types/issues/570: fixed when patch of part 18 of this Bugzilla ticket lands.
• https://github.com/w3c/trusted-types/issues/571
• https://github.com/w3c/trusted-types/issues/572

Comment 52

[agoloman]

https://hg.mozilla.org/mozilla-central/rev/c4b0f5515c7d
https://hg.mozilla.org/mozilla-central/rev/5304ae5946ed

Comment 53

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b2d83332174d
part 17.1) Move JS code from <block-string-assignment-to-DOMWindowTimers-setTimeout-setInterval.html>. r=smaug
https://hg.mozilla.org/integration/autoland/rev/958b78cb25c5
part 17.2) Add Worker/SharedWorker tests to <block-string-assignment-to-DOMWindowTimers-setTimeout-setInterval.html>. r=smaug
https://hg.mozilla.org/integration/autoland/rev/42f2f0e18f47
part 18) Add sink tests for "get Trusted Types-compliant attribute value". r=smaug

Comment 54

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

When continuing writing patches for this issue, consider filing a successor-Bugzilla ticket. In order to avoid affecting multiple releases (https://whattrainisitnow.com/calendar/).

Comment 55

[amarc]

https://hg.mozilla.org/mozilla-central/rev/b2d83332174d
https://hg.mozilla.org/mozilla-central/rev/958b78cb25c5
https://hg.mozilla.org/mozilla-central/rev/42f2f0e18f47

Comment 56

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 57

[Frédéric Wang Nélar]

It seems some commits have not been upstreamed yet by moz-wptsync-bot. Let's wait that it happens, close this and corresponding github issues and open a follow-up bug.

Comment 58

[Frédéric Wang Nélar]

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

It seems some commits have not been upstreamed yet by moz-wptsync-bot. Let's wait that it happens, close this and corresponding github issues and open a follow-up bug.

I stand corrected, I didn't realize multiple commits were grouped in a single PR:

https://github.com/web-platform-tests/wpt/pull/49621/commits
https://github.com/web-platform-tests/wpt/pull/49671/commits
https://github.com/web-platform-tests/wpt/pull/49735/commits
https://github.com/web-platform-tests/wpt/pull/49756/commits
https://github.com/web-platform-tests/wpt/pull/49859/commits
https://github.com/web-platform-tests/wpt/pull/49863/commits

Comment 59

[Frédéric Wang Nélar]

(In reply to Mirko Brodesser (:mbrodesser-Igalia) from comment #29)

It's a typo in the actual code (https://searchfox.org/mozilla-central/rev/df850fa290fe962c2c5ae8b63d0943ce768e3cc4/dom/base/Document.cpp#19796). Will fix it in a separate ticket.

Done in bug 1938069.

(In reply to Mirko Brodesser (:mbrodesser-Igalia) from comment #32)

Bug 1907849: part 11) Add WPTs for checking setHTMLUnsafe's trusted types' sink string of the default policy. r=smaug
The strings in the production code contain a superfluous whitespace.

Done in bug 1939802.

Comment 60

[Frédéric Wang Nélar]

(In reply to Mirko Brodesser (:mbrodesser-Igalia) from comment #51)

• https://github.com/w3c/trusted-types/issues/494:
  • The most but potentially not all sinks should now be covered with tests; relevant patches under review: part 17 and part 18 of this Bugzilla ticket.

Thanks for adding these tests, I'll check the remaining ones.

• Determining an complete list of all sinks is not trivial, for two reasons. Gecko might not cover all sinks currently and the different specs have pending PRs (e.g. https://github.com/whatwg/dom/pull/1268).

The spec I checked are at https://github.com/w3c/trusted-types/issues/494#issuecomment-2568807108 ; I'm pretty confident our implementation covers the sinks from these specs as I checked each reference, paragraph and open PRs one by one. I didn't check WebKit/Chromium in details though so some things with fuzzy spec status might have been missed, this happened for execCommand for example which I only found because it had tests.

• All sinks which currently support Trusted Types in Gecko can be determined via https://searchfox.org/mozilla-central/search?q=%5CbTrustedHTML%5Cb%7C%5CbTrustedScript%5Cb%7CTrustedScriptURL%7C%5CbTrustedType%5Cb&path=*.webidl&case=false&regexp=true.

Note that this is not enough, some sinks like eval and function constructors are not listed.

Comment 61

[Frédéric Wang Nélar]

(In reply to Mirko Brodesser (:mbrodesser-Igalia) from comment #54)

When continuing writing patches for this issue, consider filing a successor-Bugzilla ticket. In order to avoid affecting multiple releases (https://whattrainisitnow.com/calendar/).

Closing as opening bug 1939805. Note that I was thinking to have patches to separate bugs blocking bug 1907849 (similarly to what was done with bugs 1934610, 1935594, 1936058) so so we could have kept the meta bug open, but I guess that's ok...

Comment 62

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

• All sinks which currently support Trusted Types in Gecko can be determined via https://searchfox.org/mozilla-central/search?q=%5CbTrustedHTML%5Cb%7C%5CbTrustedScript%5Cb%7CTrustedScriptURL%7C%5CbTrustedType%5Cb&path=*.webidl&case=false&regexp=true.

Note that this is not enough, some sinks like eval and function constructors are not listed.

Right, thanks for pointing that out. One reason is that eval isn't defined in a .webidl file.

Comment 63

[Frédéric Wang Nélar]

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

The spec I checked are at https://github.com/w3c/trusted-types/issues/494#issuecomment-2568807108 ; I'm pretty confident our implementation covers the sinks from these specs as I checked each reference, paragraph and open PRs one by one. I didn't check WebKit/Chromium in details though so some things with fuzzy spec status might have been missed, this happened for execCommand for example which I only found because it had tests.

I rechecked completely this morning and commented https://github.com/w3c/trusted-types/issues/494 ; there is one sink missing for Service Workers (which is in its own spec) and another one for SVG script (which is not speced yet).