Closed Bug 1642053 Opened 6 years ago Closed 6 years ago

JavaScript Variable DOM Parsing Confusion can lead to XSS

Categories

(Core :: DOM: HTML Parser, defect)

76 Branch
defect
Not set
normal

Tracking

()

RESOLVED INVALID

People

(Reporter: tsangtmc, Unassigned)

References

()

Details

(Keywords: reporter-external, Whiteboard: [reporter-external] [web-bounty-form] [verif?])

Attachments

(2 files)

Please see https://blog.analyzenothing.com/2020/05/javascript-variable-dom-parsing_29.html for a full write up.

  • when inserting a combination of a comment tag with a script tag as a string into a JavaScript variable, <!-- <script>, the DOM parser in the browser ignores the original closing quote/double quote of the variable. This is true as of Chrome 81.0.4044.138 & Firefox 76.0.1
  • The effect of the closing quote being ignored is that the rest of the HTML page/DOM is considered part of the string for the variable.
  • It is possible to close the now unclosed variable using another comment tag. This suggests that to the DOM parser the closing quote has been commented out.
  • The risk is generally being able to control/escape a JavaScript variable, loosely similar to a XSS injection resulting in closing a JavaScript variable and writing arbitrary JavaScript afterwards.
  • Normally in a JavaScript variable, short of using a closing JavaScript tag or a quote/double quote, no other HTML tags or syntax is significant and should be recognized as only string data. Often frameworks/parsers/applications in the wild will insert untrusted data into JavaScript variables under this pretense.
Flags: sec-bounty?
Group: firefox-core-security
Component: Other → Security
Product: Websites → Firefox
Version: unspecified → 76 Branch
Attached file repro steps
thought i'd provide some clarifying details:
Attached file repro
thought i'd provide some clarifying details:

Given that apparently you published this report publicly already is there any point in keeping this bug hidden?

Group: firefox-core-security, websites-security → core-security
Type: task → defect
Component: Security → DOM: HTML Parser
Flags: needinfo?(tsangtmc)
Product: Firefox → Core

Glad to take it down the article until further notice if its a problem. I have done so in the mean time.
I would recommend the discussion of it the bug at least be hidden. The instance of the problem is one thing, but i have not had a opportunity to highlight the nature of it such as if variants exists or if its deeper rooted with higher consequences.

Flags: needinfo?(tsangtmc)

If it helps at all i have had less than 30 views on its, most of it likely from me while editing the article.

Affects chrome too? Is it potentially a spec problem then?

Group: core-security → dom-core-security
Flags: needinfo?(hsivonen)

the spec, from what i can find is not super specific. But this behavior i think violates expectations noted in w3 for HTML.
e.g: "In JavaScript, this code can be expressed legally as follows by ensuring that the apparent ETAGO delimiter does not appear immediately before an SGML name start character" - https://www.w3.org/TR/WD-script-970314#script

____Spec wise:
https://tools.ietf.org/html/rfc7992#section-2.1
"Double quotes (U+0022 QUOTATION MARK: ") are used to quote attribute values unless the HTML specification forbids quoting a particular attribute."

https://tools.ietf.org/html/rfc7992#section-4
"processor emitting HTML from the XML source"
"Double quotes (U+0022 QUOTATION MARK: ") are used to quote attribute values unless the HTML specification forbids quoting a particular attribute."

___if we are looking at w3:
https://www.w3.org/TR/WD-script-970314#script
A conforming parser would treat the "</EM>" data as an end tag and complain that it was an end tag for an element not opened, or perhaps actually close an open element. In any case, it is recognized as markup and not as
In JavaScript, this code can be expressed legally as follows by ensuring that the apparent ETAGO delimiter does not appear immediately before an SGML name start character:

<SCRIPT type="text/javascript">
  document.write ("<EM>This will work<\/EM>")
</SCRIPT>

The JavaScript engine allows the string "<!--" to occur at the start of a SCRIPT element, and ignores further characters until the end of the line. JavaScript interprets "//" as starting a comment extending to the end of the current line. This is needed to hide the string "-->" from the JavaScript parser.
Down-level browsers will ignore the SCRIPT start and end tags and interpret the "<!--" string as the start of an SGML comment. In this way the contents of the SCRIPT element are hidden within an SGML comment.

This specification reserves syntax for the future support of script macros in HTML CDATA attributes. The intention is to allow attributes to be set depending on the properties of objects that appear earlier on the page. The syntax is:

___other details:
Also keep in mind this isnt just about xss, its also concerns such as being able to commend out significant JS. e.g:
<html>
<head>
<title>POC</title>
<script>
function get_data()
{
alert(document.getElementById("somedata").innerHTML);
}
</script>
</head>
<body >

<h1 id="somedata">
bad_jello
</h1>
<button onclick="get_data()"> Do some action with data, just do a alert for now
</button>

<script>
//say this variable was data from a url paremter which can be attacker controlled
var variable_that_contains_untrusted_string_data ="<!-- <script>";

//do some security or signigicant related action
var text = document.getElementById("somedata").innerHTML;
text = text.replace("bad_", "good-");
document.getElementById("somedata").innerHTML = text;
</script>

</body>
</html>
attribute = "... &{ macro body }; ... "

The specs you referenced are obsolete. Modern browsers are using the following:
https://html.spec.whatwg.org/multipage/parsing.html (I don't know if that makes a difference for this bug, but I'm suspicious that both Chrome and Firefox behave the same here and suspect that this is something that differs from the old specs)

This is not a bug. Please check the spec from https://html.spec.whatwg.org/#script-data-state onwards.

It should be common knowledge that you should never, ever echo a user-provided string in a JavaScript string literal embedded on an HTML page without escaping the less-than sign. See the note at the start of https://html.spec.whatwg.org/#restrictions-for-contents-of-script-elements for a more constrained set of cases to escape if you insist on not escaping every instance of the less-than sign.

Status: UNCONFIRMED → RESOLVED
Closed: 6 years ago
Flags: needinfo?(hsivonen)
Resolution: --- → INVALID

huh - i stand corrected. thanks everyone for your time.
I don't quite agree that a special side case is common knowledge, but since its in the spec the bug is really on frameworks who allow this.

Thanks again

Flags: sec-bounty? → sec-bounty-
Group: dom-core-security
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: