diff --git a/WORKSPACE b/WORKSPACE index e4bab45f47a..dca87ec3a5c 100644 --- a/WORKSPACE +++ b/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", diff --git a/patches/v8/0021-Add-ValueSerializer-SetTreatProxiesAsHostObjects.patch b/patches/v8/0021-Add-ValueSerializer-SetTreatProxiesAsHostObjects.patch new file mode 100644 index 00000000000..9bd273dc0d2 --- /dev/null +++ b/patches/v8/0021-Add-ValueSerializer-SetTreatProxiesAsHostObjects.patch @@ -0,0 +1,119 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Kenton Varda +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 ValueSerializer::WriteValue(Local context, + Local value) { + auto i_isolate = reinterpret_cast(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(tag); + WriteRawBytes(&raw_tag, sizeof(raw_tag)); +@@ -602,7 +606,12 @@ Maybe ValueSerializer::WriteJSReceiver(Handle receiver) { + InstanceType instance_type = receiver->map()->instance_type(); + if (IsCallable(*receiver)) { + if (treat_functions_as_host_objects_) { +- return WriteHostObject(Cast(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 ValueSerializer::WriteSharedObject( + return ThrowIfOutOfMemory(); + } + +-Maybe ValueSerializer::WriteHostObject(Handle object) { ++Maybe ValueSerializer::WriteHostObject(Handle 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 ExpandBuffer(size_t required_capacity); +@@ -159,7 +168,7 @@ class ValueSerializer { + #endif // V8_ENABLE_WEBASSEMBLY + Maybe WriteSharedObject(DirectHandle object) + V8_WARN_UNUSED_RESULT; +- Maybe WriteHostObject(Handle object) V8_WARN_UNUSED_RESULT; ++ Maybe WriteHostObject(Handle 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_; diff --git a/src/workerd/api/tests/js-rpc-test.js b/src/workerd/api/tests/js-rpc-test.js index ff6898c8401..52f3eb888ef 100644 --- a/src/workerd/api/tests/js-rpc-test.js +++ b/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); + } + }, +}; diff --git a/src/workerd/api/worker-rpc.c++ b/src/workerd/api/worker-rpc.c++ index d82068e5df8..9d1dc2b29c2 100644 --- a/src/workerd/api/worker-rpc.c++ +++ b/src/workerd/api/worker-rpc.c++ @@ -1600,6 +1600,66 @@ void RpcSerializerExternalHander::serializeFunction( }); } +void RpcSerializerExternalHander::serializeProxy( + jsg::Lock& js, jsg::Serializer& serializer, v8::Local 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()); + auto prototypeOfRpcTarget = js.getPrototypeFor(); + 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()) { + 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(rpc::SerializationTag::JS_RPC_STUB)); + + rpc::JsRpcTarget::Client cap = + kj::heap(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 { diff --git a/src/workerd/api/worker-rpc.h b/src/workerd/api/worker-rpc.h index 879648f75a7..e32e2d40517 100644 --- a/src/workerd/api/worker-rpc.h +++ b/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 func) override; + // We can serialize a Proxy if it happens to wrap RpcTarget. + void serializeProxy( + jsg::Lock& js, jsg::Serializer& serializer, v8::Local proxy) override; + private: GetStreamSinkFunc getStreamSinkFunc; diff --git a/src/workerd/jsg/jsg.h b/src/workerd/jsg/jsg.h index eed3c8e1827..c2672311c15 100644 --- a/src/workerd/jsg/jsg.h +++ b/src/workerd/jsg/jsg.h @@ -2574,6 +2574,13 @@ class Lock { virtual Ref domException( kj::String name, kj::String message, kj::Maybe 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 + 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 getInstance(v8::Local obj, const std::type_info& type) = 0; + virtual v8::Local getPrototypeFor(const std::type_info& type) = 0; }; // Ensures that the given fn is run within both a handlescope and the context scope. diff --git a/src/workerd/jsg/jsvalue.h b/src/workerd/jsg/jsvalue.h index 1c0a862ee0c..25b0fa2cc20 100644 --- a/src/workerd/jsg/jsvalue.h +++ b/src/workerd/jsg/jsvalue.h @@ -390,6 +390,12 @@ class JsObject final: public JsBase { JsObject jsonClone(Lock&); }; +// Defined here because `JsObject` is an incomplete type in `jsg.h`. +template +inline JsObject Lock::getPrototypeFor() { + return JsObject(getPrototypeFor(typeid(T))); +} + class JsMap final: public JsBase { public: operator JsObject(); diff --git a/src/workerd/jsg/ser.c++ b/src/workerd/jsg/ser.c++ index 0aea94095a0..3bb2f223914 100644 --- a/src/workerd/jsg/ser.c++ +++ b/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 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 Serializer::WriteHostObject(v8::Isolate* isolate, v8::LocalIsFunction()) { eh.serializeFunction(js, *this, object.As()); return v8::Just(true); + } else if (object->IsProxy()) { + eh.serializeProxy(js, *this, object.As()); + return v8::Just(true); } } diff --git a/src/workerd/jsg/ser.h b/src/workerd/jsg/ser.h index e5859bda49c..766b5195413 100644 --- a/src/workerd/jsg/ser.h +++ b/src/workerd/jsg/ser.h @@ -74,6 +74,19 @@ class Serializer final: v8::ValueSerializer::Delegate { // DataCloneError. virtual void serializeFunction( jsg::Lock& js, jsg::Serializer& serializer, v8::Local func); + + // Tries to serialize a proxy as an external. The default implementation throws + // DataCloneError. + // + // TODO(cleanup): This is a bit of a hack to support an RpcTarget that is wrapped in a Proxy. + // For RpcTarget specifically, this works because inheriting RpcTarget is just a marker that + // opts into serializing by creating a stub pointing at the object -- we can create a stub + // pointing at the proxy instead. For any other type, serializing a Proxy probably isn't + // possible, since the serialization wouldn't actually capture the Proxy logic? But I'm + // not 100% certain of that. If we find other use cases in the future it may turn out that + // they call for a different design. + virtual void serializeProxy( + jsg::Lock& js, jsg::Serializer& serializer, v8::Local proxy); }; struct Options { diff --git a/src/workerd/jsg/setup.h b/src/workerd/jsg/setup.h index 5ef6c7f5da4..68b5fd0ea68 100644 --- a/src/workerd/jsg/setup.h +++ b/src/workerd/jsg/setup.h @@ -631,6 +631,20 @@ class Isolate: public IsolateBase { instance->GetAlignedPointerFromInternalField(Wrappable::WRAPPED_OBJECT_FIELD_INDEX)); } } + + virtual v8::Local getPrototypeFor(const std::type_info& type) override { + v8::EscapableHandleScope scope(v8Isolate); + auto tmpl = jsgIsolate.wrapper->getDynamicTypeInfo(v8Isolate, type).tmpl; + auto constructor = JsObject(check(tmpl->GetFunction(v8Context()))); + + // Note that `constructor.getPrototype()` returns the prototype of the constructor itself, + // which is NOT the same as the prototype of the object it constructs. For the latter we + // need to access the `prototype` property. + auto proto = constructor.get(*this, "prototype"); + + KJ_ASSERT(proto.isObject()); + return scope.Escape(v8::Local(proto).As()); + } }; // The func must be a callback with the signature: T(jsg::Lock&)