Repository navigation
Should all 3 script IDL setters change the associated script text value identically #517
Description
A good point came up during code review of an associated webkit patch that the .innerText setter steps and the .textContent/.text setter steps are different, and presumably could result in a different child contents value? If this is the case the current spec (and Chromium implementation) could lead to some script execution errors despite being set via a trusted object and sanctioned IDL attribute?
Question 1: Are these actually identical in behaviour?
Question 2: If they're not should the spec and implementationed be updated to account for the differences?
Activity
annevk commentedon May 21, 2024
Oh yeah,
innerTextis wild: https://software.hixie.ch/utilities/js/live-dom-viewer/?%3Cscript%3E%0Adocument.querySelector(%60script%60).innerText%20%3D%20%60test%5Cntest%60%0A%3C%2Fscript%3ECan we just not support that?
lukewarlow commentedon May 21, 2024
cc @otherdaniel and @koto curious what your thoughts on this are. I suspect that people might be using this in the wild? Question is can we remove it as a valid IDL sink or do we just document in the spec that it's an oddity and might not always work as expected?
koto commentedon May 21, 2024
Isn't this the case even without Trusted Types? The spec doesn't provide any indication that the value will be functional for any given sink, only that the value has passed through a policy so there's no DOM XSS risk with assigning it.
We definitely need to cover the DOM XSS sinks with Trusted Types, and script contents being set via innerText is used commonly in applications. TT consciously tries to work with the existing sinks, as opposed to e.g. proposing new ones and disabling old ones.
lukewarlow commentedon May 21, 2024
Specifically I mean trusted types related script execution errors. Because the value may mismatch in which case TT will reject it, which could be unexpected.
Sounds like we should either update the implementations and spec to set the value after it's been set so that it matches. Or just to add a note that explains there may be a mismatch?
annevk commentedon May 21, 2024
Commonly? Can you point to an example? Given what it does with newlines I'd expect all kinds of brokenness.
koto commentedon May 21, 2024
I might be missing some context. Do you mean that a string would be passed to
innerTexton a script? Applications do use innerText with script elements intentionally to execute code, which is the only case in which TT would trigger? Can you give an example of what you mean if I misunderstood?That's not easily shareable, but I see it (
HTMLElement.innerTextTT violations) right now in current violation reports, ~200K distinct violations in last 14 days. The scripts are all one liners. It seems to be just yet another variant of injecting code into applications that some developers use for no particular reason.annevk commentedon May 21, 2024
Ah, if the scripts are all on a single line they won't run into problems, but I don't think enshrining that makes sense for a standard.
koto commentedon May 21, 2024
Again, I don't think the TT claim that the values reaching the sinks are semantically correct, only that they were checked before reaching the sinks.
annevk commentedon May 21, 2024
Sure, but we also store the given value in some internal slot, no? It seems that would mismatch with the value we end up using, which seems bad.
lukewarlow commentedon May 21, 2024
Yes, but TT also claims that if they've been checked they'll attempt to execute. Which afaict isn't guarunteed for multiline text with innerText. Because we set the internal slot before the processing, and as such the prepareScript function will throw (or at least call the default policy).
If you paste the above data URL into Chrome (with experimental flag for the fromLiteral to work) then it will error rather than execute. That's not an error from .innerText's spec, that's an error from the Trusted Types spec. The fromLiteral isn't relevant it's just the easiest way to write a quick example like this.
lukewarlow commentedon Jul 9, 2024
This will be fixed by #533 as it avoids the weirdness with innerTexts newline handling (now uses a flag not a duplication of value)