Merge pull request #3212 from cloudflare/kenton/proxied-rpc-target

JSRPC: Allow wrapping RPC targets in Proxy objects.

WORKSPACE

@@ -303,6 +303,7 @@ http_archive(
         "//:patches/v8/0018-Return-rejected-promise-from-WebAssembly.compile-if-.patch",
         "//:patches/v8/0019-codegen-Don-t-pass-a-nullptr-in-InitUnwindingRecord-.patch",
         "//:patches/v8/0020-Add-another-slot-in-the-isolate-for-embedder.patch",
+        "//:patches/v8/0021-Add-ValueSerializer-SetTreatProxiesAsHostObjects.patch",
     ],
     strip_prefix = "v8-13.1.201.8",
     url = "https://github.com/v8/v8/archive/refs/tags/13.1.201.8.tar.gz",

patches/v8/0021-Add-ValueSerializer-SetTreatProxiesAsHostObjects.patch

@@ -0,0 +1,119 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Kenton Varda <[email protected]>
+Date: Wed, 4 Dec 2024 22:36:05 -0600
+Subject: Add ValueSerializer::SetTreatProxiesAsHostObjects().
+
+Previously, ValueSerializer would always refuse to serialize Proxy objects. This commit gives the embedder the option to handle them as host objects.
+
+Similar to the previous patch adding `SetTreatFunctionsAsHostObjects()`, this is intended for use in an RPC system, where an arbitrary object can be "serialized" by replacing it with a stub which, when invoked, performs an RPC back to the originating isolate in order to access the original object there.
+
+diff --git a/include/v8-value-serializer.h b/include/v8-value-serializer.h
+index 141f138e08de849e3e02b3b2b346e643b9e40c70..bdcb2831c55e21c6d511f56dfc79a5076871f05a 100644
+--- a/include/v8-value-serializer.h
++++ b/include/v8-value-serializer.h
+@@ -204,6 +204,15 @@ class V8_EXPORT ValueSerializer {
+    */
+   void SetTreatFunctionsAsHostObjects(bool mode);
+
++  /**
++   * Indicate whether to treat Proxies as host objects,
++   * i.e. pass them to Delegate::WriteHostObject. This should not be
++   * called when no Delegate was passed.
++   *
++   * The default is not to treat Proxies as host objects.
++   */
++  void SetTreatProxiesAsHostObjects(bool mode);
++
+   /**
+    * Write raw data in various common formats to the buffer.
+    * Note that integer types are written in base-128 varint format, not with a
+diff --git a/src/api/api.cc b/src/api/api.cc
+index 998b6c91a854b24c01ee58495056dca36a145042..53b8af04828bad7a1f4e18c51648add276644ae9 100644
+--- a/src/api/api.cc
++++ b/src/api/api.cc
+@@ -3570,6 +3570,10 @@ void ValueSerializer::SetTreatFunctionsAsHostObjects(bool mode) {
+   private_->serializer.SetTreatFunctionsAsHostObjects(mode);
+ }
+
++void ValueSerializer::SetTreatProxiesAsHostObjects(bool mode) {
++  private_->serializer.SetTreatProxiesAsHostObjects(mode);
++}
++
+ Maybe<bool> ValueSerializer::WriteValue(Local<Context> context,
+                                         Local<Value> value) {
+   auto i_isolate = reinterpret_cast<i::Isolate*>(context->GetIsolate());
+diff --git a/src/objects/value-serializer.cc b/src/objects/value-serializer.cc
+index 531331ee18bac95e4c6bd13f3e91fb9015765393..f7a126205649dfc991d812bbd1820219de980b32 100644
+--- a/src/objects/value-serializer.cc
++++ b/src/objects/value-serializer.cc
+@@ -332,6 +332,10 @@ void ValueSerializer::SetTreatFunctionsAsHostObjects(bool mode) {
+   treat_functions_as_host_objects_ = mode;
+ }
+
++void ValueSerializer::SetTreatProxiesAsHostObjects(bool mode) {
++  treat_proxies_as_host_objects_ = mode;
++}
++
+ void ValueSerializer::WriteTag(SerializationTag tag) {
+   uint8_t raw_tag = static_cast<uint8_t>(tag);
+   WriteRawBytes(&raw_tag, sizeof(raw_tag));
+@@ -602,7 +606,12 @@ Maybe<bool> ValueSerializer::WriteJSReceiver(Handle<JSReceiver> receiver) {
+   InstanceType instance_type = receiver->map()->instance_type();
+   if (IsCallable(*receiver)) {
+     if (treat_functions_as_host_objects_) {
+-      return WriteHostObject(Cast<JSObject>(receiver));
++      return WriteHostObject(receiver);
++    }
++    return ThrowDataCloneError(MessageTemplate::kDataCloneError, receiver);
++  } else if (instance_type == JS_PROXY_TYPE) {
++    if (treat_proxies_as_host_objects_) {
++      return WriteHostObject(receiver);
+     }
+     return ThrowDataCloneError(MessageTemplate::kDataCloneError, receiver);
+   } else if (IsSpecialReceiverInstanceType(instance_type) &&
+@@ -1207,7 +1216,7 @@ Maybe<bool> ValueSerializer::WriteSharedObject(
+   return ThrowIfOutOfMemory();
+ }
+
+-Maybe<bool> ValueSerializer::WriteHostObject(Handle<JSObject> object) {
++Maybe<bool> ValueSerializer::WriteHostObject(Handle<JSReceiver> object) {
+   WriteTag(SerializationTag::kHostObject);
+   if (!delegate_) {
+     isolate_->Throw(*isolate_->factory()->NewError(
+diff --git a/src/objects/value-serializer.h b/src/objects/value-serializer.h
+index a657bb7f6def9d795d6504eb62f49b5b34c46406..31406f1d3f430b501fe49fa207e2b8fee2ea57da 100644
+--- a/src/objects/value-serializer.h
++++ b/src/objects/value-serializer.h
+@@ -111,6 +111,15 @@ class ValueSerializer {
+    */
+   void SetTreatFunctionsAsHostObjects(bool mode);
+
++  /*
++   * Indicate whether to treat Proxies as host objects,
++   * i.e. pass them to Delegate::WriteHostObject. This should not be
++   * called when no Delegate was passed.
++   *
++   * The default is not to treat Proxies as host objects.
++   */
++  void SetTreatProxiesAsHostObjects(bool mode);
++
+  private:
+   // Managing allocations of the internal buffer.
+   Maybe<bool> ExpandBuffer(size_t required_capacity);
+@@ -159,7 +168,7 @@ class ValueSerializer {
+ #endif  // V8_ENABLE_WEBASSEMBLY
+   Maybe<bool> WriteSharedObject(DirectHandle<HeapObject> object)
+       V8_WARN_UNUSED_RESULT;
+-  Maybe<bool> WriteHostObject(Handle<JSObject> object) V8_WARN_UNUSED_RESULT;
++  Maybe<bool> WriteHostObject(Handle<JSReceiver> object) V8_WARN_UNUSED_RESULT;
+
+   /*
+    * Reads the specified keys from the object and writes key-value pairs to the
+@@ -192,6 +201,7 @@ class ValueSerializer {
+   bool has_custom_host_objects_ = false;
+   bool treat_array_buffer_views_as_host_objects_ = false;
+   bool treat_functions_as_host_objects_ = false;
++  bool treat_proxies_as_host_objects_ = false;
+   bool out_of_memory_ = false;
+   Zone zone_;
+   uint32_t version_;

src/workerd/api/tests/js-rpc-test.js

@@ -55,6 +55,11 @@ class NonRpcClass {
       },
     };
   }
+
+  i = 0;
+  increment(i) {
+    this.i += i;
+  }
 }
 
 export let nonClass = {
@@ -1550,3 +1555,114 @@ export let domExceptionClone = {
     assert.strictEqual(de2.foo, undefined);
   },
 };
+
+export let proxiedRpcTarget = {
+  async test(controller, env, ctx) {
+    // Proxy RPC target.
+    {
+      let counter = new MyCounter(0);
+      let proxy = new Proxy(counter, {
+        get(target, prop, receiver) {
+          if (prop == 'increment') {
+            return (i) => target.increment(i + 123);
+          } else {
+            let result = target[prop];
+            if (result instanceof Function) {
+              result = result.bind(target);
+            }
+            return result;
+          }
+        },
+      });
+
+      await env.MyService.incrementCounter(proxy, 1);
+
+      assert.strictEqual(counter.i, 124);
+    }
+
+    // Proxy plain object.
+    {
+      let counter = {
+        i: 0,
+        increment(i) {
+          this.i += i;
+        },
+      };
+
+      let proxy = new Proxy(counter, {
+        get(target, prop, receiver) {
+          if (prop == 'increment') {
+            return (i) => target.increment(i + 123);
+          } else {
+            let result = target[prop];
+            if (result instanceof Function) {
+              result = result.bind(target);
+            }
+            return result;
+          }
+        },
+      });
+
+      await env.MyService.incrementCounter(proxy, 1);
+
+      assert.strictEqual(counter.i, 124);
+    }
+
+    // Can't proxy a class that doesn't extend `RpcTarget`.
+    {
+      let nonRpc = new NonRpcClass();
+      let proxy = new Proxy(nonRpc, {
+        get(target, prop, receiver) {
+          if (prop == 'increment') {
+            return (i) => target.increment(i + 123);
+          } else {
+            let result = target[prop];
+            if (result instanceof Function) {
+              result = result.bind(target);
+            }
+            return result;
+          }
+        },
+      });
+
+      await assert.rejects(() => env.MyService.incrementCounter(proxy, 1), {
+        name: 'DataCloneError',
+        message:
+          'Proxy could not be serialized because it is not a valid RPC receiver type. The ' +
+          'Proxy must emulate either a plain object or an RpcTarget, as indicated by the ' +
+          "Proxy's prototype chain.",
+      });
+    }
+
+    // Can't proxy an RpcTarget if we've overridden the prototype to say it's something else.
+    {
+      let counter = new MyCounter(0);
+      let proxy = new Proxy(counter, {
+        getPrototypeOf(target) {
+          return NonRpcClass.prototype;
+        },
+      });
+
+      await assert.rejects(() => env.MyService.incrementCounter(proxy, 1), {
+        name: 'DataCloneError',
+        message:
+          'Proxy could not be serialized because it is not a valid RPC receiver type. The ' +
+          'Proxy must emulate either a plain object or an RpcTarget, as indicated by the ' +
+          "Proxy's prototype chain.",
+      });
+    }
+
+    // CAN proxy a class that doesn't extend `RpcTarget` if we fake the prototype.
+    {
+      let nonRpc = new NonRpcClass();
+      let proxy = new Proxy(nonRpc, {
+        getPrototypeOf(target) {
+          return RpcTarget.prototype;
+        },
+      });
+
+      await env.MyService.incrementCounter(proxy, 321);
+      assert.strictEqual(nonRpc.i, 321);
+    }
+  },
+};

src/workerd/api/worker-rpc.c++

@@ -1600,6 +1600,66 @@ void RpcSerializerExternalHander::serializeFunction(
   });
 }
 
+void RpcSerializerExternalHander::serializeProxy(
+    jsg::Lock& js, jsg::Serializer& serializer, v8::Local<v8::Proxy> proxy) {
+  js.withinHandleScope([&]() {
+    auto handle = jsg::JsObject(proxy);
+
+    // Proxies are only allowed to wrap objects that would normally be serialized by writing a
+    // stub, e.g. plain objects and RpcTargets. In such cases, we can write a stub pointing to the
+    // proxy.
+    //
+    // However, note that we don't actually want to test the Proxy's *target* directly, because
+    // it's possible the Proxy is trying to disguise the target as something else. Instead, we must
+    // determine the type by following the prototype chain. That way, if the Proxy overrides
+    // getPrototype(), we will honor that override.
+    //
+    // Note that we don't support functions. This is because our isFunctionForRpc() check is not
+    // prototype-based, and as such it's unclear how exactly we should go about checking for a
+    // function here. Luckily, you really don't need to use a `Proxy` to wrap a function... you
+    // can just use a function.
+
+    // TODO(perf): We should really cache `prototypeOfObject` somewhere so we don't have to create
+    //   an object to get it. (We do this other places in this file, too...)
+    auto prototypeOfObject = KJ_ASSERT_NONNULL(js.obj().getPrototype(js).tryCast<jsg::JsObject>());
+    auto prototypeOfRpcTarget = js.getPrototypeFor<JsRpcTarget>();
+    bool allowInstanceProperties = false;
+    auto proto = handle.getPrototype(js);
+    if (proto == prototypeOfObject) {
+      // A regular object. Allow access to instance properties.
+      allowInstanceProperties = true;
+    } else {
+      // Walk the prototype chain looking for RpcTarget.
+      for (;;) {
+        if (proto == prototypeOfRpcTarget) {
+          // An RpcTarget, don't allow instance properties.
+          allowInstanceProperties = false;
+          break;
+        }
+
+        KJ_IF_SOME(protoObj, proto.tryCast<jsg::JsObject>()) {
+          proto = protoObj.getPrototype(js);
+        } else {
+          // End of prototype chain, and didn't find RpcTarget.
+          JSG_FAIL_REQUIRE(DOMDataCloneError,
+              "Proxy could not be serialized because it is not a valid RPC receiver type. The "
+              "Proxy must emulate either a plain object or an RpcTarget, as indicated by the "
+              "Proxy's prototype chain.");
+        }
+      }
+    }
+
+    // Great, we've concluded we can indeed point a stub at this proxy.
+    serializer.writeRawUint32(static_cast<uint>(rpc::SerializationTag::JS_RPC_STUB));
+
+    rpc::JsRpcTarget::Client cap =
+        kj::heap<TransientJsRpcTarget>(js, IoContext::current(), handle, allowInstanceProperties);
+    write([cap = kj::mv(cap)](rpc::JsValue::External::Builder builder) mutable {
+      builder.setRpcTarget(kj::mv(cap));
+    });
+  });
+}
+
 // JsRpcTarget implementation specific to entrypoints. This is used to deliver the first, top-level
 // call of an RPC session.
 class EntrypointJsRpcTarget final: public JsRpcTargetBase {

src/workerd/api/worker-rpc.h

@@ -66,6 +66,10 @@ class RpcSerializerExternalHander final: public jsg::Serializer::ExternalHandler
   void serializeFunction(
       jsg::Lock& js, jsg::Serializer& serializer, v8::Local<v8::Function> func) override;
 
+  // We can serialize a Proxy if it happens to wrap RpcTarget.
+  void serializeProxy(
+      jsg::Lock& js, jsg::Serializer& serializer, v8::Local<v8::Proxy> proxy) override;
+
  private:
   GetStreamSinkFunc getStreamSinkFunc;
 

src/workerd/jsg/jsg.h

@@ -2574,6 +2574,13 @@ class Lock {
   virtual Ref<DOMException> domException(
       kj::String name, kj::String message, kj::Maybe<kj::String> stackValue = kj::none) = 0;
 
+  // Get the prototype object for the given C++ type (which must be a JSG_RESOURCE_TYPE).
+  //
+  // WARNING: A malicious script can tamper with this by overwriting the `prototype` property
+  // of the class object.
+  template <typename T>
+  JsObject getPrototypeFor();
+
   // ====================================================================================
   JsObject global() KJ_WARN_UNUSED_RESULT;
   JsValue undefined() KJ_WARN_UNUSED_RESULT;
@@ -2688,6 +2695,7 @@ class Lock {
 
   friend class JsObject;
   virtual kj::Maybe<Object&> getInstance(v8::Local<v8::Object> obj, const std::type_info& type) = 0;
+  virtual v8::Local<v8::Object> getPrototypeFor(const std::type_info& type) = 0;
 };
 
 // Ensures that the given fn is run within both a handlescope and the context scope.

src/workerd/jsg/jsvalue.h

@@ -390,6 +390,12 @@ class JsObject final: public JsBase<v8::Object, JsObject> {
   JsObject jsonClone(Lock&);
 };
 
+// Defined here because `JsObject` is an incomplete type in `jsg.h`.
+template <typename T>
+inline JsObject Lock::getPrototypeFor() {
+  return JsObject(getPrototypeFor(typeid(T)));
+}
+
 class JsMap final: public JsBase<v8::Map, JsMap> {
  public:
   operator JsObject();

src/workerd/jsg/ser.c++

@@ -13,6 +13,11 @@ void Serializer::ExternalHandler::serializeFunction(
   JSG_FAIL_REQUIRE(DOMDataCloneError, func, " could not be cloned.");
 }
 
+void Serializer::ExternalHandler::serializeProxy(
+    jsg::Lock& js, jsg::Serializer& serializer, v8::Local<v8::Proxy> proxy) {
+  JSG_FAIL_REQUIRE(DOMDataCloneError, proxy, " could not be cloned.");
+}
+
 Serializer::Serializer(Lock& js, Options options)
     : externalHandler(options.externalHandler),
       treatClassInstancesAsPlainObjects(options.treatClassInstancesAsPlainObjects),
@@ -26,6 +31,7 @@ Serializer::Serializer(Lock& js, Options options)
   if (externalHandler != kj::none) {
     // If we have an ExternalHandler, we'll ask it to serialize host objects.
     ser.SetTreatFunctionsAsHostObjects(true);
+    ser.SetTreatProxiesAsHostObjects(true);
   }
   KJ_IF_SOME(version, options.version) {
     KJ_ASSERT(version >= 13, "The minimum serialization version is 13.");
@@ -108,6 +114,9 @@ v8::Maybe<bool> Serializer::WriteHostObject(v8::Isolate* isolate, v8::Local<v8::
         if (object->IsFunction()) {
           eh.serializeFunction(js, *this, object.As<v8::Function>());
           return v8::Just(true);
+        } else if (object->IsProxy()) {
+          eh.serializeProxy(js, *this, object.As<v8::Proxy>());
+          return v8::Just(true);
         }
       }