diff --git a/docshell/test/browser/browser_loadDisallowInherit.js b/docshell/test/browser/browser_loadDisallowInherit.js
index d623c138201..22a5edfe63a 100644
--- a/docshell/test/browser/browser_loadDisallowInherit.js
+++ b/docshell/test/browser/browser_loadDisallowInherit.js
@@ -47,9 +47,7 @@ function test() {
let urls = [
"data:text/html,hi",
- // We used to test javascript: here as well, but now that we no longer run
- // javascript: in a sandbox, we end up not running it at all in the
- // DISALLOW_INHERIT_OWNER case, so never actually do a load for it at all.
+ "javascript:1;"
];
function nextTest() {
diff --git a/dom/base/nsIScriptChannel.idl b/dom/base/nsIScriptChannel.idl
index 0e57313f7ac..32bdc6b604c 100644
--- a/dom/base/nsIScriptChannel.idl
+++ b/dom/base/nsIScriptChannel.idl
@@ -34,9 +34,10 @@ interface nsIScriptChannel : nsISupports
const unsigned long NO_EXECUTION = 0;
/**
- * There used to be an EXECUTE_IN_SANDBOX = 1 value. It has been removed, but
- * we're not changing the value of EXECUTE_NORMAL to avoid breaking compat.
+ * Execute in a sandbox, no matter how the various principals involved are
+ * related to each other.
*/
+ const unsigned long EXECUTE_IN_SANDBOX = 1;
/**
* Execute against the target environment if the principals allow it.
diff --git a/dom/src/jsurl/crashtests/1018583.html b/dom/src/jsurl/crashtests/1018583.html
deleted file mode 100644
index 8ede367c6bf..00000000000
--- a/dom/src/jsurl/crashtests/1018583.html
+++ /dev/null
@@ -1,3 +0,0 @@
-
-
-{})
diff --git a/dom/src/jsurl/crashtests/crashtests.list b/dom/src/jsurl/crashtests/crashtests.list
index b23499d4c1d..51f92d51b52 100644
--- a/dom/src/jsurl/crashtests/crashtests.list
+++ b/dom/src/jsurl/crashtests/crashtests.list
@@ -2,4 +2,3 @@ load 341963-1.html
load 344874-1.html
load 344996-1.xhtml
load 457050-1.html
-load 1018583.html
diff --git a/dom/src/jsurl/nsJSProtocolHandler.cpp b/dom/src/jsurl/nsJSProtocolHandler.cpp
index c8101ab83d8..78a3fd8de35 100644
--- a/dom/src/jsurl/nsJSProtocolHandler.cpp
+++ b/dom/src/jsurl/nsJSProtocolHandler.cpp
@@ -237,6 +237,9 @@ nsresult nsJSThunk::EvaluateScript(nsIChannel *aChannel,
if (NS_FAILED(rv))
return rv;
+ bool useSandbox =
+ (aExecutionPolicy == nsIScriptChannel::EXECUTE_IN_SANDBOX);
+
// New script entry point required, due to the "Create a script" step of
// http://www.whatwg.org/specs/web-apps/current-work/#javascript-protocol
AutoEntryScript entryScript(innerGlobal, true,
@@ -245,37 +248,90 @@ nsresult nsJSThunk::EvaluateScript(nsIChannel *aChannel,
JS::Rooted globalJSObject(cx, innerGlobal->GetGlobalJSObject());
NS_ENSURE_TRUE(globalJSObject, NS_ERROR_UNEXPECTED);
- //-- Don't execute unless the script principal subsumes the
- // principal of the context.
- nsIPrincipal* objectPrincipal = nsContentUtils::ObjectPrincipal(globalJSObject);
+ if (!useSandbox) {
+ //-- Don't outside a sandbox unless the script principal subsumes the
+ // principal of the context.
+ nsIPrincipal* objectPrincipal = nsContentUtils::ObjectPrincipal(globalJSObject);
- bool subsumes;
- rv = principal->Subsumes(objectPrincipal, &subsumes);
- if (NS_FAILED(rv))
- return rv;
+ bool subsumes;
+ rv = principal->Subsumes(objectPrincipal, &subsumes);
+ if (NS_FAILED(rv))
+ return rv;
- if (!subsumes) {
- return NS_ERROR_DOM_RETVAL_UNDEFINED;
+ useSandbox = !subsumes;
}
JS::Rooted v (cx, JS::UndefinedValue());
// Finally, we have everything needed to evaluate the expression.
- JS::CompileOptions options(cx);
- options.setFileAndLine(mURL.get(), 1)
- .setVersion(JSVERSION_DEFAULT);
- nsJSUtils::EvaluateOptions evalOptions;
- evalOptions.setCoerceToString(true);
- rv = nsJSUtils::EvaluateString(cx, NS_ConvertUTF8toUTF16(script),
- globalJSObject, options, evalOptions, &v);
+ if (useSandbox) {
+ // We were asked to use a sandbox, or the channel owner isn't allowed
+ // to execute in this context. Evaluate the javascript URL in a
+ // sandbox to prevent it from accessing data it doesn't have
+ // permissions to access.
- // If there's an error on cx as a result of that call, report
- // it now -- either we're just running under the event loop,
- // so we shouldn't propagate JS exceptions out of here, or we
- // can't be sure that our caller is JS (and if it's not we'll
- // lose the error), or it might be JS that then proceeds to
- // cause an error of its own (which will also make us lose
- // this error).
- ::JS_ReportPendingException(cx);
+ // First check to make sure it's OK to evaluate this script to
+ // start with. For example, script could be disabled.
+ if (!securityManager->ScriptAllowed(globalJSObject)) {
+ // Treat this as returning undefined from the script. That's what
+ // nsJSContext does.
+ return NS_ERROR_DOM_RETVAL_UNDEFINED;
+ }
+
+ nsIXPConnect *xpc = nsContentUtils::XPConnect();
+
+ nsCOMPtr sandbox;
+ // Important: Use a null principal here
+ rv = xpc->CreateSandbox(cx, nullptr, getter_AddRefs(sandbox));
+ NS_ENSURE_SUCCESS(rv, rv);
+
+ // The nsXPConnect sandbox API gives us a wrapper to the sandbox for
+ // our current compartment. Because our current context doesn't necessarily
+ // subsume that of the sandbox, we want to unwrap and enter the sandbox's
+ // compartment. It's a shame that the APIs here are so clunkly. :-(
+ JS::Rooted sandboxObj(cx, sandbox->GetJSObject());
+ NS_ENSURE_STATE(sandboxObj);
+ sandboxObj = js::UncheckedUnwrap(sandboxObj);
+ JSAutoCompartment ac(cx, sandboxObj);
+
+ // Push our JSContext on the context stack so the EvalInSandboxObject call (and
+ // JS_ReportPendingException, if relevant) will use the principal of cx.
+ nsCxPusher pusher;
+ pusher.Push(cx);
+ rv = xpc->EvalInSandboxObject(NS_ConvertUTF8toUTF16(script),
+ /* filename = */ nullptr, cx,
+ sandboxObj, true, &v);
+
+ // Propagate and report exceptions that happened in the
+ // sandbox.
+ if (JS_IsExceptionPending(cx)) {
+ JS_ReportPendingException(cx);
+ }
+ } else {
+ // No need to use the sandbox, evaluate the script directly in
+ // the given scope.
+ JS::CompileOptions options(cx);
+ options.setFileAndLine(mURL.get(), 1)
+ .setVersion(JSVERSION_DEFAULT);
+ nsJSUtils::EvaluateOptions evalOptions;
+ evalOptions.setCoerceToString(true);
+ rv = nsJSUtils::EvaluateString(cx, NS_ConvertUTF8toUTF16(script),
+ globalJSObject, options, evalOptions, &v);
+
+ // If there's an error on cx as a result of that call, report
+ // it now -- either we're just running under the event loop,
+ // so we shouldn't propagate JS exceptions out of here, or we
+ // can't be sure that our caller is JS (and if it's not we'll
+ // lose the error), or it might be JS that then proceeds to
+ // cause an error of its own (which will also make us lose
+ // this error).
+ ::JS_ReportPendingException(cx);
+ }
+
+ // If we took the sandbox path above, v might be in the sandbox
+ // compartment.
+ if (!JS_WrapValue(cx, &v)) {
+ return NS_ERROR_OUT_OF_MEMORY;
+ }
if (NS_FAILED(rv) || !(v.isString() || v.isUndefined())) {
return NS_ERROR_MALFORMED_URI;
@@ -378,7 +434,7 @@ nsJSChannel::nsJSChannel() :
mLoadFlags(LOAD_NORMAL),
mActualLoadFlags(LOAD_NORMAL),
mPopupState(openOverridden),
- mExecutionPolicy(NO_EXECUTION),
+ mExecutionPolicy(EXECUTE_IN_SANDBOX),
mIsAsync(true),
mIsActive(false),
mOpenedStreamChannel(false)
diff --git a/layout/reftests/bugs/376484-1-ref.html b/layout/reftests/bugs/376484-1-ref.html
index 2cded6bc487..5bc00deb06a 100644
--- a/layout/reftests/bugs/376484-1-ref.html
+++ b/layout/reftests/bugs/376484-1-ref.html
@@ -2,9 +2,9 @@
This is text
-This is more text
+This is more text
This is yet more text
-This is even more text
+This is even more text