Bug 1685801: Part 12 - Remove BrowserUtils.urlSecurityCheck. r=mccr8

This moves the exception prettifying to the script security manager for all JS
callers, where it is much cheaper and more consistently applied.

Differential Revision: https://phabricator.services.mozilla.com/D101492
This commit is contained in:
Kris Maglione 2021-01-28 20:58:48 +00:00
parent 791703a9b8
commit b92138146b
7 changed files with 100 additions and 58 deletions

View File

@ -7,11 +7,6 @@ var EXPORTED_SYMBOLS = ["ClickHandlerChild"];
const { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm");
ChromeUtils.defineModuleGetter(
this,
"BrowserUtils",
"resource://gre/modules/BrowserUtils.jsm"
);
ChromeUtils.defineModuleGetter(
this,
"PrivateBrowsingUtils",
@ -102,7 +97,10 @@ class ClickHandlerChild extends JSWindowActorChild {
if (href) {
try {
BrowserUtils.urlSecurityCheck(href, principal);
Services.scriptSecurityManager.checkLoadURIStrWithPrincipal(
principal,
href
);
} catch (e) {
return;
}

View File

@ -17,7 +17,6 @@ XPCOMUtils.defineLazyGlobalGetters(this, ["URL"]);
XPCOMUtils.defineLazyModuleGetters(this, {
E10SUtils: "resource://gre/modules/E10SUtils.jsm",
BrowserUtils: "resource://gre/modules/BrowserUtils.jsm",
SpellCheckHelper: "resource://gre/modules/InlineSpellChecker.jsm",
LoginManagerChild: "resource://gre/modules/LoginManagerChild.jsm",
WebNavigationFrames: "resource://gre/modules/WebNavigationFrames.jsm",
@ -238,9 +237,9 @@ class ContextMenuChild extends JSWindowActorChild {
if (!disable) {
try {
BrowserUtils.urlSecurityCheck(
target.currentURI.spec,
target.ownerDocument.nodePrincipal
Services.scriptSecurityManager.checkLoadURIWithPrincipal(
target.ownerDocument.nodePrincipal,
target.currentURI
);
let canvas = this.document.createElement("canvas");
canvas.width = target.naturalWidth;

View File

@ -14,7 +14,6 @@ XPCOMUtils.defineLazyModuleGetters(this, {
AppConstants: "resource://gre/modules/AppConstants.jsm",
BrowserSearchTelemetry: "resource:///modules/BrowserSearchTelemetry.jsm",
BrowserUIUtils: "resource:///modules/BrowserUIUtils.jsm",
BrowserUtils: "resource://gre/modules/BrowserUtils.jsm",
ExtensionSearchHandler: "resource://gre/modules/ExtensionSearchHandler.jsm",
ObjectUtils: "resource://gre/modules/ObjectUtils.jsm",
PrivateBrowsingUtils: "resource://gre/modules/PrivateBrowsingUtils.jsm",
@ -3258,10 +3257,11 @@ function getDroppableData(event) {
}
try {
// If this throws, urlSecurityCheck would also throw, as that's what it
// does with things that don't pass the IO service's newURI constructor
// without fixup. It's conceivable we may want to relax this check in
// the future (so e.g. www.foo.com gets fixed up), but not right now.
// If this throws, checkLoadURStrWithPrincipal would also throw,
// as that's what it does with things that don't pass the IO
// service's newURI constructor without fixup. It's conceivable we
// may want to relax this check in the future (so e.g. www.foo.com
// gets fixed up), but not right now.
let url = new URL(href);
// If we succeed, try to pass security checks. If this works, return the
// URL object. If the *security checks* fail, return null.
@ -3269,9 +3269,9 @@ function getDroppableData(event) {
let principal = Services.droppedLinkHandler.getTriggeringPrincipal(
event
);
BrowserUtils.urlSecurityCheck(
url,
Services.scriptSecurityManager.checkLoadURIStrWithPrincipal(
principal,
url.href,
Ci.nsIScriptSecurityManager.DISALLOW_INHERIT_PRINCIPAL
);
return url;

View File

@ -113,9 +113,20 @@ interface nsIScriptSecurityManager : nsISupports
* will only appear in the browser console, not window-associated
* consoles like the web console.
*/
[binaryname(CheckLoadURIWithPrincipal)]
void checkLoadURIWithPrincipalXPCOM(in nsIPrincipal aPrincipal,
in nsIURI uri,
in unsigned long flags,
[optional] in unsigned long long innerWindowID);
/**
* Same as the above, but when called from JS, raises exceptions with more
* useful messages, including both the tested URI and the principal string.
*/
[implicit_jscontext, binaryname(CheckLoadURIWithPrincipalFromJS)]
void checkLoadURIWithPrincipal(in nsIPrincipal aPrincipal,
in nsIURI uri,
in unsigned long flags,
[optional] in unsigned long flags,
[optional] in unsigned long long innerWindowID);
/**
@ -127,9 +138,19 @@ interface nsIScriptSecurityManager : nsISupports
* load as well); if any of the versions of this URI is not allowed, this
* function will return error code NS_ERROR_DOM_BAD_URI.
*/
[binaryname(CheckLoadURIStrWithPrincipal)]
void checkLoadURIStrWithPrincipalXPCOM(in nsIPrincipal aPrincipal,
in AUTF8String uri,
in unsigned long flags);
/**
* Same as the above, but when called from JS, raises exceptions with more
* useful messages, including both the tested URI and the principal string.
*/
[implicit_jscontext, binaryname(CheckLoadURIStrWithPrincipalFromJS)]
void checkLoadURIStrWithPrincipal(in nsIPrincipal aPrincipal,
in AUTF8String uri,
in unsigned long flags);
[optional] in unsigned long flags);
/**
* Returns true if the URI is from a domain that is allow-listed through

View File

@ -57,9 +57,11 @@
#include <stdint.h>
#include "mozilla/dom/ContentChild.h"
#include "mozilla/dom/ContentParent.h"
#include "mozilla/dom/Exceptions.h"
#include "mozilla/dom/nsCSPContext.h"
#include "mozilla/dom/ScriptSettings.h"
#include "mozilla/ClearOnShutdown.h"
#include "mozilla/ResultExtensions.h"
#include "mozilla/StaticPtr.h"
#include "mozilla/dom/WorkerCommon.h"
#include "mozilla/dom/WorkerPrivate.h"
@ -1116,6 +1118,49 @@ nsScriptSecurityManager::CheckLoadURIStrWithPrincipal(
return rv;
}
NS_IMETHODIMP
nsScriptSecurityManager::CheckLoadURIWithPrincipalFromJS(nsIPrincipal* aPrincipal,
nsIURI* aTargetURI,
uint32_t aFlags,
uint64_t aInnerWindowID,
JSContext* aCx) {
NS_ENSURE_ARG_POINTER(aTargetURI);
nsresult rv = CheckLoadURIWithPrincipal(aPrincipal,
aTargetURI,
aFlags,
aInnerWindowID);
if (NS_FAILED(rv)) {
nsAutoCString uriStr;
Unused << aTargetURI->GetSpec(uriStr);
nsAutoCString message("Load of ");
message.Append(uriStr);
nsAutoCString principalStr;
Unused << aPrincipal->GetSpec(principalStr);
if (!principalStr.IsEmpty()) {
message.AppendPrintf(" from %s", principalStr.get());
}
message.Append(" denied");
dom::Throw(aCx, rv, message);
}
return rv;
}
NS_IMETHODIMP
nsScriptSecurityManager::CheckLoadURIStrWithPrincipalFromJS(
nsIPrincipal* aPrincipal, const nsACString& aTargetURIStr,
uint32_t aFlags, JSContext* aCx) {
nsCOMPtr<nsIURI> targetURI;
MOZ_TRY(NS_NewURI(getter_AddRefs(targetURI), aTargetURIStr));
return CheckLoadURIWithPrincipalFromJS(aPrincipal, targetURI, aFlags, 0, aCx);
}
NS_IMETHODIMP
nsScriptSecurityManager::InFileURIAllowlist(nsIURI* aUri, bool* aResult) {
MOZ_ASSERT(aUri);

View File

@ -31,8 +31,24 @@ var ContentAreaUtils = {
},
};
function urlSecurityCheck(aURL, aPrincipal, aFlags) {
return BrowserUtils.urlSecurityCheck(aURL, aPrincipal, aFlags);
function urlSecurityCheck(
aURL,
aPrincipal,
aFlags = Services.scriptSecurityManager
) {
if (aURL instanceof Ci.nsIURI) {
Services.scriptSecurityManager.checkLoadURIWithPrincipal(
aPrincipal,
aURL,
aFlags
);
} else {
Services.scriptSecurityManager.checkLoadURIStrWithPrincipal(
aPrincipal,
aURL,
aFlags
);
}
}
// Clientele: (Make sure you don't break any of these)

View File

@ -13,43 +13,6 @@ const { XPCOMUtils } = ChromeUtils.import(
);
var BrowserUtils = {
/**
* urlSecurityCheck: JavaScript wrapper for checkLoadURIWithPrincipal
* and checkLoadURIStrWithPrincipal.
* If |aPrincipal| is not allowed to link to |aURL|, this function throws with
* an error message.
*
* @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.
*/
urlSecurityCheck(aURL, aPrincipal, aFlags) {
var secMan = Services.scriptSecurityManager;
if (aFlags === undefined) {
aFlags = secMan.STANDARD;
}
try {
if (aURL instanceof Ci.nsIURI) {
secMan.checkLoadURIWithPrincipal(aPrincipal, aURL, aFlags);
} else {
secMan.checkLoadURIStrWithPrincipal(aPrincipal, aURL, aFlags);
}
} catch (e) {
let principalStr = "";
try {
principalStr = " from " + aPrincipal.spec;
} catch (e2) {}
throw new Error(`Load of ${aURL + principalStr} denied.`);
}
},
/**
* Return or create a principal with the content of one, and the originAttributes
* of an existing principal (e.g. on a docshell, where the originAttributes ought