Add script protection mechanisms to SVGScriptElement #524
Conversation
spec/index.bs
Outdated
| through a compliant sink. Equivalent to script's | ||
| [=child text content=]. Initially an empty string. | ||
|
|
||
| Issue: There's currently no defined processing model for {{SVGScriptElement}}s. This spec assumes one that is similar to that of {{HTMLScriptElement}}'s model. |
There was a problem hiding this comment.
This feels iffy but there really seems to be no spec for SVGScriptElement's atm. So this is an attempt to say "use common sense and good luck"
There was a problem hiding this comment.
There's https://www.w3.org/TR/SVGMobile12/script.html#ScriptContentProcessing which is an old version of an SVG spec which could be monkey patched but idk that it really provides much value doing that.
|
I've rebased this atop the parser PR so I can account for everything SVG in one PR. |
lukewarlow
changed the title
|
Wouldn't it be better to handle HTML and SVG script at the same time? |
Perhaps but they're pretty distinct from each other spec wise. This PR adds the "script text" and parser handling to svg script element that HTML script has. The only bit this PR is missing is actually doing the enforcement. I'm not sure how best to go about that due to the fact that SVG doesn't have a "prepare the script" method definition. #525 will account for the more complicated parser stuff in one go for both SVG and HTML. |
| <li>... | ||
| </ol> | ||
|
|
||
| Issue: There's no proper definition for the processing of SVG script elements. However, you should apply a similar change to the processing of {{SVGScriptElement}}s. |
There was a problem hiding this comment.
@annevk can you think of a better way to word this, or a way to actually spec this?
I could try patching https://www.w3.org/TR/SVGMobile12/script.html#ScriptContentProcessing but it's a bit lose on the details and is fairly old, so idk that we'd ever upstream the patching.
There was a problem hiding this comment.
The long term solution is probably for HTML to define both script elements, similar to how they are implemented.
Short term we might get away with some issue markers?
It's still not entirely clear to me that "script text" is a good approach as it essentially doubles the storage cost. What happened to some kind of boolean that we'd invalidate?
There was a problem hiding this comment.
The boolean bit is currently only used as part of the "have the children been changed by an API during parsing" code.
There was a problem hiding this comment.
The problem with a Boolean bit is how to allow the specific IDL and the "actually parsed" stuff but nothing else. I'm currently unable to think how you would do that.
There was a problem hiding this comment.
Everything else that mutates would end up flipping the bit? It might help if we went through all the entry points we are concerned about or why this would be hard for some of them.
There was a problem hiding this comment.
I'm still at 1 boolean. Presumably if a script set is trustworthy you can just set the existing bit to trustworthy in the end.
There was a problem hiding this comment.
Ah yeah actually we don't need a parse specific one anymore and provided it's all synchronous you're right you could set the "trusted" bit to true after the children changed steps (which would set it to false for all API calls).
If we make children changed always untrust API changes I think that solves any weirdness from mutation events too. Because we'd fail safe and the script would be untrusted. Because we only need the 3 sanctioned APIs to behave properly wrt to mutation events.
Thinking on it some more I think that could work?
cc @otherdaniel what do you think about that (Assuming we can get children changed steps spec to correctly tell us if it's API or parser)?
Relatively small changes needed (assuming my mental model is right) such as that below and then updating the children changed steps and the prepare script steps:
There was a problem hiding this comment.
Thinking on this some more it would also fix #517 (different handling of new lines leading to TT errors)
There was a problem hiding this comment.
Okay so I tried implementing this and hit a snag.
This bit of the HTML spec is ran inside of children changed steps (at least in WebKit and I believe Chromium from a quick look), so prepare the script runs before the isTrusted flag is set back to true at the end of setInnerText.
So that seems to make the above plan not possible?
Edit: I think I've got a way that works using two booleans.
There was a problem hiding this comment.
I've opened a new draft PR which I'll use to do this new model (and will also apply to svg as best it can) #533
Addresses #483
Preview | Diff