Closed Bug 770684 (CVE-2012-3975) Opened 8 years ago Closed 8 years ago

DOMParser does subresource loads when used from privileged code


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

16 Branch
Windows XP
Not set



Tracking Status
firefox14 + wontfix
firefox15 + verified
firefox16 + verified
firefox17 + verified
firefox-esr10 --- unaffected


(Reporter: vsemozhetbyt, Assigned: smaug)



(Keywords: regression, sec-moderate, Whiteboard: [advisory-tracking+])


(2 files, 1 obsolete file)

If DOMParser parses HTML string to DOM, should not it omit not only script processing but also recourse loading (as the pure XHR does with responseType = "document")?

I run this code from JavaScript Editor (Scratchpad, in browser context):

var parser = new DOMParser();
var xhr = new XMLHttpRequest();
xhr.mozBackgroundRequest = true;"GET", "", true);
xhr.timeout = 10000; |= Components.interfaces.nsIRequest.LOAD_BYPASS_CACHE;
xhr.onload = function() {
	var doc = parser.parseFromString(this.responseText, "text/html");
	var title = doc.evaluate("//title", doc, null, XPathResult.FIRST_ORDERED_NODE_TYPE, null).singleNodeValue;
	if (title) {
	else {
		alert("Parsing error");
xhr.ontimeout = function() {
xhr.onerror = function() {
	alert("HTTP error");

HttpFox ( shows additional recourse loading. Here some HTTP logs for three sites:

Started, Time, Sent, Received, Method, Result, Type, URL

00:00:01.221	1.197	603		316		GET	200		text/html
00:00:02.492	0.171	332		(34312)	GET	(Cache)	text/css
00:00:02.548	0.143	321		(824)	GET	(Cache)	text/css
00:00:02.618	0.464	968		201		GET	303		Redirect to: /dcso6de4r0000082npfcmh4rf_4b1e/njs.gif?dcsredirect=108&dcstlh=1298194945&dcstlv=1298194945&dcsuri=/nojavascript&WT.js=No&
00:00:03.383	0.622	1050	327		GET	200		image/gif

00:00:02.218	0.163	893	6678	GET	200		text/html
00:00:02.470	0.582	844	21059	GET	200		text/css
00:00:02.527	0.487	859	8094	GET	200		image/png
00:00:02.581	0.461	864	613		GET	200		image/gif
00:00:02.639	0.377	865	613		GET	200		image/gif
00:00:02.698	0.112	853	(165)	GET	(Cache)	image/gif
00:00:02.756	0.324	352	231		GET	200		image/gif

00:00:18.897	0.283	1433	243		GET	200		text/html
00:00:19.236	0.339	335		(28286)	GET	(Cache)	text/css
00:00:19.290	0.448	597		188		GET	302		Redirect to:;tile=2;sz=1008x150,1008x200,1008x30,9x1;p=t;p=top;ct=com;bpx=1;ab=a;ka=0;ord=717635184750?
00:00:19.344	0.261	346		(96)	GET	(Cache)	text/css
00:00:19.396	0.340	582		188		GET	302		Redirect to:;tile=5;sz=1008x60,1008x66,7x1;p=ns;ct=com;bpx=1;ab=a;ka=0;ord=717635184750?
00:00:19.449	0.288	580		188		GET	302		Redirect to:;tile=3;sz=300x250,11x1;p=tr;p=tc;ct=com;bpx=1;ab=a;ka=0;ord=717635184750?
00:00:19.504	0.146	425		220		GET	200		image/gif
00:00:19.899	0.418	328		(43)	GET	(Cache)	image/gif
00:00:19.957	0.362	328		(43)	GET	(Cache)	image/gif
00:00:20.015	0.305	328	(	43)		GET	(Cache)	image/gif

Why DOMParser needs these files for just DOM parsing?
It shouldn't load those files.

(Btw, why do you parse responseText and not just load HTML using XHR?)
Olli Pettay

Because I have a problem with some sites which I describe in this bug:

And I have been advised to use DOMParser in my extension untill this bug will be fixed.
Sorry, wrong link. See:
I'm not convinced that this is a bug.  If you parse the markup as HTML, you can adopt the nodes into the document and they should behave like normal HTML nodes.  It seems weird to only kick off the loads on adoption.
Kyle Huey

But DOMParser does not load all the images and so on. Just some.
Kyle Huey

And I think we must discern page rendering and DOM parsing. For background parsing we don't need any image loading or styles.
If I run the code from comment 0 in the scratchpad, I get an exception because is undefined, as expected.  I assume that the "in browser context" bit hides some crucial part of the steps to reproduce?  I'd really appreciate actually being told what that is.

Kyle, DOMParser is not supposed to do subresource loads.  See bug 421228.  ;)
2 Boris Zbarsky

See the part "A Word about Scopes", the last paragraph.
Ah, looks like the "" pref needs to be set to true.

This is a bad bug in the patch for bug 102699.  Before that patch, the only codepath that could lead to parsing looked like this, in order:

1)  Create a document with the DOMParser's mOriginalPrincipal.
2)  Call EnableXULXBL() on the document if needed
3)  Call StartDocumentLoad()
4)  Set the document's base URI
5)  Reset the document's principal to mPrincipal.
6)  Feed data into the parser.

That sequence of steps was pretty clearly documented (at least in terms of the whole principal dance) and _very_ critical.

When that bug was fixed, the XML codepath stayed as above, but HTML codepath was written more like this:

1)  Create a document with the DOMParser's mOriginalPrincipal.
2)  Feed data into the parser.
3)  Call EnableXULXBL() on the document if needed
4)  Set the document's base URI
5)  Reset the document's principal to mPrincipal.

But the whole point of resetting to mPrincipal is that it MUST happen before any data goes in.  Otherwise you're parsing with the system principal.  Also, this is never calling StartDocumentLoad, so afaict it's not setting up whatever state that would normally set up (e.g. the document URI) the same way as the XML path.  And it's calling EnableXULXBL() too late, of course.  Not like this matters much for text/html.

This bug means that using DOMParser on text/html is pretty unsafe from chrome: It allows whatever string you're parsing to poke any URI it wants, including ones that web content normally can't access.  (On a Unix system it allows at minimum a DoS attack by reading from file:///dev/tty.)

Henri, Olli, can one of you grab this?  Or should I try to?
Group: core-security
Severity: normal → critical
Ever confirmed: true
Our final beta build is tomorrow EOD pacific time. Is there any reason to consider blocking that build on this FF12 regression?
No, though this might be worth taking in a chemspill if we do one for some reason: every extension using DOMParser is basically insecure at the moment.
I think Henri will be back from vacation real soon.
What is the critical part of this bug?
The created document is a data document, so it itself shouldn't load anything.
HTML parser may speculatively load something. (It shouldn't enable speculative loads for data documents)
Or what am I missing here.
Though, I really don't understand why anything is loaded. Document is a data document, so
the data document content policy should prevent all the loads.
Content policy is never consulted for the system principal.  See the CHECK_PRINCIPAL bit in NS_CheckContentLoadPolicy.  So the document is in fact loading things here.
bug 388597 added a bizarre inconsistency to content policy handling.
(Another option would be to not have data document cp at all, but prevent such loads in some other way.)
Blocks: 388597
I guess I've looked at the code enough to write a patch.
Assignee: nobody → bugs
//every extension using DOMParser is basically insecure at the moment

I use it only because of the bug 765620. I just have given a simple testcase in the last comment there. May be somebody can confirm the bug.
Attached patch v1 (obsolete) — Splinter Review
Like this?
Attachment #640106 - Flags: review?(bzbarsky)
Comment on attachment 640106 [details] [diff] [review]

Unfortunately this seems to leak. We're not supposed to call
StartDocumentLoad in this case for HTML documents.
Attachment #640106 - Flags: review?(bzbarsky)
Attached patch v2Splinter Review
Attachment #640106 - Attachment is obsolete: true
Attached patch simple approachSplinter Review
Attachment #640187 - Flags: review?(bzbarsky)
Comment on attachment 640187 [details] [diff] [review]
simple approach

Attachment #640187 - Flags: review?(bzbarsky) → review+
Closed: 8 years ago
Resolution: --- → FIXED
Tracking for FF14, for possible 14.0.1 consideration.
Thank you, people.
(In reply to Olli Pettay [:smaug] (limited connectivity July 26-29) from comment #24)

Is this low risk enough to uplift to FF15 on mozilla-beta?
(In reply to Alex Keybl [:akeybl] from comment #27)
> (In reply to Olli Pettay [:smaug] (limited connectivity July 26-29) from
> comment #24)
> >
> Is this low risk enough to uplift to FF15 on mozilla-beta?

Seems so to me. (Looks like smaug is away right now.)
Comment on attachment 640187 [details] [diff] [review]
simple approach

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 102699, bug 388597
User impact if declined: security issues 
Testing completed (on m-c, etc.): Landed m-c 2012-07-09 
Risk to taking this patch (and alternatives if risky): Should be very low risk
String or UUID changes made by this patch: NA
Attachment #640187 - Flags: approval-mozilla-beta?
Comment on attachment 640187 [details] [diff] [review]
simple approach

[Triage Comment]
Low risk sg:moderate fix - let's take this for the next beta (going to build tomorrow). Please land as soon as possible.
Attachment #640187 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Summary: Why DOMParser should load .css and images for parsing HTML? → DOMParser does subresource loads when used from privileged code
Whiteboard: [advisory-tracking+]
Alias: CVE-2012-3975
Reporter, can you see if this is fixed for you now? Can you try the latest Firefox 15 release, 16 beta, and 17 aurora? Thanks.
Yes, it is fixed in these versions. Thank you.
Thanks vvsemozhetbyt@. Marking this bug verified based on comment 33.
Group: core-security
You need to log in before you can comment on or make changes to this bug.