From bc34e96b923ced10b99bd0553921d217bdc61b66 Mon Sep 17 00:00:00 2001 From: "mozilla.mano%sent.com" Date: Sat, 17 Feb 2007 04:12:28 +0000 Subject: [PATCH] Bug 342485 - Replace calls to CheckLoadURI() with calls to CheckLoadURIWithPrincipal(). r=gavin, sayrer (for tests). --- browser/base/content/browser.js | 27 ++--- browser/base/content/nsContextMenu.js | 33 +++--- browser/base/content/utilityOverlay.js | 76 +++++++++++++ toolkit/content/Makefile.in | 4 + toolkit/content/contentAreaUtils.js | 105 ++++-------------- .../tests/unit/test_contentAreaUtils.js | 86 ++++++++++++++ toolkit/content/widgets/browser.xml | 4 + toolkit/content/widgets/tabbrowser.xml | 35 +++--- toolkit/content/widgets/text.xml | 18 +-- 9 files changed, 251 insertions(+), 137 deletions(-) create mode 100644 toolkit/content/tests/unit/test_contentAreaUtils.js diff --git a/browser/base/content/browser.js b/browser/base/content/browser.js index 2900652bd98..f8a46ae96e4 100644 --- a/browser/base/content/browser.js +++ b/browser/base/content/browser.js @@ -2445,7 +2445,8 @@ var urlbarObserver = { try { gURLBar.value = url; const nsIScriptSecMan = Components.interfaces.nsIScriptSecurityManager; - urlSecurityCheck(gURLBar.value, gBrowser.currentURI.spec, + urlSecurityCheck(gURLBar.value, + gBrowser.contentPrincipal, nsIScriptSecMan.DISALLOW_INHERIT_PRINCIPAL); handleURLBarCommand(); } catch (ex) {} @@ -2859,10 +2860,9 @@ var goButtonObserver = { var url = getShortcutOrURI(draggedText, postData); try { getBrowser().dragDropSecurityCheck(aEvent, aDragSession, url); - - const nsIScriptSecMan = Components.interfaces.nsIScriptSecurityManager; - urlSecurityCheck(url, gBrowser.currentURI.spec, - nsIScriptSecMan.DISALLOW_INHERIT_PRINCIPAL); + urlSecurityCheck(url, + gBrowser.contentPrincipal, + Ci.nsIScriptSecurityManager.DISALLOW_INHERIT_PRINCIPAL); loadURI(url, null, postData.value, true); } catch (ex) {} }, @@ -4520,7 +4520,7 @@ function asyncOpenWebPanel(event) function handleLinkClick(event, href, linkNode) { - var docURL = event.target.ownerDocument.location.href; + var doc = event.target.ownerDocument; switch (event.button) { case 0: // if left button clicked @@ -4529,7 +4529,7 @@ function handleLinkClick(event, href, linkNode) #else if (event.ctrlKey) { #endif - openNewTabWith(href, docURL, null, event, false); + openNewTabWith(href, doc, null, event, false); event.stopPropagation(); return true; } @@ -4544,14 +4544,14 @@ function handleLinkClick(event, href, linkNode) } if (event.shiftKey) { - openNewWindowWith(href, docURL, null, false); + openNewWindowWith(href, doc, null, false); event.stopPropagation(); return true; } if (event.altKey) { saveURL(href, linkNode ? gatherTextUnder(linkNode) : "", null, true, - true, makeURI(docURL, event.target.ownerDocument.characterSet)); + true, doc.documentURIObject); return true; } @@ -4565,9 +4565,9 @@ function handleLinkClick(event, href, linkNode) tab = true; } if (tab) - openNewTabWith(href, docURL, null, event, false); + openNewTabWith(href, doc, null, event, false); else - openNewWindowWith(href, docURL, null, false); + openNewWindowWith(href, doc, null, false); event.stopPropagation(); return true; } @@ -5437,7 +5437,7 @@ var FeedHandler = { // preview UI if (!href) href = event.target.getAttribute("feed"); - urlSecurityCheck(href, gBrowser.currentURI.spec, + urlSecurityCheck(href, gBrowser.contentPrincipal, Ci.nsIScriptSecurityManager.DISALLOW_SCRIPT_OR_DATA); var feedURI = makeURI(href, document.characterSet); // Use the feed scheme so X-Moz-Is-Feed will be set @@ -5602,7 +5602,8 @@ var FeedHandler = { var wrapper = event.target; try { - urlSecurityCheck(wrapper.href, gBrowser.currentURI.spec, + urlSecurityCheck(wrapper.href, + gBrowser.contentPrincipal, Ci.nsIScriptSecurityManager.DISALLOW_SCRIPT_OR_DATA); } catch (ex) { diff --git a/browser/base/content/nsContextMenu.js b/browser/base/content/nsContextMenu.js index d38276bcd14..9ebd19cd365 100644 --- a/browser/base/content/nsContextMenu.js +++ b/browser/base/content/nsContextMenu.js @@ -373,10 +373,6 @@ nsContextMenu.prototype = { // Remember the node that was clicked. this.target = aNode; - // Remember the URL of the document containing the node - // for referrer header and for security checks. - this.docURL = aNode.ownerDocument.location.href; - // First, do checks for nodes that never have children. if (this.target.nodeType == Node.ELEMENT_NODE) { // See if the user clicked on an image. @@ -610,12 +606,12 @@ nsContextMenu.prototype = { // Open linked-to URL in a new window. openLink : function () { - openNewWindowWith(this.linkURL, this.docURL, null, false); + openNewWindowWith(this.linkURL, this.target.ownerDocument, null, false); }, // Open linked-to URL in a new tab. openLinkInTab: function() { - openNewTabWith(this.linkURL, this.docURL, null, null, false); + openNewTabWith(this.linkURL, this.target.ownerDocument, null, null, false); }, // Open frame in a new tab. @@ -640,7 +636,7 @@ nsContextMenu.prototype = { var frameURL = this.target.ownerDocument.location.href; try { - urlSecurityCheck(frameURL, this.browser.currentURI.spec, + urlSecurityCheck(frameURL, this.browser.contentPrincipal, Ci.nsIScriptSecurityManager.DISALLOW_SCRIPT); this.browser.loadURI(frameURL); } catch(e) {} @@ -689,15 +685,17 @@ nsContextMenu.prototype = { // Change current window to the URL of the image. viewImage: function(e) { - urlSecurityCheck(this.imageURL, this.browser.currentURI.spec, - Ci.nsIScriptSecurityManager.DISALLOW_SCRIPT ); + urlSecurityCheck(this.imageURL, + this.browser.contentPrincipal, + Ci.nsIScriptSecurityManager.DISALLOW_SCRIPT); openUILink( this.imageURL, e ); }, // Change current window to the URL of the background image. viewBGImage: function(e) { - urlSecurityCheck(this.bgImageURL, this.browser.currentURI.spec, - Ci.nsIScriptSecurityManager.DISALLOW_SCRIPT ); + urlSecurityCheck(this.bgImageURL, + this.browser.contentPrincipal, + Ci.nsIScriptSecurityManager.DISALLOW_SCRIPT); openUILink(this.bgImageURL, e ); }, @@ -728,7 +726,8 @@ nsContextMenu.prototype = { if (this.disableSetDesktopBackground()) return; - urlSecurityCheck(this.target.currentURI.spec, this.docURL); + urlSecurityCheck(this.target.currentURI.spec, + this.target.ownerDocument.nodePrincipal); // Confirm since it's annoying if you hit this accidentally. const kDesktopBackgroundURL = @@ -763,9 +762,10 @@ nsContextMenu.prototype = { // Save URL of clicked-on link. saveLink: function() { - urlSecurityCheck(this.linkURL, this.docURL); + var doc = this.target.ownerDocument; + urlSecurityCheck(this.linkURL, doc.nodePrincipal); saveURL(this.linkURL, this.linkText(), null, true, false, - makeURI(this.docURL, this.target.ownerDocument.characterSet)); + doc.documentURIObject); }, sendLink: function() { @@ -775,9 +775,10 @@ nsContextMenu.prototype = { // Save URL of clicked-on image. saveImage: function() { - urlSecurityCheck(this.imageURL, this.docURL); + var doc = this.target.ownerDocument; + urlSecurityCheck(this.imageURL, doc.nodePrincipal); saveImageURL(this.imageURL, null, "SaveImageTitle", false, - false, makeURI(this.docURL)); + false, doc.documentURIObject); }, sendImage: function() { diff --git a/browser/base/content/utilityOverlay.js b/browser/base/content/utilityOverlay.js index f7b43ada8d9..ca8bc2feecf 100644 --- a/browser/base/content/utilityOverlay.js +++ b/browser/base/content/utilityOverlay.js @@ -503,3 +503,79 @@ function getBrowserFromContentWindow(aContentWindow) } return null; } + + +/** + * openNewTabWith: opens a new tab with the given URL. + * + * @param aURL + * The URL to open (as a string). + * @param aDocument + * The document from which the URL came, or null. This is used to set the + * referrer header and to do a security check of whether the document is + * allowed to reference the URL. If null, there will be no referrer + * header and no security check. + * @param aPostData + * Form POST data, or null. + * @param aEvent + * The triggering event (for the purpose of determining whether to open + * in the background), or null. + * @param aAllowThirdPartyFixup + * If true, then we allow the URL text to be sent to third party services + * (e.g., Google's I Feel Lucky) for interpretation. This parameter may + * be undefined in which case it is treated as false. + */ +function openNewTabWith(aURL, aDocument, aPostData, aEvent, + aAllowThirdPartyFixup) +{ + if (aDocument) + urlSecurityCheck(aURL, aDocument.nodePrincipal); + + var prefSvc = Components.classes["@mozilla.org/preferences-service;1"] + .getService(Components.interfaces.nsIPrefService); + prefSvc = prefSvc.getBranch(null); + + // should we open it in a new tab? + var loadInBackground = true; + try { + loadInBackground = prefSvc.getBoolPref("browser.tabs.loadInBackground"); + } + catch(ex) { + } + + if (aEvent && aEvent.shiftKey) + loadInBackground = !loadInBackground; + + // As in openNewWindowWith(), we want to pass the charset of the + // current document over to a new tab. + var wintype = document.documentElement.getAttribute("windowtype"); + var originCharset; + if (wintype == "navigator:browser") + originCharset = window.content.document.characterSet; + + // open link in new tab + var referrerURI = aDocument ? aDocument.documentURIObject : null; + var browser = top.document.getElementById("content"); + browser.loadOneTab(aURL, referrerURI, originCharset, aPostData, + loadInBackground, aAllowThirdPartyFixup || false); +} + +function openNewWindowWith(aURL, aDocument, aPostData, aAllowThirdPartyFixup) +{ + if (aDocument) + urlSecurityCheck(aURL, aDocument.nodePrincipal); + + // if and only if the current window is a browser window and it has a + // document with a character set, then extract the current charset menu + // setting from the current document and use it to initialize the new browser + // window... + var charsetArg = null; + var wintype = document.documentElement.getAttribute("windowtype"); + if (wintype == "navigator:browser") + charsetArg = "charset=" + window.content.document.characterSet; + + var referrerURI = aDocument ? aDocument.documentURIObject : null; + window.openDialog(getBrowserURL(), "_blank", "chrome,all,dialog=no", + aURL, charsetArg, referrerURI, aPostData, + aAllowThirdPartyFixup); +} diff --git a/toolkit/content/Makefile.in b/toolkit/content/Makefile.in index 72bb4761b01..fb08abf0ac4 100644 --- a/toolkit/content/Makefile.in +++ b/toolkit/content/Makefile.in @@ -47,6 +47,10 @@ include $(DEPTH)/config/autoconf.mk DEFINES += -DMOZ_APP_VERSION=$(MOZ_APP_VERSION) +ifdef ENABLE_TESTS +DIRS += tests +endif + include $(topsrcdir)/config/rules.mk distclean:: diff --git a/toolkit/content/contentAreaUtils.js b/toolkit/content/contentAreaUtils.js index 71fe1131077..b67ba142ef2 100644 --- a/toolkit/content/contentAreaUtils.js +++ b/toolkit/content/contentAreaUtils.js @@ -38,97 +38,38 @@ # # ***** END LICENSE BLOCK ***** - /** - * openNewTabWith: opens a new tab with the given URL. + * urlSecurityCheck: JavaScript wrapper for checkLoadURIWithPrincipal + * and checkLoadURIStrWithPrincipal. + * If |aPrincipal| is not allowed to link to |aURL|, this function throws with + * an error message. * - * @param href The URL to open (as a string). - * @param sourceURL The URL of the document from which the URL came, or null. - * This is used to set the referrer header and to do a security check of whether - * the document as allowed to reference the URL. - * If null, there will be no referrer header and no security check. - * @param postData Form POST data, or null. - * @param event The triggering event (for the purpose of determining whether to open in the background), or null - * @param allowThirdPartyFixup if true, then we allow the URL text to be sent to third party - * services (e.g., Google's I Feel Lucky) for interpretation. This parameter may be undefined in - * which case it is treated as false. - */ -function openNewTabWith(href, sourceURL, postData, event, allowThirdPartyFixup) -{ - if (sourceURL) - urlSecurityCheck(href, sourceURL); - - var prefSvc = Components.classes["@mozilla.org/preferences-service;1"] - .getService(Components.interfaces.nsIPrefService); - prefSvc = prefSvc.getBranch(null); - - // should we open it in a new tab? - var loadInBackground = true; - try { - loadInBackground = prefSvc.getBoolPref("browser.tabs.loadInBackground"); - } - catch(ex) { - } - - if (event && event.shiftKey) - loadInBackground = !loadInBackground; - - // As in openNewWindowWith(), we want to pass the charset of the - // current document over to a new tab. - var wintype = document.documentElement.getAttribute('windowtype'); - var originCharset; - if (wintype == "navigator:browser") - originCharset = window.content.document.characterSet; - - // open link in new tab - var browser = top.document.getElementById("content"); - - var referrerURI = sourceURL ? makeURI(sourceURL) : null; - - browser.loadOneTab(href, referrerURI, originCharset, postData, loadInBackground, - allowThirdPartyFixup || false); -} - -function openNewWindowWith(href, sourceURL, postData, allowThirdPartyFixup) -{ - if (sourceURL) - urlSecurityCheck(href, sourceURL); - - // if and only if the current window is a browser window and it has a document with a character - // set, then extract the current charset menu setting from the current document and use it to - // initialize the new browser window... - var charsetArg = null; - var wintype = document.documentElement.getAttribute('windowtype'); - if (wintype == "navigator:browser") - charsetArg = "charset=" + window.content.document.characterSet; - - var referrerURI = sourceURL ? makeURI(sourceURL) : null; - - window.openDialog(getBrowserURL(), "_blank", "chrome,all,dialog=no", - href, charsetArg, referrerURI, postData, allowThirdPartyFixup); -} - -/** - * urlSecurityCheck: JavaScript wrapper for CheckLoadURIStr. - * If |sourceURL| is not allowed to link to |url|, this function throws with an error message. - * - * @param url The URL a page has linked to. - * @param sourceURL The URL of the document from which the URL came. - * @param flags Flags to be passed to checkLoadURIStr. If undefined, - * nsIScriptSecurityManager.STANDARD will be passed to checkLoadURIStr. + * @param aURL + * The URL a page has linked to. This could be passed either as a string + * or as a nsIURI object. + * @param aPrincipal + * The principal of the document from which aURL came. + * @param aFlags + * Flags to be passed to checkLoadURIStr. If undefined, + * nsIScriptSecurityManager.STANDARD will be passed. */ -function urlSecurityCheck(url, sourceURL, flags) +function urlSecurityCheck(aURL, aPrincipal, aFlags) { - const nsIScriptSecurityManager = Components.interfaces.nsIScriptSecurityManager; + const nsIScriptSecurityManager = + Components.interfaces.nsIScriptSecurityManager; var secMan = Components.classes["@mozilla.org/scriptsecuritymanager;1"] .getService(nsIScriptSecurityManager); - if (flags === undefined) - flags = nsIScriptSecurityManager.STANDARD; + if (aFlags === undefined) + aFlags = nsIScriptSecurityManager.STANDARD; try { - secMan.checkLoadURIStr(sourceURL, url, flags); + if (aURL instanceof Components.interfaces.nsIURI) + secMan.checkLoadURIWithPrincipal(aPrincipal, aURL, aFlags); + else + secMan.checkLoadURIStrWithPrincipal(aPrincipal, aURL, aFlags); } catch (e) { - throw "Load of " + url + " from " + sourceURL + " denied."; + // XXXmano: dump the prinicipal url here too + throw "Load of " + aURL + " denied."; } } diff --git a/toolkit/content/tests/unit/test_contentAreaUtils.js b/toolkit/content/tests/unit/test_contentAreaUtils.js new file mode 100644 index 00000000000..abd501f17d1 --- /dev/null +++ b/toolkit/content/tests/unit/test_contentAreaUtils.js @@ -0,0 +1,86 @@ +/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ +/* ***** BEGIN LICENSE BLOCK ***** + * Version: MPL 1.1/GPL 2.0/LGPL 2.1 + * + * The contents of this file are subject to the Mozilla Public License Version + * 1.1 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * http://www.mozilla.org/MPL/ + * + * Software distributed under the License is distributed on an "AS IS" basis, + * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License + * for the specific language governing rights and limitations under the + * License. + * + * The Original Code is bug 342485 unit test. + * + * The Initial Developer of the Original Code is Mozilla Corporation + * Portions created by the Initial Developer are Copyright (C) 2007 + * the Initial Developer. All Rights Reserved. + * + * Contributor(s): + * Asaf Romano + * + * Alternatively, the contents of this file may be used under the terms of + * either the GNU General Public License Version 2 or later (the "GPL"), or + * the GNU Lesser General Public License Version 2.1 or later (the "LGPL"), + * in which case the provisions of the GPL or the LGPL are applicable instead + * of those above. If you wish to allow use of your version of this file only + * under the terms of either the GPL or the LGPL, and not to allow others to + * use your version of this file under the terms of the MPL, indicate your + * decision by deleting the provisions above and replace them with the notice + * and other provisions required by the GPL or the LGPL. If you do not delete + * the provisions above, a recipient may use your version of this file under + * the terms of any one of the MPL, the GPL or the LGPL. + * + * ***** END LICENSE BLOCK ***** */ + +const Ci = Components.interfaces; +const Cc = Components.classes; +const Cr = Components.results; + +function loadUtilsScript() { + var loader = Cc["@mozilla.org/moz/jssubscript-loader;1"]. + getService(Ci.mozIJSSubScriptLoader); + loader.loadSubScript("chrome://global/content/contentAreaUtils.js"); +} + +function test_urlSecurityCheck() { + var nullPrincipal = Cc["@mozilla.org/nullprincipal;1"]. + createInstance(Ci.nsIPrincipal); + + const HTTP_URI = "http://www.mozilla.org/"; + const CHROME_URI = "chrome://browser/content/browser.xul"; + const DISALLOW_INHERIT_PRINCIPAL = + Ci.nsIScriptSecurityManager.DISALLOW_INHERIT_PRINCIPAL; + + try { + urlSecurityCheck(makeURI(HTTP_URI), nullPrincipal, + DISALLOW_INHERIT_PRINCIPAL); + } + catch(ex) { + do_throw("urlSecurityCheck should not throw when linking to a http uri with a null principal"); + } + + // urlSecurityCheck also supports passing the url as a string + try { + urlSecurityCheck(HTTP_URI, nullPrincipal, + DISALLOW_INHERIT_PRINCIPAL); + } + catch(ex) { + do_throw("urlSecurityCheck failed to handle the http URI as a string (uri spec)"); + } + + try { + urlSecurityCheck(CHROME_URI, nullPrincipal, + DISALLOW_INHERIT_PRINCIPAL); + do_throw("urlSecurityCheck should throw when linking to a chrome uri with a null principal"); + } + catch(ex) { } +} + +function run_test() +{ + loadUtilsScript(); + test_urlSecurityCheck(); +} diff --git a/toolkit/content/widgets/browser.xml b/toolkit/content/widgets/browser.xml index d8cbec8ff9a..4ea043ed478 100644 --- a/toolkit/content/widgets/browser.xml +++ b/toolkit/content/widgets/browser.xml @@ -314,6 +314,10 @@ onget="return this.contentDocument.title;" readonly="true"/> + + Components.classes['@mozilla.org/preferences-service;1'] .getService(Components.interfaces.nsIPrefService) diff --git a/toolkit/content/widgets/tabbrowser.xml b/toolkit/content/widgets/tabbrowser.xml index 63e11dac284..853f3d135bd 100644 --- a/toolkit/content/widgets/tabbrowser.xml +++ b/toolkit/content/widgets/tabbrowser.xml @@ -856,12 +856,6 @@ return; // Refuse to load if we can't do a security check. } - // Verify that the load of this icon is legal. - // We check first with the security manager - const secMan = - Components.classes["@mozilla.org/scriptsecuritymanager;1"] - .getService(Components.interfaces.nsIScriptSecurityManager); - // Get the IOService so we can make URIs const ioService = Components.classes["@mozilla.org/network/io-service;1"] @@ -870,28 +864,27 @@ const targetDoc = event.target.ownerDocument; // Make a URI out of our href. var uri = ioService.newURI(href, targetDoc.characterSet, null); - - var origURI = ioService.newURI(targetDoc.documentURI, targetDoc.characterSet, null); - - const nsIScriptSecMan = - Components.interfaces.nsIScriptSecurityManager; - try { - // error pages can load their favicon - // to be on the safe side, only allow chrome:// favicons + // Verify that the load of this icon is legal. + // error pages can load their favicon, to be on the safe side, + // only allow chrome:// favicons + const nsIScriptSecMan = + Components.interfaces.nsIScriptSecurityManager; + var secMan = Components.classes["@mozilla.org/scriptsecuritymanager;1"] + .getService(nsIScriptSecMan); const aboutNeterr = "about:neterror?"; - if (origURI.spec.substr(0, aboutNeterr.length) != aboutNeterr || + if (targetDoc.documentURI.substr(0, aboutNeterr.length) != aboutNeterr || !uri.schemeIs("chrome")) - secMan.checkLoadURI(origURI, uri, - nsIScriptSecMan.DISALLOW_SCRIPT); + secMan.checkLoadURIWithPrincipal(targetDoc.nodePrincipal, uri, + nsIScriptSecMan.DISALLOW_SCRIPT); } catch(e) { return; } // Security says okay, now ask content policy if (contentPolicy.shouldLoad(nsIContentPolicy.TYPE_IMAGE, - uri, origURI, event.target, - event.target.type, + uri, targetDoc.documentURIObject, + event.target, event.target.type, null) != nsIContentPolicy.ACCEPT) return; @@ -2282,6 +2275,10 @@ onget="return this.mCurrentBrowser.contentTitle;" readonly="true"/> + + diff --git a/toolkit/content/widgets/text.xml b/toolkit/content/widgets/text.xml index 91df684b03d..cf9a274e861 100644 --- a/toolkit/content/widgets/text.xml +++ b/toolkit/content/widgets/text.xml @@ -310,12 +310,15 @@ .getService(Components.interfaces.nsIIOService); uri = ioService.newURI(href, null, null); - var safeURI = ioService.newURI("about:blank", null, null); + var nullPrincipal = + Components.classes["@mozilla.org/nullprincipal;1"] + .createInstance(Components.interfaces.nsIPrincipal); try { - secMan.checkLoadURI(safeURI, uri, - nsISSM.DISALLOW_INHERIT_PRINCIPAL) - } catch (ex) { + secMan.checkLoadURIWithPrincipal(nullPrincipal, uri, + nsISSM.DISALLOW_INHERIT_PRINCIPAL) + } + catch (ex) { var msg = "Error: Cannot open a " + uri.scheme + ": link using \ the text-link binding."; Components.utils.reportError(msg); @@ -334,7 +337,10 @@ return; } - } catch (ex) {} + } + catch (ex) { + Components.utils.reportError(ex); + } // otherwise, fall back to opening the anchor directly var win = window; @@ -349,7 +355,6 @@ win.open(href); aEvent.preventDefault(); - ]]> @@ -363,4 +368,3 @@ -