Bug 770684 (CVE-2012-3975)

DOMParser does subresource loads when used from privileged code




DOM: Core & HTML
5 years ago
5 years ago


(Reporter: vsemozhetbyt, Assigned: smaug)


({regression, sec-moderate})

16 Branch
Windows XP
regression, sec-moderate
Dependency tree / graph

Firefox Tracking Flags

(firefox14+ wontfix, firefox15+ verified, firefox16+ verified, firefox17+ verified, firefox-esr10 unaffected)


(Whiteboard: [advisory-tracking+])


(2 attachments, 1 obsolete attachment)



5 years ago
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;
xhr.open("GET", "https://addons.mozilla.org/en-US/firefox/", true);
xhr.timeout = 10000;
xhr.channel.loadFlags |= 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 (https://addons.mozilla.org/ru/firefox/addon/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	https://addons.mozilla.org/en-US/firefox/
00:00:02.492	0.171	332		(34312)	GET	(Cache)	text/css	https://static-ssl-cdn.addons.mozilla.net/media/css/zamboni/impala-min.css?build=2628081
00:00:02.548	0.143	321		(824)	GET	(Cache)	text/css	https://static-ssl-cdn.addons.mozilla.net/media/css/impala/nojs.css?b=78073c4
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&WT.tv=8.6.2	https://statse.webtrendslive.com/dcso6de4r0000082npfcmh4rf_4b1e/njs.gif?dcsuri=/nojavascript&WT.js=No&WT.tv=8.6.2
00:00:03.383	0.622	1050	327		GET	200		image/gif	https://statse.webtrendslive.com/dcso6de4r0000082npfcmh4rf_4b1e/njs.gif?dcsredirect=108&dcstlh=1298194945&dcstlv=1298194945&dcsuri=/nojavascript&WT.js=No&WT.tv=8.6.2


00:00:02.218	0.163	893	6678	GET	200		text/html	http://www.urbandictionary.com/
00:00:02.470	0.582	844	21059	GET	200		text/css	http://static2.urbandictionary.com/rel-30747e0/assets/base-datauri.css
00:00:02.527	0.487	859	8094	GET	200		image/png	http://static0.urbandictionary.com/rel-30747e0/images/logo-holiday.png
00:00:02.581	0.461	864	613		GET	200		image/gif	http://static2.urbandictionary.com/rel-30747e0/images/left_browse_arrow.gif
00:00:02.639	0.377	865	613		GET	200		image/gif	http://static1.urbandictionary.com/rel-30747e0/images/right_browse_arrow.gif
00:00:02.698	0.112	853	(165)	GET	(Cache)	image/gif	http://static1.urbandictionary.com/rel-30747e0/images/quotes.gif
00:00:02.756	0.324	352	231		GET	200		image/gif	http://pixel.quantserve.com/pixel/p-77H27_lnOeCCI.gif


00:00:18.897	0.283	1433	243		GET	200		text/html	http://www.imdb.com/
00:00:19.236	0.339	335		(28286)	GET	(Cache)	text/css	http://i.media-imdb.com/images/SF3997b2af7161e82df725bc7ca86f84ce/css/min/consumerhome.css
00:00:19.290	0.448	597		188		GET	302		Redirect to: http://s0.2mdn.net/viewad/817-grey.gif	http://ad.doubleclick.net/ad/imdb2.consumer.homepage/;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	http://i.media-imdb.com/images/SF52e6b9f11712d3ec552179f6c869b63a/css2/site/consumer-navbar-no-js.css
00:00:19.396	0.340	582		188		GET	302		Redirect to: http://s0.2mdn.net/viewad/817-grey.gif	http://ad.doubleclick.net/ad/imdb2.consumer.homepage/;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: http://s0.2mdn.net/viewad/817-grey.gif	http://ad.doubleclick.net/ad/imdb2.consumer.homepage/;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	http://b.scorecardresearch.com/p?c1=2&c2=6034961&c3=&c4=http%3A%2F%2Fwww.imdb.com%2F&c5=c6=&15=&cj=1
00:00:19.899	0.418	328		(43)	GET	(Cache)	image/gif	http://s0.2mdn.net/viewad/817-grey.gif
00:00:19.957	0.362	328		(43)	GET	(Cache)	image/gif	http://s0.2mdn.net/viewad/817-grey.gif
00:00:20.015	0.305	328	(	43)		GET	(Cache)	image/gif	http://s0.2mdn.net/viewad/817-grey.gif

Why DOMParser needs these files for just DOM parsing?

Comment 1

5 years ago
It shouldn't load those files.

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

Comment 2

5 years ago
Olli Pettay

Because I have a problem with some sites which I describe in this bug: https://bugzilla.mozilla.org/show_bug.cgi?id=770684

And I have been advised to use DOMParser in my extension untill this bug will be fixed.

Comment 3

5 years ago
Sorry, wrong link. See: https://bugzilla.mozilla.org/show_bug.cgi?id=765620
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.

Comment 5

5 years ago
Kyle Huey

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

Comment 6

5 years ago
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 xhr.channel 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.  ;)

Comment 8

5 years ago
2 Boris Zbarsky


See the part "A Word about Scopes", the last paragraph.
Ah, looks like the "devtools.chrome.enable" 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
tracking-firefox14: --- → ?
tracking-firefox15: --- → ?
tracking-firefox16: --- → ?
Ever confirmed: true
Blocks: 102699
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.

Comment 12

5 years ago
I think Henri will be back from vacation real soon.

Comment 13

5 years ago
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.

Comment 14

5 years ago
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.

Comment 16

5 years ago
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

Comment 17

5 years ago
I guess I've looked at the code enough to write a patch.
Assignee: nobody → bugs

Comment 18

5 years ago
//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.

Comment 19

5 years ago
Created attachment 640106 [details] [diff] [review]

Like this?
Attachment #640106 - Flags: review?(bzbarsky)

Comment 20

5 years ago
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)

Comment 21

5 years ago
Created attachment 640113 [details] [diff] [review]
Attachment #640106 - Attachment is obsolete: true

Comment 22

5 years ago
Created attachment 640187 [details] [diff] [review]
simple approach
Attachment #640187 - Flags: review?(bzbarsky)
Comment on attachment 640187 [details] [diff] [review]
simple approach

Attachment #640187 - Flags: review?(bzbarsky) → review+

Comment 24

5 years ago
Last Resolved: 5 years ago
Resolution: --- → FIXED
Tracking for FF14, for possible 14.0.1 consideration.
status-firefox14: --- → affected
status-firefox15: --- → affected
status-firefox16: --- → fixed
tracking-firefox14: ? → +
tracking-firefox15: ? → +
tracking-firefox16: ? → +

Comment 26

5 years ago
Thank you, people.
status-firefox-esr10: --- → unaffected
status-firefox17: --- → fixed
tracking-firefox17: --- → +
Keywords: regression, sec-moderate


5 years ago
status-firefox14: affected → wontfix
(In reply to Olli Pettay [:smaug] (limited connectivity July 26-29) from comment #24)
> https://hg.mozilla.org/mozilla-central/rev/15f4ed9607b3

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)
> > https://hg.mozilla.org/mozilla-central/rev/15f4ed9607b3
> Is this low risk enough to uplift to FF15 on mozilla-beta?

Seems so to me. (Looks like smaug is away right now.)

Comment 29

5 years ago
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+

Comment 31

5 years ago
status-firefox15: affected → fixed


5 years ago
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.

Comment 33

5 years ago
Yes, it is fixed in these versions. Thank you.
Thanks vvsemozhetbyt@. Marking this bug verified based on comment 33.
status-firefox15: fixed → verified
status-firefox16: fixed → verified
status-firefox17: fixed → verified
Group: core-security
You need to log in before you can comment on or make changes to this bug.