From 2f07b10c6e2b822872cb3187f4bf92cd918dbf36 Mon Sep 17 00:00:00 2001
From: Marc Horowitz <mhorowitz@fb.com>
Date: Sat, 26 Sep 2020 11:25:14 -0700
Subject: [PATCH] Don't pass numeric form keys to JSProxy functions

Summary:
Internally, Hermes will sometimes keep keys as numbers for
efficiency.  In copyDataPropertiesSlowPath_RJS, which is for Proxy,
ensure numeric keys are converted to strings before calling functions
which might call into JSProxy.

Reviewed By: avp

Differential Revision: D23834086

fbshipit-source-id: b1df8e16573c15b63e3993793f6f484848bb89af
---
 lib/VM/JSLib/HermesBuiltin.cpp |  9 +++++++++
 test/hermes/proxy.js           | 14 ++++++++++++++
 2 files changed, 23 insertions(+)

diff --git a/lib/VM/JSLib/HermesBuiltin.cpp b/lib/VM/JSLib/HermesBuiltin.cpp
index 5b63304b4a6..0adaaa60534 100644
--- a/lib/VM/JSLib/HermesBuiltin.cpp
+++ b/lib/VM/JSLib/HermesBuiltin.cpp
@@ -275,6 +275,15 @@ CallResult<HermesValue> copyDataPropertiesSlowPath_RJS(
        ++nextKeyIdx) {
     marker.flush();
     nextKeyHandle = keys->at(runtime, nextKeyIdx);
+    if (nextKeyHandle->isNumber()) {
+      CallResult<PseudoHandle<StringPrimitive>> strRes =
+          toString_RJS(runtime, nextKeyHandle);
+      if (LLVM_UNLIKELY(strRes == ExecutionStatus::EXCEPTION)) {
+        return ExecutionStatus::EXCEPTION;
+      }
+      nextKeyHandle = strRes->getHermesValue();
+    }
+
     // b. For each element e of excludedItems in List order, do
     //   i. If SameValue(e, nextKey) is true, then
     //     1. Set excluded to true.
diff --git a/test/hermes/proxy.js b/test/hermes/proxy.js
index a7db5e38e5f..db1eb77f390 100644
--- a/test/hermes/proxy.js
+++ b/test/hermes/proxy.js
@@ -2143,6 +2143,20 @@ f.a = 1;
 f.b = 2;
 checkDeep({...f})(_ => ({a:1, b:2}));
 
+// spread passes string not numeric arguments to traps
+var output = [];
+var p = new Proxy({1:""}, {
+        getOwnPropertyDescriptor(t, k) {
+            output.push(typeof k)
+            return Object.getOwnPropertyDescriptor(t, k);
+        },
+        get(t, k) {
+            output.push(typeof k)
+            return t[k];
+        }});
+({...p});
+assert.arrayEqual(output, ["string", "string"]);
+
 // newTarget.prototype for Proxy ctor is !== Object.prototype does not throw
 Reflect.construct(Proxy, [{}, {}], WeakSet);