Add an assertion to XMLDocument to ensure that that callingDoc and the doc have the same principal

RESOLVED FIXED in mozilla34

Status

()

Core
XML
RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: tanvi, Assigned: reuseme2600)

Tracking

({addon-compat})

unspecified
mozilla34
addon-compat
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

4 years ago
In dom/xml/XMLDocument.cpp :: Load(), we call content policies with the callingDoc, but the current documents principal. 
https://mxr.mozilla.org/mozilla-central/source/dom/xml/XMLDocument.cpp#292

Per Jonas, the principal and the callingDoc's principal should be the same.  This method takes an existing document that has content in it and loads new content in it; it doesn't return a new document.  Hence the principal of ther caller should be the same as the principal of the current document.

This bug is to add an assertion to ensure they are the same.  Note that this might cause an addon compat issue.  Addons should be using xmlhttprequest instead of xmldocument.
(Reporter)

Comment 1

4 years ago
Created attachment 8477613 [details] [diff] [review]
Bug1057518-08-22-14.patch
Attachment #8477613 - Flags: review?(jonas)
Comment on attachment 8477613 [details] [diff] [review]
Bug1057518-08-22-14.patch

Review of attachment 8477613 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/xml/XMLDocument.cpp
@@ +304,5 @@
>  
>    nsCOMPtr<nsIDocument> callingDoc = GetEntryDocument();
> +  nsCOMPtr<nsIPrincipal> principal = NodePrincipal();
> +  NS_ASSERTION(callingDoc->NodePrincipal() == principal,
> +              "callingDoc's Principal and doc's Principal should be the same");

Rather than asserting, you should throw NS_ERROR_UNEXPECTED, and put an error message in the developer console, if this happens.

Here's an easy example of how to log something to the error console:

http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsDocument.cpp#9920
Attachment #8477613 - Flags: review?(jonas) → review-
(Reporter)

Comment 3

4 years ago
Created attachment 8481628 [details] [diff] [review]
Bug1057518-08-29-14.patch

Updated patch and push to try: https://tbpl.mozilla.org/?tree=Try&rev=e144aa7f0594
Attachment #8481628 - Flags: review?(jonas)
Comment on attachment 8481628 [details] [diff] [review]
Bug1057518-08-29-14.patch

Review of attachment 8481628 [details] [diff] [review]:
-----------------------------------------------------------------

Great!

::: dom/locales/en-US/chrome/dom/dom.properties
@@ +207,5 @@
>  KeyNameZoomWarning=KeyboardEvent.key value "Zoom" is obsolete and will be renamed to "ZoomToggle". For more help https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent.key
>  # LOCALIZATION NOTE: Do not translate "KeyboardEvent.key" and "Dead".
>  KeyNameDeadKeysWarning=KeyboardEvent.key values starting with "Dead" are obsolete and will be merged into just "Dead". For more help https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent.key
>  ImportXULIntoContentWarning=Importing XUL nodes into a content document is deprecated. This functionality may be removed soon.
> +XMLDocumentLoadPrincipalMismatch=Use of document.load forbidden on documents that come from other Windows. Only the Window in which a document was created is allowed to call document.load on that Window. Or use XMLHttpRequest instead.

Some minor errors here (sorry, I think they are my fault)

Use of document.load forbidden on Documents that come from other Windows. Only the Window in which a Document was created is allowed to call .load on that Document. Preferably, use XMLHttpRequest instead.
Attachment #8481628 - Flags: review?(jonas) → review+
(Reporter)

Comment 5

4 years ago
Created attachment 8481656 [details] [diff] [review]
Bug1057518-08-29-14B.patch

Text updated and carrying over Jonas' r+.
Attachment #8477613 - Attachment is obsolete: true
Attachment #8481628 - Attachment is obsolete: true
Attachment #8481656 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/fe0200fd2d3a
Assignee: nobody → tvyas
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34

Updated

4 years ago
Depends on: 1084513

Comment 8

3 years ago
Was this intended to break simple code such as

var xslt = document.implementation.createDocument("", "", null);
xslt.async = false;
xslt.load(someurlonsamesite);
You need to log in before you can comment on or make changes to this bug.