Change Script Enforcement Mechanism to use flags #533
Conversation
| [=child text content=]. Initially an empty string. | ||
| : an associated boolean <dfn export for="HTMLScriptElement">is trusted</dfn>. | ||
| :: A boolean indicating whether a script element is considered trustworthy for execution. | ||
| Initially true. |
There was a problem hiding this comment.
Initially true which covers scripts inline in the page being parsed.
There was a problem hiding this comment.
Might be worth promoting that comment to a note in the spec.
|
|
||
| Modify the [=The text insertion mode=] algorithm as follows: | ||
| 1. If <var ignore=''>parserChange</var> is false, set [=this=]'s [=HTMLScriptElement/is trusted=] to false. |
There was a problem hiding this comment.
This parserChange is a placeholder for what we end up speccing in whatwg/dom#1288
There was a problem hiding this comment.
Which issues, besides the one mentioned in #533 (comment), is this PR intended to fix?
There was a problem hiding this comment.
When "parserChange" is false and changed by trusted sink is true, couldn't still malicious code have been injected? E.g. if a trusted sink called only someScript.innerText = someScript.innerText that'd make the untrusted code trusted.
There was a problem hiding this comment.
E.g. if a trusted sink called only
someScript.innerText = someScript.innerTextthat'd make the untrusted code trusted.
That would only work if a default policy had sanctioned that value. Else the assignment would fail before the "changed by trusted sink" Boolean is set
0c7d33e
to
f22111f
Compare
| <dt id="scriptEndTag">An end tag whose tag name is "script"</dt> | ||
| <dd> | ||
| <p>...</p> | ||
| 1. If [=this=]'s [=HTMLScriptElement/changed by trusted sink=] is true, set [=this=]'s [=HTMLScriptElement/is trusted=] to true. |
There was a problem hiding this comment.
This flag is used to say hey this is an API change but it's a trusted one. We unset the flag once used.
| element</span> <var>script</var>. This might cause some script to execute, which might cause | ||
| <span data-x="dom-document-write">new characters to be inserted into the tokenizer</span>, and | ||
| might cause the tokenizer to output more tokens, resulting in a [=reentrant invocation of the parser=].</p> | ||
| Issue: Need to double check how [part of script element's spec](https://html.spec.whatwg.org/#prepare-the-script-element:~:text=When%20a%20script%20element%20el%20that%20is%20not%20parser%2Dinserted%20experiences) fits into this. These steps need to happen before prepare the script is called. |
There was a problem hiding this comment.
I think we need to change the html spec when upstreaming to run the prepare the script (under relevant conditions) at the end of the children changed steps.
There was a problem hiding this comment.
Interestingly it would rely on having a bit more information in the children changed steps algorithm if we want to inline it. Because it needs to know what type of change it is (insertion specifically in this case).
I suspect this is why some Chrome and WebKit's childrenChanged functions include more than the dom spec's algorithm. (And is why Firefox implements it in a way that also gives them this more granular informaion).
There was a problem hiding this comment.
@domfarolino should probably look at this.
Also would that create issues with re-entrant invocations?
There was a problem hiding this comment.
It would seem that whatwg/html#10188 already changes that part of the HTML spec to be defined in terms of the children changed steps so I think we'd just need to put our new steps first and then run the post-insertion steps and it'll fix the concerns I had here.
Also add SVGScriptElement to spec
f22111f
to
ac68dd7
Compare
| element</span> <var>script</var>. This might cause some script to execute, which might cause | ||
| <span data-x="dom-document-write">new characters to be inserted into the tokenizer</span>, and | ||
| might cause the tokenizer to output more tokens, resulting in a [=reentrant invocation of the parser=].</p> | ||
| Issue: Need to double check how [part of script element's spec](https://html.spec.whatwg.org/#prepare-the-script-element:~:text=When%20a%20script%20element%20el%20that%20is%20not%20parser%2Dinserted%20experiences) fits into this. These steps need to happen before prepare the script is called. |
There was a problem hiding this comment.
@domfarolino should probably look at this.
Also would that create issues with re-entrant invocations?
Also add SVGScriptElement to spec
Fixes #483, #517
Preview | Diff