Backed out changeset bc387540075d (bug 1615405) on evilpie's request …

…CLOSED TREE
dothq · Feb 14, 2020 · 2a147d1 · 2a147d1
1 parent 140762b
commit 2a147d1
Show file tree

Hide file tree

Showing 9 changed files with 60 additions and 21 deletions.
diff --git a/caps/nsScriptSecurityManager.cpp b/caps/nsScriptSecurityManager.cpp
@@ -398,7 +398,7 @@ NS_IMPL_ISUPPORTS(nsScriptSecurityManager, nsIScriptSecurityManager)
 ///////////////// Security Checks /////////////////
 
 bool nsScriptSecurityManager::ContentSecurityPolicyPermitsJSAction(
-    JSContext* cx, JS::HandleString aCode) {
+    JSContext* cx, JS::HandleValue aValue) {
   MOZ_ASSERT(cx == nsContentUtils::GetCurrentJSContext());
 
   // Get the window, if any, corresponding to the current global
@@ -445,7 +445,13 @@ bool nsScriptSecurityManager::ContentSecurityPolicyPermitsJSAction(
   nsAutoJSString scriptSample;
   if (reportViolation || subjectPrincipal->IsSystemPrincipal() ||
       XRE_IsE10sParentProcess()) {
-    if (NS_WARN_IF(!scriptSample.init(cx, aCode))) {
+    JS::Rooted<JSString*> jsString(cx, JS::ToString(cx, aValue));
+    if (NS_WARN_IF(!jsString)) {
+      JS_ClearPendingException(cx);
+      return false;
+    }
+
+    if (NS_WARN_IF(!scriptSample.init(cx, jsString))) {
       JS_ClearPendingException(cx);
       return false;
     }

diff --git a/caps/nsScriptSecurityManager.h b/caps/nsScriptSecurityManager.h
@@ -90,7 +90,7 @@ class nsScriptSecurityManager final : public nsIScriptSecurityManager {
 
   // Decides, based on CSP, whether or not eval() and stuff can be executed.
   static bool ContentSecurityPolicyPermitsJSAction(JSContext* cx,
-                                                   JS::HandleString aCode);
+                                                   JS::HandleValue aValue);
 
   static bool JSPrincipalsSubsume(JSPrincipals* first, JSPrincipals* second);
 

diff --git a/dom/workers/RuntimeService.cpp b/dom/workers/RuntimeService.cpp
@@ -581,12 +581,18 @@ class LogViolationDetailsRunnable final : public WorkerMainThreadRunnable {
   ~LogViolationDetailsRunnable() = default;
 };
 
-bool ContentSecurityPolicyAllows(JSContext* aCx, JS::HandleString aCode) {
+bool ContentSecurityPolicyAllows(JSContext* aCx, JS::HandleValue aValue) {
   WorkerPrivate* worker = GetWorkerPrivateFromContext(aCx);
   worker->AssertIsOnWorkerThread();
 
+  JS::Rooted<JSString*> jsString(aCx, JS::ToString(aCx, aValue));
+  if (NS_WARN_IF(!jsString)) {
+    JS_ClearPendingException(aCx);
+    return false;
+  }
+
   nsAutoJSString scriptSample;
-  if (NS_WARN_IF(!scriptSample.init(aCx, aCode))) {
+  if (NS_WARN_IF(!scriptSample.init(aCx, jsString))) {
     JS_ClearPendingException(aCx);
     return false;
   }

diff --git a/js/public/Principals.h b/js/public/Principals.h
@@ -70,7 +70,7 @@ typedef bool (*JSSubsumesOp)(JSPrincipals* first, JSPrincipals* second);
  * Used to check if a CSP instance wants to disable eval() and friends.
  * See GlobalObject::isRuntimeCodeGenEnabled() in vm/GlobalObject.cpp.
  */
-typedef bool (*JSCSPEvalChecker)(JSContext* cx, JS::HandleString code);
+typedef bool (*JSCSPEvalChecker)(JSContext* cx, JS::HandleValue value);
 
 struct JSSecurityCallbacks {
   JSCSPEvalChecker contentSecurityPolicyAllows;

diff --git a/js/src/builtin/Eval.cpp b/js/src/builtin/Eval.cpp
@@ -209,8 +209,6 @@ static EvalJSONResult TryEvalJSON(JSContext* cx, JSLinearString* str,
 
 enum EvalType { DIRECT_EVAL, INDIRECT_EVAL };
 
-// 18.2.1.1 PerformEval
-//
 // Common code implementing direct and indirect eval.
 //
 // Evaluate call.argv[2], if it is a string, in the context of the given calling
@@ -227,21 +225,20 @@ static bool EvalKernel(JSContext* cx, HandleValue v, EvalType evalType,
   MOZ_ASSERT_IF(evalType == INDIRECT_EVAL, IsGlobalLexicalEnvironment(env));
   AssertInnerizedEnvironmentChain(cx, *env);
 
-  // Step 2.
+  if (!GlobalObject::isRuntimeCodeGenEnabled(cx, v, cx->global())) {
+    JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr,
+                              JSMSG_CSP_BLOCKED_EVAL);
+    return false;
+  }
+
+  // ES5 15.1.2.1 step 1.
   if (!v.isString()) {
     vp.set(v);
     return true;
   }
-
-  // Steps 3-4.
   RootedString str(cx, v.toString());
-  if (!GlobalObject::isRuntimeCodeGenEnabled(cx, str, cx->global())) {
-    JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr,
-                              JSMSG_CSP_BLOCKED_EVAL);
-    return false;
-  }
 
-  // Step 5 ff.
+  // ES5 15.1.2.1 steps 2-8.
 
   // Per ES5, indirect eval runs in the global scope. (eval is specified this
   // way so that the compiler can make assumptions about what bindings may or
@@ -357,7 +354,8 @@ bool js::DirectEvalStringFromIon(JSContext* cx, HandleObject env,
                                  jsbytecode* pc, MutableHandleValue vp) {
   AssertInnerizedEnvironmentChain(cx, *env);
 
-  if (!GlobalObject::isRuntimeCodeGenEnabled(cx, str, cx->global())) {
+  RootedValue v(cx, StringValue(str));
+  if (!GlobalObject::isRuntimeCodeGenEnabled(cx, v, cx->global())) {
     JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr,
                               JSMSG_CSP_BLOCKED_EVAL);
     return false;

diff --git a/js/src/vm/GlobalObject.cpp b/js/src/vm/GlobalObject.cpp
@@ -812,7 +812,7 @@ bool GlobalObject::initSelfHostingBuiltins(JSContext* cx,
 }
 
 /* static */
-bool GlobalObject::isRuntimeCodeGenEnabled(JSContext* cx, HandleString code,
+bool GlobalObject::isRuntimeCodeGenEnabled(JSContext* cx, HandleValue code,
                                            Handle<GlobalObject*> global) {
   HeapSlot& v = global->getSlotRef(RUNTIME_CODEGEN_ENABLED);
   if (v.isUndefined()) {

diff --git a/js/src/vm/GlobalObject.h b/js/src/vm/GlobalObject.h
@@ -806,7 +806,7 @@ class GlobalObject : public NativeObject {
   static JSObject* getOrCreateThrowTypeError(JSContext* cx,
                                              Handle<GlobalObject*> global);
 
-  static bool isRuntimeCodeGenEnabled(JSContext* cx, HandleString code,
+  static bool isRuntimeCodeGenEnabled(JSContext* cx, HandleValue code,
                                       Handle<GlobalObject*> global);
 
   static bool getOrCreateEval(JSContext* cx, Handle<GlobalObject*> global,

diff --git a/js/src/vm/JSFunction.cpp b/js/src/vm/JSFunction.cpp
@@ -1906,7 +1906,8 @@ static bool CreateDynamicFunction(JSContext* cx, const CallArgs& args,
 
   // Block this call if security callbacks forbid it.
   Handle<GlobalObject*> global = cx->global();
-  if (!GlobalObject::isRuntimeCodeGenEnabled(cx, functionText, global)) {
+  RootedValue v(cx, StringValue(functionText));
+  if (!GlobalObject::isRuntimeCodeGenEnabled(cx, v, global)) {
     JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr,
                               JSMSG_CSP_BLOCKED_FUNCTION);
     return false;

diff --git a/...latform/tests/content-security-policy/generic/eval-typecheck-callout-order.tentative.html b/...latform/tests/content-security-policy/generic/eval-typecheck-callout-order.tentative.html
@@ -0,0 +1,28 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+    <script src='/resources/testharness.js' nonce='abc'></script>
+    <script src='/resources/testharnessreport.js' nonce='abc'></script>
+    <title>Test for order of Type(evalInput) and host callout</title>
+</head>
+<body>
+    <div id='log'></div>
+
+    <script nonce='abc'>
+    test(function() {
+      assert_throws_js(EvalError, function() {
+        eval("0");
+      }, "eval of a string should reach host callout");
+    }, "eval of a string should be checked by CSP");
+
+    test(function() {
+      let array = ["0"];
+      assert_equals(
+        eval(array),
+        array,
+        "eval is identity when applied to non-strings");
+    }, "eval of a non-string should not be checked by CSP");
+    </script>
+
+</body>
+</html>