[JSC] eval() function from another realm shouldn't cause direct eval …

…call https://bugs.webkit.org/show_bug.cgi?id=268027 <rdar://problem/121546048> Reviewed by Yusuke Suzuki. Whether eval() is direct or not is determined by SameValue() in [1] that compares the function to `%eval%`, which denotes [2] specifically the eval() function of current global object, not any built-in eval(). This change tightens the check for direct eval() and aligns JSC with V8 and SpiderMonkey. Since Interpreter::eval() has tricky way of resolving global object, accounting for DFG inlining code blocks, the callee check was moved there, ensuring all call sites handle empty JSValue. [1]: https://tc39.es/ecma262/#sec-function-calls-runtime-semantics-evaluation (step 6.a) [2]: https://tc39.es/ecma262/#sec-well-known-intrinsic-objects * JSTests/stress/direct-eval-cross-realm.js: Added. * JSTests/test262/expectations.yaml: Mark 1 test as passing. * Source/JavaScriptCore/jit/JITOperations.cpp: (JSC::JSC_DEFINE_JIT_OPERATION): * Source/JavaScriptCore/llint/LLIntSlowPaths.cpp: (JSC::LLInt::commonCallDirectEval): * Source/JavaScriptCore/runtime/JSFunctionInlines.h: (JSC::isHostFunction): Deleted. Canonical link: https://commits.webkit.org/273782@main
WebKit · Jan 30, 2024 · 4737a82 · 4737a82
1 parent 6405dde
commit 4737a82
Show file tree

Hide file tree

Showing 6 changed files with 37 additions and 20 deletions.
diff --git a/JSTests/stress/direct-eval-cross-realm.js b/JSTests/stress/direct-eval-cross-realm.js
@@ -0,0 +1,29 @@
+function shouldBe(actual, expected) {
+  if (actual !== expected)
+    throw new Error(`Bad value: ${actual}!`);
+}
+
+(function() {
+  function maybeReplaceEval(i) {
+    if (i === (1e5 / 2)) {
+      eval = otherEval;
+    }
+  }
+
+  function isDirectEval() {
+    return true;
+  }
+
+  var otherGlobal = createGlobalObject();
+  otherGlobal.i = 0;
+  otherGlobal.maybeReplaceEval = maybeReplaceEval;
+  otherGlobal.isDirectEval = () => false;
+
+  var otherEval = otherGlobal.eval;
+  var eval = globalThis.eval;
+
+  for (var i = 0; i < 1e5; i++) {
+    eval("maybeReplaceEval(i)");
+    shouldBe(eval("isDirectEval()"), i < (1e5 / 2));
+  }
+})();
diff --git a/JSTests/test262/expectations.yaml b/JSTests/test262/expectations.yaml
@@ -941,8 +941,6 @@ test/language/expressions/assignmenttargettype/direct-callexpression-arguments.j
   default: 'Test262: This statement should not be evaluated.'
 test/language/expressions/assignmenttargettype/parenthesized-callexpression-arguments.js:
   default: 'Test262: This statement should not be evaluated.'
-test/language/expressions/call/eval-realm-indirect.js:
-  default: 'Test262Error: Expected SameValue(«inside», «outside») to be true'
 test/language/expressions/call/eval-spread-empty-leading.js:
   default: 'Test262Error: Expected SameValue(«local», «0») to be true'
   strict mode: 'Test262Error: Expected SameValue(«local», «0») to be true'

diff --git a/Source/JavaScriptCore/interpreter/Interpreter.cpp b/Source/JavaScriptCore/interpreter/Interpreter.cpp
@@ -107,6 +107,9 @@ JSValue eval(CallFrame* callFrame, JSValue thisValue, JSScope* callerScopeChain,
     UnlinkedCodeBlock* callerUnlinkedCodeBlock = callerBaselineCodeBlock->unlinkedCodeBlock();
     JSGlobalObject* globalObject = callerBaselineCodeBlock->globalObject();
 
+    if (UNLIKELY(callFrame->guaranteedJSValueCallee() != globalObject->evalFunction()))
+        return { };
+
     VM& vm = globalObject->vm();
     auto scope = DECLARE_THROW_SCOPE(vm);
 

diff --git a/Source/JavaScriptCore/jit/JITOperations.cpp b/Source/JavaScriptCore/jit/JITOperations.cpp
@@ -2034,9 +2034,6 @@ JSC_DEFINE_JIT_OPERATION(operationCallDirectEvalSloppy, EncodedJSValue, (void* f
     CallFrame* calleeFrame = bitwise_cast<CallFrame*>(frame);
     calleeFrame->setCodeBlock(nullptr);
 
-    if (!isHostFunction(calleeFrame->guaranteedJSValueCallee(), globalFuncEval))
-        return JSValue::encode(JSValue());
-
     return JSValue::encode(eval(calleeFrame, JSValue::decode(encodedThisValue), callerScopeChain, ECMAMode::sloppy()));
 }
 
@@ -2045,9 +2042,6 @@ JSC_DEFINE_JIT_OPERATION(operationCallDirectEvalStrict, EncodedJSValue, (void* f
     CallFrame* calleeFrame = bitwise_cast<CallFrame*>(frame);
     calleeFrame->setCodeBlock(nullptr);
 
-    if (!isHostFunction(calleeFrame->guaranteedJSValueCallee(), globalFuncEval))
-        return JSValue::encode(JSValue());
-
     return JSValue::encode(eval(calleeFrame, JSValue::decode(encodedThisValue), callerScopeChain, ECMAMode::strict()));
 }
 

diff --git a/Source/JavaScriptCore/llint/LLIntSlowPaths.cpp b/Source/JavaScriptCore/llint/LLIntSlowPaths.cpp
@@ -2141,12 +2141,13 @@ static inline UGPRPair commonCallDirectEval(CallFrame* callFrame, const JSInstru
     calleeFrame->setCodeBlock(nullptr);
     callFrame->setCurrentVPC(pc);
 
-    if (!isHostFunction(calleeAsValue, globalFuncEval))
-        RELEASE_AND_RETURN(throwScope, setUpCall(calleeFrame, CodeForCall, calleeAsValue));
-
     JSScope* callerScopeChain = jsCast<JSScope*>(getOperand(callFrame, bytecode.m_scope));
     JSValue thisValue = getOperand(callFrame, bytecode.m_thisValue);
-    vm.encodedHostCallReturnValue = JSValue::encode(eval(calleeFrame, thisValue, callerScopeChain, bytecode.m_ecmaMode));
+    JSValue result = eval(calleeFrame, thisValue, callerScopeChain, bytecode.m_ecmaMode);
+    if (!result)
+        RELEASE_AND_RETURN(throwScope, setUpCall(calleeFrame, CodeForCall, calleeAsValue));
+
+    vm.encodedHostCallReturnValue = JSValue::encode(result);
     DisallowGC disallowGC;
     auto* callerSP = calleeFrame + CallerFrameAndPC::sizeInRegisters;
     LLINT_CALL_RETURN(globalObject, callerSP, LLInt::getHostCallReturnValueEntrypoint().code().taggedPtr(), JSEntryPtrTag);

diff --git a/Source/JavaScriptCore/runtime/JSFunctionInlines.h b/Source/JavaScriptCore/runtime/JSFunctionInlines.h
@@ -121,14 +121,6 @@ inline TaggedNativeFunction JSFunction::nativeConstructor()
     return static_cast<NativeExecutable*>(executable())->constructor();
 }
 
-inline bool isHostFunction(JSValue value, TaggedNativeFunction nativeFunction)
-{
-    JSFunction* function = jsCast<JSFunction*>(getJSFunction(value));
-    if (!function || !function->isHostFunction())
-        return false;
-    return function->nativeFunction() == nativeFunction;
-}
-
 inline bool isRemoteFunction(JSValue value)
 {
     return value.inherits<JSRemoteFunction>();