Closed Bug 1057518 Opened 8 years ago Closed 8 years ago

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

Categories

(Core :: XML, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla34

People

(Reporter: tanvi, Assigned: reuseme2600)

References

Details

(Keywords: addon-compat)

Attachments

(1 file, 2 obsolete files)

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.
Attached patch Bug1057518-08-22-14.patch (obsolete) — Splinter Review
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-
Attached patch Bug1057518-08-29-14.patch (obsolete) — Splinter Review
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+
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
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
Depends on: 1084513
Depends on: 1085452
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.