From 5d2e9156239cd707a22ecea6c87d48e5fc1cbe84 Mon Sep 17 00:00:00 2001
From: Maciej <sppmacd@pm.me>
Date: Thu, 4 May 2023 14:33:20 +0200
Subject: [PATCH] LibJS: Guard against stack overflow in ProxyObject
 set_property()

For similar reason as in the previous commit.
---
 .../Libraries/LibJS/Runtime/ProxyObject.cpp    | 18 +++++++++++++++++-
 .../Tests/builtins/Proxy/Proxy.handler-set.js  | 17 +++++++++++++++++
 2 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/Userland/Libraries/LibJS/Runtime/ProxyObject.cpp b/Userland/Libraries/LibJS/Runtime/ProxyObject.cpp
index 0bf1588fb9..f23d0a852c 100644
--- a/Userland/Libraries/LibJS/Runtime/ProxyObject.cpp
+++ b/Userland/Libraries/LibJS/Runtime/ProxyObject.cpp
@@ -470,7 +470,7 @@ ThrowCompletionOr<Value> ProxyObject::internal_get(PropertyKey const& property_k
     // 3. Assert: Type(handler) is Object.
     // 4. Let target be O.[[ProxyTarget]].
 
-    // NOTE: We need to protect ourselves from a Proxy with the handler's prototype set to the
+    // NOTE: We need to protect ourselves from a Proxy with its (or handler's) prototype set to the
     // Proxy itself, which would by default bounce between these functions indefinitely and lead to
     // a stack overflow when the Proxy's (p) or Proxy handler's (h) Object::get() is called and the
     // handler doesn't have a `get` trap:
@@ -539,6 +539,22 @@ ThrowCompletionOr<bool> ProxyObject::internal_set(PropertyKey const& property_ke
     // 3. Assert: Type(handler) is Object.
     // 4. Let target be O.[[ProxyTarget]].
 
+    // NOTE: We need to protect ourselves from a Proxy with its prototype set to the
+    // Proxy itself, which would by default bounce between these functions indefinitely and lead to
+    // a stack overflow when the Proxy's (p) or Proxy handler's (h) Object::get() is called and the
+    // handler doesn't have a `has` trap:
+    //
+    // 1. p -> ProxyObject::internal_set()  <- you are here
+    // 2. target -> Object::internal_set()
+    // 3. target -> Object::ordinary_set_with_own_descriptor()
+    // 4. target.[[Prototype]] -> Object::internal_set()
+    // 5. target.[[Prototype]] -> Object::ordinary_set_with_own_descriptor()
+    // 6. target.[[Prototype]].[[Prototype]] (which is ProxyObject) -> Object::internal_set()
+    //
+    // In JS code: `const proxy = new Proxy({}, {}); proxy.__proto__ = Object.create(proxy); proxy["foo"] = "bar";`
+    if (vm.did_reach_stack_space_limit())
+        return vm.throw_completion<InternalError>(ErrorType::CallStackSizeExceeded);
+
     // 5. Let trap be ? GetMethod(handler, "set").
     auto trap = TRY(Value(m_handler).get_method(vm, vm.names.set));
 
diff --git a/Userland/Libraries/LibJS/Tests/builtins/Proxy/Proxy.handler-set.js b/Userland/Libraries/LibJS/Tests/builtins/Proxy/Proxy.handler-set.js
index 093d4b0e09..c067211c4f 100644
--- a/Userland/Libraries/LibJS/Tests/builtins/Proxy/Proxy.handler-set.js
+++ b/Userland/Libraries/LibJS/Tests/builtins/Proxy/Proxy.handler-set.js
@@ -112,3 +112,20 @@ describe("[[Set]] invariants", () => {
         );
     });
 });
+
+test("Proxy handler that has the Proxy itself as its prototype", () => {
+    const handler = {};
+    const proxy = new Proxy({}, handler);
+    handler.__proto__ = proxy;
+    expect(() => {
+        proxy["foo"] = "bar";
+    }).toThrowWithMessage(InternalError, "Call stack size limit exceeded");
+});
+
+test("Proxy that has the Proxy itself as its prototype", () => {
+    const proxy = new Proxy({}, {});
+    proxy.__proto__ = Object.create(proxy);
+    expect(() => {
+        proxy["foo"] = "bar";
+    }).toThrowWithMessage(InternalError, "Call stack size limit exceeded");
+});