From 239b9d8662b67579665bdb11b890c0dee93716c2 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Sat, 11 May 2024 12:13:31 +0200 Subject: [PATCH] LibJS: Manually limit the recursion depth in Proxy Instead of relying on native stack overflows to kick us out of circular proxy chains, we now keep track of the recursion depth and kick ourselves out if it exceeds 10'000. This fixes an issue where compiler tail/sibling call optimizations would turn infinite recursion into infinite loops, and thus never hit a stack overflow to kick itself out. For whatever reason, we've only seen the issue on SerenityOS with UBSAN, but it could theoretically happen on any platform. --- .../Libraries/LibJS/Runtime/ProxyObject.cpp | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/Userland/Libraries/LibJS/Runtime/ProxyObject.cpp b/Userland/Libraries/LibJS/Runtime/ProxyObject.cpp index 80dfe64beb3..c511d9289ca 100644 --- a/Userland/Libraries/LibJS/Runtime/ProxyObject.cpp +++ b/Userland/Libraries/LibJS/Runtime/ProxyObject.cpp @@ -18,6 +18,24 @@ namespace JS { JS_DEFINE_ALLOCATOR(ProxyObject); +// NOTE: We can't rely on native stack overflows to catch infinite recursion in Proxy traps, +// since the compiler may decide to optimize tail/sibling calls into loops. +// So instead we keep track of the recursion depth and throw a TypeError if it exceeds a certain limit. + +static int s_recursion_depth = 0; + +struct RecursionDepthUpdater { + RecursionDepthUpdater() { ++s_recursion_depth; } + ~RecursionDepthUpdater() { --s_recursion_depth; } +}; + +#define LIMIT_PROXY_RECURSION_DEPTH() \ + RecursionDepthUpdater recursion_depth_updater; \ + do { \ + if (s_recursion_depth >= 10000) \ + return vm().throw_completion(ErrorType::CallStackSizeExceeded); \ + } while (0) + NonnullGCPtr ProxyObject::create(Realm& realm, Object& target, Object& handler) { return realm.heap().allocate(realm, target, handler, realm.intrinsics().object_prototype()); @@ -46,6 +64,8 @@ static Value property_key_to_value(VM& vm, PropertyKey const& property_key) // 10.5.1 [[GetPrototypeOf]] ( ), https://tc39.es/ecma262/#sec-proxy-object-internal-methods-and-internal-slots-getprototypeof ThrowCompletionOr ProxyObject::internal_get_prototype_of() const { + LIMIT_PROXY_RECURSION_DEPTH(); + auto& vm = this->vm(); // 1. Let handler be O.[[ProxyHandler]]. @@ -94,6 +114,8 @@ ThrowCompletionOr ProxyObject::internal_get_prototype_of() const // 10.5.2 [[SetPrototypeOf]] ( V ), https://tc39.es/ecma262/#sec-proxy-object-internal-methods-and-internal-slots-setprototypeof-v ThrowCompletionOr ProxyObject::internal_set_prototype_of(Object* prototype) { + LIMIT_PROXY_RECURSION_DEPTH(); + auto& vm = this->vm(); // 1. Let handler be O.[[ProxyHandler]]. @@ -142,6 +164,8 @@ ThrowCompletionOr ProxyObject::internal_set_prototype_of(Object* prototype // 10.5.3 [[IsExtensible]] ( ), https://tc39.es/ecma262/#sec-proxy-object-internal-methods-and-internal-slots-isextensible ThrowCompletionOr ProxyObject::internal_is_extensible() const { + LIMIT_PROXY_RECURSION_DEPTH(); + auto& vm = this->vm(); // 1. Let handler be O.[[ProxyHandler]]. @@ -179,6 +203,8 @@ ThrowCompletionOr ProxyObject::internal_is_extensible() const // 10.5.4 [[PreventExtensions]] ( ), https://tc39.es/ecma262/#sec-proxy-object-internal-methods-and-internal-slots-preventextensions ThrowCompletionOr ProxyObject::internal_prevent_extensions() { + LIMIT_PROXY_RECURSION_DEPTH(); + auto& vm = this->vm(); // 1. Let handler be O.[[ProxyHandler]]. @@ -219,6 +245,8 @@ ThrowCompletionOr ProxyObject::internal_prevent_extensions() // 10.5.5 [[GetOwnProperty]] ( P ), https://tc39.es/ecma262/#sec-proxy-object-internal-methods-and-internal-slots-getownproperty-p ThrowCompletionOr> ProxyObject::internal_get_own_property(PropertyKey const& property_key) const { + LIMIT_PROXY_RECURSION_DEPTH(); + auto& vm = this->vm(); VERIFY(property_key.is_valid()); @@ -310,6 +338,8 @@ ThrowCompletionOr> ProxyObject::internal_get_own_pr // 10.5.6 [[DefineOwnProperty]] ( P, Desc ), https://tc39.es/ecma262/#sec-proxy-object-internal-methods-and-internal-slots-defineownproperty-p-desc ThrowCompletionOr ProxyObject::internal_define_own_property(PropertyKey const& property_key, PropertyDescriptor const& property_descriptor) { + LIMIT_PROXY_RECURSION_DEPTH(); + auto& vm = this->vm(); VERIFY(property_key.is_valid()); @@ -392,6 +422,8 @@ ThrowCompletionOr ProxyObject::internal_define_own_property(PropertyKey co // 10.5.7 [[HasProperty]] ( P ), https://tc39.es/ecma262/#sec-proxy-object-internal-methods-and-internal-slots-hasproperty-p ThrowCompletionOr ProxyObject::internal_has_property(PropertyKey const& property_key) const { + LIMIT_PROXY_RECURSION_DEPTH(); + auto& vm = this->vm(); VERIFY(property_key.is_valid()); @@ -457,6 +489,8 @@ ThrowCompletionOr ProxyObject::internal_has_property(PropertyKey const& pr // 10.5.8 [[Get]] ( P, Receiver ), https://tc39.es/ecma262/#sec-proxy-object-internal-methods-and-internal-slots-get-p-receiver ThrowCompletionOr ProxyObject::internal_get(PropertyKey const& property_key, Value receiver, CacheablePropertyMetadata*, PropertyLookupPhase) const { + LIMIT_PROXY_RECURSION_DEPTH(); + // NOTE: We don't return any cacheable metadata for proxy lookups. VERIFY(!receiver.is_empty()); @@ -529,6 +563,8 @@ ThrowCompletionOr ProxyObject::internal_get(PropertyKey const& property_k // 10.5.9 [[Set]] ( P, V, Receiver ), https://tc39.es/ecma262/#sec-proxy-object-internal-methods-and-internal-slots-set-p-v-receiver ThrowCompletionOr ProxyObject::internal_set(PropertyKey const& property_key, Value value, Value receiver, CacheablePropertyMetadata*) { + LIMIT_PROXY_RECURSION_DEPTH(); + auto& vm = this->vm(); VERIFY(property_key.is_valid()); @@ -602,6 +638,8 @@ ThrowCompletionOr ProxyObject::internal_set(PropertyKey const& property_ke // 10.5.10 [[Delete]] ( P ), https://tc39.es/ecma262/#sec-proxy-object-internal-methods-and-internal-slots-delete-p ThrowCompletionOr ProxyObject::internal_delete(PropertyKey const& property_key) { + LIMIT_PROXY_RECURSION_DEPTH(); + auto& vm = this->vm(); VERIFY(property_key.is_valid()); @@ -656,6 +694,8 @@ ThrowCompletionOr ProxyObject::internal_delete(PropertyKey const& property // 10.5.11 [[OwnPropertyKeys]] ( ), https://tc39.es/ecma262/#sec-proxy-object-internal-methods-and-internal-slots-ownpropertykeys ThrowCompletionOr> ProxyObject::internal_own_property_keys() const { + LIMIT_PROXY_RECURSION_DEPTH(); + auto& vm = this->vm(); // 1. Let handler be O.[[ProxyHandler]]. @@ -776,6 +816,8 @@ ThrowCompletionOr> ProxyObject::internal_own_property_keys() // 10.5.12 [[Call]] ( thisArgument, argumentsList ), https://tc39.es/ecma262/#sec-proxy-object-internal-methods-and-internal-slots-call-thisargument-argumentslist ThrowCompletionOr ProxyObject::internal_call(Value this_argument, ReadonlySpan arguments_list) { + LIMIT_PROXY_RECURSION_DEPTH(); + auto& vm = this->vm(); auto& realm = *vm.current_realm(); @@ -820,6 +862,8 @@ bool ProxyObject::has_constructor() const // 10.5.13 [[Construct]] ( argumentsList, newTarget ), https://tc39.es/ecma262/#sec-proxy-object-internal-methods-and-internal-slots-construct-argumentslist-newtarget ThrowCompletionOr> ProxyObject::internal_construct(ReadonlySpan arguments_list, FunctionObject& new_target) { + LIMIT_PROXY_RECURSION_DEPTH(); + auto& vm = this->vm(); auto& realm = *vm.current_realm();