412257 - document.createComment rejects "--" in violation of the specification

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Amnon Aaronsohn]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9b3pre) Gecko/2008011304 Minefield/3.0b3pre
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9b3pre) Gecko/2008011304 Minefield/3.0b3pre

When you give document.createComment a string which can't appear in a comment according to XML rules, the method throws an exception.
This is non-standard behavior. See DOM level 3 core:

"""
No lexical check is done on the content of a comment and it is therefore possible to have the character sequence "--" (double-hyphen) in the content, which is illegal in a comment per section 2.5 of [XML 1.0]. The presence of this character sequence must generate a fatal error during serialization. 
"""


Reproducible: Always

Steps to Reproduce:
1.Type document.createComment("--") in the JS console

Actual Results:  
An exception is thrown:

Error: uncaught exception: [Exception... "String contains an invalid character"  code: "5" nsresult: "0x80530005 (NS_ERROR_DOM_INVALID_CHARACTER_ERR)"  location: "javascript:%20document.createComment("--") Line: 1"]

Expected Results:  
A Comment node containing the string "--" should be created

Comment 1

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

Still present in Firefox 4.0 RC, and still a bug per the current DOM Core spec:

http://dvcs.w3.org/hg/domcore/raw-file/tip/Overview.html#dom-document-createcomment

All other browsers tested (IE 9 RC, Chrome 11 dev, Opera 11) create the comment successfully.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Yeah, we should probably undo the createComment change from bug 352728.

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

Same for CDATASection, I think.

Comment 5

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

Attachment: Patch v1

This might well still cause test failures, but they should be simple to fix.  Try: https://tbpl.mozilla.org/?tree=Try&rev=3ea71ee9f5c5

Comment 6

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 633110 [details] [diff] [review]
Patch v1

r=me

Comment 7

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/bc241a81c6a5

Comment 8

[Pomax]

should https://hg.mozilla.org/integration/mozilla-inbound/rev/bc241a81c6a5#l2.65 read "should" rather than "shouldn't"?

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/bc241a81c6a5

Comment 10

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

(In reply to Mike "Pomax" Kamermans from comment #8)
> should
> https://hg.mozilla.org/integration/mozilla-inbound/rev/bc241a81c6a5#l2.65
> read "should" rather than "shouldn't"?

Oops -- thanks!

https://hg.mozilla.org/integration/mozilla-inbound/rev/4795650d4153

Comment 11

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/4795650d4153

Comment 12

[Ilguiz Latypov]

The current document.createComment() goes against the spec by writing to the document. This effect can be observed by creating a comment with a comment-closing string followed by a newline, selecting Edit As HTML.. in Inspector, observing the raw fake comment closure, then folding the edit inset by clicking outside of it. An alert pops up. I do not know if any intermediary layer (such as Edit as HTML editor) allowed the unexpected interpretation or if this was a result of createComment() writing into the to-be-interpreted document.

document.getElementById("dom-document-createcomment").append(document.createComment("foobar-->\x0D\x0A<img src=. onerror=alert(22)>"))

Creating a text node appears to behave properly. The following will add content safely (i.e. preventing it from further interpretation). Clicking Edit as HTML will show the angle brackets of the embedded img tag as HTML-encoded < and >.

document.getElementById("dom-document-createcomment").append(document.createTextNode("foobar\x0D\x0A<img src=. onerror=alert(22)>"))

The same "attack" can be observed in Chrome. This finding is inspired by an Angular's createComment() XSS vulnerability that lacks a proof-of-concept. I did not try creating one.

Comment 13

[Ilguiz Latypov]

I used this site's browser console for my experiment,

https://dom.spec.whatwg.org/#dom-document-createcomment

(many other sites disable inline javascript with their CSP).

Comment 14

[Ilguiz Latypov]

Attachment: test-create-comment.html

Clicking "Show DOM Properties" on the injected comment shows a proper encapsulation of the entire comment, but clicking "Edit as HTML" on the surrounding tag, then closing the editor's inset re-interprets the comment. So I guess it's a bug a part of which has to do with proper encapsulation (as createTextNode() does) and another part has to do with the HTML editor.

Attaching the self-contained site test-create-comment.html to be used follows,

http://localhost:8000/test-create-comment.html#alert(33)