[JSC] Add assertions for MarkedArgumentBuffer size

https://bugs.webkit.org/show_bug.cgi?id=245286 <rdar://99272310> Reviewed by Alexey Shvayka. 1. Add MarkedArgumentBuffer size assertions to places with fixed-sized MarkedArgumentBuffer arguments. 2. Suppress warning in JSRemoteFunction.cpp when we return in the middle of MarkedArgumentBuffer construction due to different exception. In this case, we do not need to check since we don't use constructed MarkedArgumentBuffer. * Source/JavaScriptCore/API/JSAPIGlobalObject.mm: (JSC::JSAPIGlobalObject::moduleLoaderFetch): * Source/JavaScriptCore/runtime/JSModuleLoader.cpp: (JSC::JSModuleLoader::dependencyKeysIfEvaluated): * Source/JavaScriptCore/runtime/JSPromise.cpp: (JSC::JSPromise::resolvedPromise): * Source/JavaScriptCore/runtime/JSRemoteFunction.cpp: (JSC::JSC_DEFINE_HOST_FUNCTION): * Source/JavaScriptCore/runtime/TemporalCalendarPrototype.cpp: (JSC::JSC_DEFINE_HOST_FUNCTION): * Source/JavaScriptCore/runtime/VM.cpp: (JSC::VM::callPromiseRejectionCallback): * Source/JavaScriptCore/tools/JSDollarVM.cpp: (JSC::JSC_DEFINE_HOST_FUNCTION): * Source/JavaScriptCore/wasm/WasmOperations.cpp: (JSC::Wasm::JSC_DEFINE_JIT_OPERATION): * Source/WebCore/Modules/webaudio/AudioWorkletProcessor.cpp: (WebCore::AudioWorkletProcessor::process): * Source/WebCore/bindings/js/JSDOMMapLike.cpp: (WebCore::setToBackingMap): * Source/WebCore/bindings/js/JSDOMPromise.cpp: (WebCore::DOMPromise::whenPromiseIsSettled): * Source/WebCore/bindings/js/JSDOMSetLike.cpp: (WebCore::addToBackingSet): * Source/WebCore/bindings/js/ReadableStreamDefaultController.cpp: (WebCore::ReadableStreamDefaultController::close): (WebCore::ReadableStreamDefaultController::error): (WebCore::ReadableStreamDefaultController::enqueue): * Source/WebCore/bindings/js/ScriptController.cpp: (WebCore::ScriptController::callInWorld): (WebCore::ScriptController::executeAsynchronousUserAgentScriptInWorld): Canonical link: https://commits.webkit.org/254571@main
WebKit · Sep 16, 2022 · 3c544f1 · 3c544f1
1 parent ce7674d
commit 3c544f1
Show file tree

Hide file tree

Showing 15 changed files with 36 additions and 4 deletions.
diff --git a/JSTests/stress/shadow-realm-arguments.js b/JSTests/stress/shadow-realm-arguments.js
@@ -0,0 +1,5 @@
+//@ runDefault("--watchdog=10", "--watchdog-exception-ok", "--useShadowRealm=1")
+for (let i = 0; i < 100; i++) {
+  let f = new ShadowRealm().evaluate(`new Proxy(()=>{}, {});`);
+  f.apply(undefined, new Array(10_000));
+}
diff --git a/Source/JavaScriptCore/API/JSAPIGlobalObject.mm b/Source/JavaScriptCore/API/JSAPIGlobalObject.mm
@@ -198,8 +198,6 @@
         JSContext *context = [JSContext contextWithJSGlobalContextRef:toGlobalRef(globalObject)];
         id script = valueToObject(context, toRef(globalObject, callFrame->argument(0)));
 
-        MarkedArgumentBuffer args;
-
         auto rejectPromise = [&] (String message) {
             strongPromise.get()->reject(globalObject, createTypeError(globalObject, message));
             return encodedJSUndefined();

diff --git a/Source/JavaScriptCore/runtime/JSModuleLoader.cpp b/Source/JavaScriptCore/runtime/JSModuleLoader.cpp
@@ -126,6 +126,7 @@ JSArray* JSModuleLoader::dependencyKeysIfEvaluated(JSGlobalObject* globalObject,
 
     MarkedArgumentBuffer arguments;
     arguments.append(key);
+    ASSERT(!arguments.hasOverflowed());
 
     JSValue result = call(globalObject, function, callData, this, arguments);
     RETURN_IF_EXCEPTION(scope, nullptr);

diff --git a/Source/JavaScriptCore/runtime/JSPromise.cpp b/Source/JavaScriptCore/runtime/JSPromise.cpp
@@ -154,6 +154,7 @@ JSPromise* JSPromise::resolvedPromise(JSGlobalObject* globalObject, JSValue valu
 
     MarkedArgumentBuffer arguments;
     arguments.append(value);
+    ASSERT(!arguments.hasOverflowed());
     auto result = call(globalObject, function, callData, globalObject->promiseConstructor(), arguments);
     RETURN_IF_EXCEPTION(scope, nullptr);
     ASSERT(result.inherits<JSPromise>());

diff --git a/Source/JavaScriptCore/runtime/JSRemoteFunction.cpp b/Source/JavaScriptCore/runtime/JSRemoteFunction.cpp
@@ -132,9 +132,17 @@ JSC_DEFINE_HOST_FUNCTION(remoteFunctionCallGeneric, (JSGlobalObject* globalObjec
     JSGlobalObject* targetGlobalObject = targetFunction->globalObject();
 
     MarkedArgumentBuffer args;
+    auto clearArgOverflowCheckAndReturnAbortValue = [&] () -> EncodedJSValue {
+        // This is only called because we'll be imminently returning due to an
+        // exception i.e. we won't be using the args, and we don't care if they
+        // overflowed. However, we still need to call overflowCheckNotNeeded()
+        // to placate an ASSERT in the MarkedArgumentBuffer destructor.
+        args.overflowCheckNotNeeded();
+        return { };
+    };
     for (unsigned i = 0; i < callFrame->argumentCount(); ++i) {
         JSValue wrappedValue = wrapArgument(globalObject, targetGlobalObject, callFrame->uncheckedArgument(i));
-        RETURN_IF_EXCEPTION(scope, { });
+        RETURN_IF_EXCEPTION(scope, clearArgOverflowCheckAndReturnAbortValue());
         args.append(wrappedValue);
     }
     if (UNLIKELY(args.hasOverflowed())) {

diff --git a/Source/JavaScriptCore/runtime/TemporalCalendarPrototype.cpp b/Source/JavaScriptCore/runtime/TemporalCalendarPrototype.cpp
@@ -183,12 +183,16 @@ JSC_DEFINE_HOST_FUNCTION(temporalCalendarPrototypeFuncFields, (JSGlobalObject* g
                 shouldAddEraAndEraYear = true;
         }
         fieldNames.append(value);
+        if (UNLIKELY(fieldNames.hasOverflowed()))
+            throwStackOverflowError(globalObject, scope);
     });
     RETURN_IF_EXCEPTION(scope, { });
 
     if (shouldAddEraAndEraYear) {
         fieldNames.append(jsNontrivialString(vm, vm.propertyNames->era.impl()));
         fieldNames.append(jsNontrivialString(vm, vm.propertyNames->eraYear.impl()));
+        if (UNLIKELY(fieldNames.hasOverflowed()))
+            throwStackOverflowError(globalObject, scope);
     }
 
     RELEASE_AND_RETURN(scope, JSValue::encode(constructArray(globalObject, static_cast<ArrayAllocationProfile*>(nullptr), fieldNames)));

diff --git a/Source/JavaScriptCore/runtime/VM.cpp b/Source/JavaScriptCore/runtime/VM.cpp
@@ -1226,6 +1226,7 @@ void VM::callPromiseRejectionCallback(Strong<JSPromise>& promise)
     MarkedArgumentBuffer args;
     args.append(promise.get());
     args.append(promise->result(*this));
+    ASSERT(!args.hasOverflowed());
     call(promise->globalObject(), callback, callData, jsNull(), args);
     scope.clearException();
 }

diff --git a/Source/JavaScriptCore/tools/JSDollarVM.cpp b/Source/JavaScriptCore/tools/JSDollarVM.cpp
@@ -3097,6 +3097,7 @@ JSC_DEFINE_HOST_FUNCTION(functionCreateWasmStreamingCompilerForCompile, (JSGloba
     auto compiler = WasmStreamingCompiler::create(vm, globalObject, Wasm::CompilerMode::Validation, nullptr);
     MarkedArgumentBuffer args;
     args.append(compiler);
+    ASSERT(!args.hasOverflowed());
     call(globalObject, callback, jsUndefined(), args, "You shouldn't see this..."_s);
     if (UNLIKELY(scope.exception()))
         scope.clearException();
@@ -3124,6 +3125,7 @@ JSC_DEFINE_HOST_FUNCTION(functionCreateWasmStreamingCompilerForInstantiate, (JSG
     auto compiler = WasmStreamingCompiler::create(vm, globalObject, Wasm::CompilerMode::FullCompile, importObject);
     MarkedArgumentBuffer args;
     args.append(compiler);
+    ASSERT(!args.hasOverflowed());
     call(globalObject, callback, jsUndefined(), args, "You shouldn't see this..."_s);
     if (UNLIKELY(scope.exception()))
         scope.clearException();

diff --git a/Source/JavaScriptCore/wasm/WasmOperations.cpp b/Source/JavaScriptCore/wasm/WasmOperations.cpp
@@ -533,8 +533,11 @@ JSC_DEFINE_JIT_OPERATION(operationIterateResults, void, (CallFrame* callFrame, I
     MarkedArgumentBuffer buffer;
     JSValue result = JSValue::decode(encResult);
     forEachInIterable(globalObject, result, [&] (VM&, JSGlobalObject*, JSValue value) -> void {
-        if (buffer.size() < signature->returnCount())
+        if (buffer.size() < signature->returnCount()) {
             buffer.append(value);
+            if (UNLIKELY(buffer.hasOverflowed()))
+                throwOutOfMemoryError(globalObject, scope);
+        }
         ++iterationCount;
     });
     RETURN_IF_EXCEPTION(scope, void());

diff --git a/Source/WebCore/Modules/webaudio/AudioWorkletProcessor.cpp b/Source/WebCore/Modules/webaudio/AudioWorkletProcessor.cpp
@@ -251,6 +251,7 @@ bool AudioWorkletProcessor::process(const Vector<RefPtr<AudioBus>>& inputs, Vect
 
     MarkedArgumentBuffer args;
     buildJSArguments(vm, globalObject, args, inputs, outputs, paramValuesMap);
+    ASSERT(!args.hasOverflowed());
 
     NakedPtr<JSC::Exception> returnedException;
     auto result = JSCallbackData::invokeCallback(globalObject, wrapper(), jsUndefined(), args, JSCallbackData::CallbackType::Object, Identifier::fromString(vm, "process"_s), returnedException);

diff --git a/Source/WebCore/bindings/js/JSDOMMapLike.cpp b/Source/WebCore/bindings/js/JSDOMMapLike.cpp
@@ -71,6 +71,7 @@ void setToBackingMap(JSC::JSGlobalObject& lexicalGlobalObject, JSC::JSObject& ba
     JSC::MarkedArgumentBuffer arguments;
     arguments.append(key);
     arguments.append(value);
+    ASSERT(!arguments.hasOverflowed());
     JSC::call(&lexicalGlobalObject, function, callData, &backingMap, arguments);
 }
 

diff --git a/Source/WebCore/bindings/js/JSDOMPromise.cpp b/Source/WebCore/bindings/js/JSDOMPromise.cpp
@@ -66,6 +66,7 @@ auto DOMPromise::whenPromiseIsSettled(JSDOMGlobalObject* globalObject, JSC::JSOb
     JSC::MarkedArgumentBuffer arguments;
     arguments.append(handler);
     arguments.append(handler);
+    ASSERT(!arguments.hasOverflowed());
 
     auto callData = JSC::getCallData(thenFunction);
     ASSERT(callData.type != JSC::CallData::Type::None);

diff --git a/Source/WebCore/bindings/js/JSDOMSetLike.cpp b/Source/WebCore/bindings/js/JSDOMSetLike.cpp
@@ -74,6 +74,7 @@ void addToBackingSet(JSC::JSGlobalObject& lexicalGlobalObject, JSC::JSObject& ba
     ASSERT(callData.type != JSC::CallData::Type::None);
     JSC::MarkedArgumentBuffer arguments;
     arguments.append(item);
+    ASSERT(!arguments.hasOverflowed());
     JSC::call(&lexicalGlobalObject, function, callData, &backingSet, arguments);
 }
 

diff --git a/Source/WebCore/bindings/js/ReadableStreamDefaultController.cpp b/Source/WebCore/bindings/js/ReadableStreamDefaultController.cpp
@@ -62,6 +62,7 @@ void ReadableStreamDefaultController::close()
 {
     JSC::MarkedArgumentBuffer arguments;
     arguments.append(&jsController());
+    ASSERT(!arguments.hasOverflowed());
 
     auto* clientData = static_cast<JSVMClientData*>(globalObject().vm().clientData);
     auto& privateName = clientData->builtinFunctions().readableStreamInternalsBuiltins().readableStreamDefaultControllerClosePrivateName();
@@ -86,6 +87,7 @@ void ReadableStreamDefaultController::error(const Exception& exception)
     JSC::MarkedArgumentBuffer arguments;
     arguments.append(&jsController());
     arguments.append(value);
+    ASSERT(!arguments.hasOverflowed());
 
     auto* clientData = static_cast<JSVMClientData*>(vm.clientData);
     auto& privateName = clientData->builtinFunctions().readableStreamInternalsBuiltins().readableStreamDefaultControllerErrorPrivateName();
@@ -102,6 +104,7 @@ bool ReadableStreamDefaultController::enqueue(JSC::JSValue value)
     JSC::MarkedArgumentBuffer arguments;
     arguments.append(&jsController());
     arguments.append(value);
+    ASSERT(!arguments.hasOverflowed());
 
     auto* clientData = static_cast<JSVMClientData*>(lexicalGlobalObject.vm().clientData);
     auto& privateName = clientData->builtinFunctions().readableStreamInternalsBuiltins().readableStreamDefaultControllerEnqueuePrivateName();

diff --git a/Source/WebCore/bindings/js/ScriptController.cpp b/Source/WebCore/bindings/js/ScriptController.cpp
@@ -629,6 +629,7 @@ ValueOrException ScriptController::callInWorld(RunJavaScriptParameters&& paramet
         if (argument != parameters.arguments->end())
             functionStringBuilder.append(',');
     }
+    ASSERT(!markedArguments.hasOverflowed());
 
     if (!errorMessage.isEmpty())
         return makeUnexpected(ExceptionDetails { errorMessage });
@@ -751,6 +752,7 @@ void ScriptController::executeAsynchronousUserAgentScriptInWorld(DOMWrapperWorld
     JSC::MarkedArgumentBuffer arguments;
     arguments.append(fulfillHandler);
     arguments.append(rejectHandler);
+    ASSERT(!arguments.hasOverflowed());
 
     call(&globalObject, thenFunction, callData, result.value(), arguments);
 }