490790 - (CVE-2010-0182) XMLDocument::load() doesn't check nsIContentPolicy

Status: VERIFIED FIXED

(Core :: XML, defect)

Description

[Wladimir Palant]

nsXMLDocument::load() will do same-origin checks on the URL to be loaded but won't check content policies. I think it should call content policies with TYPE_XMLHTTPREQUEST and use calling document as context.

Comment 1

[Daniel Veditz [:dveditz]]

All kinds of things use content policies, and some of them are security related.

Comment 2

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

I guess I can take this. Not sure if using TYPE_XMLHTTPREQUEST is ok. I agree it's conceptually the same, i just worry that people expect to be able to get to an XHR object.

Comment 3

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Attachment: Patch to fix

Wladimir, it would be great if you wanna try to run with this patch to see if things work properly.

Comment 4

[Daniel Veditz [:dveditz]]

Comment on attachment 429031 [details] [diff] [review]
Patch to fix

r=dveditz

In the old code, lack of codebase is not equal to system principal!

Comment 5

[Wladimir Palant]

I don't think I will be able to test before the patch lands.

Comment 6

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Attachment: Patch v2

Turns out that the first patch breaks a bunch of mochitests. The problem is that if we use the document itself as context, the nsDataDocumentContentPolicy content policy blocks the load. Instead we should using the *calling* document as the context.

This patch does that.

Comment 7

[Daniel Veditz [:dveditz]]

Comment on attachment 429272 [details] [diff] [review]
Patch v2

r=dveditz

Comment 8

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Checked in. Thanks for finding this!

http://hg.mozilla.org/mozilla-central/rev/ed1612a1ffa8

Comment 9

[Daniel Veditz [:dveditz]]

Comment on attachment 429272 [details] [diff] [review]
Patch v2

Approved for 1.9.2.2, and 1.9.1.9, a=dveditz for release-drivers

Comment 10

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Checked in to 1.9.1 branch for 1.9.1.9
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/4d4f3726d884

And to 1.9.2 branch for 1.9.2.2
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/46d94cbc195f

Comment 11

[Al Billings [:abillings - ex-MoCo]]

Is there a testcase or some sort of manual reproduction steps for this?

Comment 12

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

I don't think we have any tests for contentpolicies. The best thing would be if Wladimir could check as I believe he has good tests for contentpolicies.

Comment 13

[Wladimir Palant]

I added this test to my test suite: https://hg.adblockplus.org/adblockplus/rev/235e750203cb (tested without the version check however, will remove it in the test suite once the releases are out).

Fails in:

Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.1.9pre) Gecko/20100307 Shiretoko/3.5.9pre
Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.2.2pre) Gecko/20100307 Namoroka/3.6.2pre
Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a3pre) Gecko/20100301 Minefield/3.7a3pre

Succeeds in:

Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.1.9pre) Gecko/20100315 Shiretoko/3.5.9pre
Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.2.2pre) Gecko/20100315 Namoroka/3.6.2pre
Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a3pre) Gecko/20100312 Minefield/3.7a3pre

Marking as verified.

Comment 14

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Thanks Wladimir!

Comment 15

[Al Billings [:abillings - ex-MoCo]]

Adding verified1.9.1 and verified1.9.2 keywords.