From 2aaae6fc7037ef2088fa01bb83a244c7c6d26e98 Mon Sep 17 00:00:00 2001
From: Luke Wilde <lukew@serenityos.org>
Date: Tue, 29 Aug 2023 21:18:31 +0100
Subject: [PATCH] LibJS: Avoid ToPropertyKey for spreading in
 PutByValue(WithThis)

This is not we're supposed to do according to https://tc39.es/ecma262/#sec-runtime-semantics-propertydefinitionevaluation
Furthermore, this was observable by ToPrimitive looking up toString and
valueOf and potentially calling them if they exist. The big ticket
issue however is that for objects without toString and valueOf, such as
null-proto objects, this would unexpectedly throw.
---
 Userland/Libraries/LibJS/Bytecode/Op.cpp      |  4 ++--
 .../Libraries/LibJS/Tests/object-spread.js    | 22 ++++++++++++++++++-
 2 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/Userland/Libraries/LibJS/Bytecode/Op.cpp b/Userland/Libraries/LibJS/Bytecode/Op.cpp
index 10e371824a..1125ee7e9f 100644
--- a/Userland/Libraries/LibJS/Bytecode/Op.cpp
+++ b/Userland/Libraries/LibJS/Bytecode/Op.cpp
@@ -1183,7 +1183,7 @@ ThrowCompletionOr<void> PutByValue::execute_impl(Bytecode::Interpreter& interpre
 
     auto base = interpreter.reg(m_base);
 
-    auto property_key = TRY(interpreter.reg(m_property).to_property_key(vm));
+    auto property_key = m_kind != PropertyKind::Spread ? TRY(interpreter.reg(m_property).to_property_key(vm)) : PropertyKey {};
     TRY(put_by_property_key(vm, base, base, value, property_key, m_kind));
     interpreter.accumulator() = value;
     return {};
@@ -1198,7 +1198,7 @@ ThrowCompletionOr<void> PutByValueWithThis::execute_impl(Bytecode::Interpreter&
 
     auto base = interpreter.reg(m_base);
 
-    auto property_key = TRY(interpreter.reg(m_property).to_property_key(vm));
+    auto property_key = m_kind != PropertyKind::Spread ? TRY(interpreter.reg(m_property).to_property_key(vm)) : PropertyKey {};
     TRY(put_by_property_key(vm, base, interpreter.reg(m_this_value), value, property_key, m_kind));
     interpreter.accumulator() = value;
     return {};
diff --git a/Userland/Libraries/LibJS/Tests/object-spread.js b/Userland/Libraries/LibJS/Tests/object-spread.js
index c34f932226..e55f38f9e0 100644
--- a/Userland/Libraries/LibJS/Tests/object-spread.js
+++ b/Userland/Libraries/LibJS/Tests/object-spread.js
@@ -144,7 +144,7 @@ describe("modification of spreadable objects during spread", () => {
         expect(Object.getOwnPropertyNames(result)).toContain("bar");
     });
 
-    test.xfail("spreading array", () => {
+    test("spreading array", () => {
         const array = [0];
         array[2] = 2;
         array[999] = 999;
@@ -192,3 +192,23 @@ test("allows assignment expressions", () => {
     expect("({ ...a ??= 'hello' })").toEval();
     expect("function* test() { return ({ ...yield a }); }").toEval();
 });
+
+test("spreading null-proto objects", () => {
+    const obj = {
+        __proto__: null,
+        hello: "world",
+        friends: "well hello",
+        toString() {
+            expect().fail("called toString()");
+        },
+        valueOf() {
+            expect().fail("called valueOf()");
+        },
+    };
+    let res;
+    expect(() => {
+        res = { ...obj };
+    }).not.toThrow();
+    expect(res).toHaveProperty("hello", "world");
+    expect(res).toHaveProperty("friends", "well hello");
+});