Closed Bug 490790 (CVE-2010-0182) Opened 11 years ago Closed 10 years ago

XMLDocument::load() doesn't check nsIContentPolicy

Categories

(Core :: XML, defect)

defect
Not set

Tracking

()

VERIFIED FIXED
Tracking Status
blocking2.0 --- final+
status1.9.2 --- .2-fixed
status1.9.1 --- .9-fixed

People

(Reporter: gaubugzilla, Assigned: sicking)

References

(Blocks 1 open bug)

Details

(Keywords: verified1.9.1, verified1.9.2, Whiteboard: [sg:low+])

Attachments

(1 file, 1 obsolete file)

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.
All kinds of things use content policies, and some of them are security related.
Blocks: CSP
Whiteboard: [sg:low+]
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.
Assignee: nobody → jonas
blocking2.0: --- → final
Summary: XMLDocument::load() doesn't check content policies → XMLDocument::load() doesn't check nsIContentPolicy
Attached patch Patch to fix (obsolete) — Splinter Review
Wladimir, it would be great if you wanna try to run with this patch to see if things work properly.
Attachment #429031 - Flags: review?(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!
Attachment #429031 - Flags: review?(dveditz) → review+
I don't think I will be able to test before the patch lands.
Attached patch Patch v2Splinter Review
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.
Attachment #429031 - Attachment is obsolete: true
Attachment #429272 - Flags: review?(dveditz)
Comment on attachment 429272 [details] [diff] [review]
Patch v2

r=dveditz
Attachment #429272 - Flags: review?(dveditz) → review+
Checked in. Thanks for finding this!

http://hg.mozilla.org/mozilla-central/rev/ed1612a1ffa8
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Attachment #429272 - Flags: approval1.9.2.2?
Attachment #429272 - Flags: approval1.9.1.9?
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
Attachment #429272 - Flags: approval1.9.2.2?
Attachment #429272 - Flags: approval1.9.2.2+
Attachment #429272 - Flags: approval1.9.1.9?
Attachment #429272 - Flags: approval1.9.1.9+
Is there a testcase or some sort of manual reproduction steps for this?
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.
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.
Status: RESOLVED → VERIFIED
Adding verified1.9.1 and verified1.9.2 keywords.
Flags: wanted1.9.0.x+
Alias: CVE-2010-0182
Group: core-security
See Also: → 1057518
You need to log in before you can comment on or make changes to this bug.