diff --git a/src/js/internal/streams/native-readable.ts b/src/js/internal/streams/native-readable.ts index 65307212b821..c9e960c75775 100644 --- a/src/js/internal/streams/native-readable.ts +++ b/src/js/internal/streams/native-readable.ts @@ -66,7 +66,9 @@ function constructNativeReadable(readableStream: ReadableStream, options): Nativ stream.debugId = ++debugId; } - stream.$bunNativePtr = bunNativePtr; + // Define the own property directly: an ordinary put would walk the prototype + // chain, which user code can graft onto ReadableStream.prototype's accessors. + $putByIdDirectPrivate(stream, "bunNativePtr", bunNativePtr); stream[kRefCount] = 0; stream[kConstructed] = false; stream[kPendingRead] = false; diff --git a/src/jsc/bindings/webcore/streams/JSReadableStream.cpp b/src/jsc/bindings/webcore/streams/JSReadableStream.cpp index d3d81565f422..007dc119037f 100644 --- a/src/jsc/bindings/webcore/streams/JSReadableStream.cpp +++ b/src/jsc/bindings/webcore/streams/JSReadableStream.cpp @@ -786,48 +786,78 @@ JSC_DEFINE_HOST_FUNCTION(jsReadableStreamPrototypeFunction_blob, (JSGlobalObject } // Bun private-name accessors ($bunNativePtr / $bunNativeType / $disturbed). +// JSC brand-checks DOMAttribute getters (PropertySlot::customGetter) but invokes +// custom setters with any receiver inheriting the accessor, so each one validates +// thisValue itself. -JSC_DEFINE_CUSTOM_GETTER(jsReadableStreamPrototype_nativePtrGetter, (JSGlobalObject*, JSC::EncodedJSValue thisValue, PropertyName)) +JSC_DEFINE_CUSTOM_GETTER(jsReadableStreamPrototype_nativePtrGetter, (JSGlobalObject * lexicalGlobalObject, JSC::EncodedJSValue thisValue, PropertyName propertyName)) { - auto* stream = uncheckedDowncast(JSValue::decode(thisValue)); + auto& vm = JSC::getVM(lexicalGlobalObject); + auto scope = DECLARE_THROW_SCOPE(vm); + auto* stream = dynamicDowncast(JSValue::decode(thisValue)); + if (!stream) [[unlikely]] + return throwVMDOMAttributeGetterTypeError(lexicalGlobalObject, scope, JSReadableStream::info(), propertyName); JSValue nativePtr = stream->nativePtrForJS(); return JSValue::encode(nativePtr.isEmpty() ? jsUndefined() : nativePtr); } -JSC_DEFINE_CUSTOM_SETTER(jsReadableStreamPrototype_nativePtrSetter, (JSGlobalObject * lexicalGlobalObject, JSC::EncodedJSValue thisValue, JSC::EncodedJSValue encodedValue, PropertyName)) +JSC_DEFINE_CUSTOM_SETTER(jsReadableStreamPrototype_nativePtrSetter, (JSGlobalObject * lexicalGlobalObject, JSC::EncodedJSValue thisValue, JSC::EncodedJSValue encodedValue, PropertyName propertyName)) { auto& vm = JSC::getVM(lexicalGlobalObject); - auto* stream = uncheckedDowncast(JSValue::decode(thisValue)); + auto scope = DECLARE_THROW_SCOPE(vm); + auto* stream = dynamicDowncast(JSValue::decode(thisValue)); + if (!stream) [[unlikely]] { + throwDOMAttributeSetterTypeError(lexicalGlobalObject, scope, JSReadableStream::info(), propertyName); + return false; + } stream->m_nativePtr.set(vm, stream, JSValue::decode(encodedValue)); return true; } -JSC_DEFINE_CUSTOM_GETTER(jsReadableStreamPrototype_nativeTypeGetter, (JSGlobalObject*, JSC::EncodedJSValue thisValue, PropertyName)) +JSC_DEFINE_CUSTOM_GETTER(jsReadableStreamPrototype_nativeTypeGetter, (JSGlobalObject * lexicalGlobalObject, JSC::EncodedJSValue thisValue, PropertyName propertyName)) { - const auto* stream = uncheckedDowncast(JSValue::decode(thisValue)); + auto& vm = JSC::getVM(lexicalGlobalObject); + auto scope = DECLARE_THROW_SCOPE(vm); + const auto* stream = dynamicDowncast(JSValue::decode(thisValue)); + if (!stream) [[unlikely]] + return throwVMDOMAttributeGetterTypeError(lexicalGlobalObject, scope, JSReadableStream::info(), propertyName); return JSValue::encode(jsNumber(stream->m_nativeType)); } -JSC_DEFINE_CUSTOM_SETTER(jsReadableStreamPrototype_nativeTypeSetter, (JSGlobalObject * lexicalGlobalObject, JSC::EncodedJSValue thisValue, JSC::EncodedJSValue encodedValue, PropertyName)) +JSC_DEFINE_CUSTOM_SETTER(jsReadableStreamPrototype_nativeTypeSetter, (JSGlobalObject * lexicalGlobalObject, JSC::EncodedJSValue thisValue, JSC::EncodedJSValue encodedValue, PropertyName propertyName)) { auto& vm = JSC::getVM(lexicalGlobalObject); auto scope = DECLARE_THROW_SCOPE(vm); - auto* stream = uncheckedDowncast(JSValue::decode(thisValue)); + auto* stream = dynamicDowncast(JSValue::decode(thisValue)); + if (!stream) [[unlikely]] { + throwDOMAttributeSetterTypeError(lexicalGlobalObject, scope, JSReadableStream::info(), propertyName); + return false; + } int32_t nativeType = JSValue::decode(encodedValue).toInt32(lexicalGlobalObject); RETURN_IF_EXCEPTION(scope, false); stream->m_nativeType = nativeType; return true; } -JSC_DEFINE_CUSTOM_GETTER(jsReadableStreamPrototype_disturbedGetter, (JSGlobalObject*, JSC::EncodedJSValue thisValue, PropertyName)) +JSC_DEFINE_CUSTOM_GETTER(jsReadableStreamPrototype_disturbedGetter, (JSGlobalObject * lexicalGlobalObject, JSC::EncodedJSValue thisValue, PropertyName propertyName)) { - const auto* stream = uncheckedDowncast(JSValue::decode(thisValue)); + auto& vm = JSC::getVM(lexicalGlobalObject); + auto scope = DECLARE_THROW_SCOPE(vm); + const auto* stream = dynamicDowncast(JSValue::decode(thisValue)); + if (!stream) [[unlikely]] + return throwVMDOMAttributeGetterTypeError(lexicalGlobalObject, scope, JSReadableStream::info(), propertyName); return JSValue::encode(jsBoolean(stream->m_disturbed)); } -JSC_DEFINE_CUSTOM_SETTER(jsReadableStreamPrototype_disturbedSetter, (JSGlobalObject * lexicalGlobalObject, JSC::EncodedJSValue thisValue, JSC::EncodedJSValue encodedValue, PropertyName)) +JSC_DEFINE_CUSTOM_SETTER(jsReadableStreamPrototype_disturbedSetter, (JSGlobalObject * lexicalGlobalObject, JSC::EncodedJSValue thisValue, JSC::EncodedJSValue encodedValue, PropertyName propertyName)) { - auto* stream = uncheckedDowncast(JSValue::decode(thisValue)); + auto& vm = JSC::getVM(lexicalGlobalObject); + auto scope = DECLARE_THROW_SCOPE(vm); + auto* stream = dynamicDowncast(JSValue::decode(thisValue)); + if (!stream) [[unlikely]] { + throwDOMAttributeSetterTypeError(lexicalGlobalObject, scope, JSReadableStream::info(), propertyName); + return false; + } stream->m_disturbed = JSValue::decode(encodedValue).toBoolean(lexicalGlobalObject); return true; } diff --git a/test/js/node/stream/node-stream.test.js b/test/js/node/stream/node-stream.test.js index 0bc56bd5fc87..a0f76e1ce77c 100644 --- a/test/js/node/stream/node-stream.test.js +++ b/test/js/node/stream/node-stream.test.js @@ -388,6 +388,41 @@ it("Readable.fromWeb", async () => { expect(Buffer.concat(chunks).toString()).toBe("Hello World!\n"); }); +// fromWeb assigns stream.$bunNativePtr on the node Readable. When user code grafts +// ReadableStream.prototype into the node stream prototype chain, that put used to +// reach ReadableStream's private custom setter with the Readable as the receiver, +// writing a JSValue through a type-confused pointer into the Readable's own +// property storage (observable below as a clobbered Symbol(kCapture) slot). +it("Readable.fromWeb with ReadableStream.prototype grafted into the prototype chain", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + const { Readable } = require("node:stream"); + const { EventEmitter } = require("node:events"); + const snap = o => Object.fromEntries(Reflect.ownKeys(o).map(k => [String(k), Object.prototype.toString.call(o[k])])); + const before = snap(Readable.fromWeb(new Response("x").body)); + Object.setPrototypeOf(EventEmitter.prototype, ReadableStream.prototype); + const stream = Readable.fromWeb(new Response("y").body); + const after = snap(stream); + for (const key in before) { + if (before[key] !== after[key]) throw new Error("clobbered own slot " + key + ": " + before[key] + " -> " + after[key]); + } + const chunks = []; + for await (const chunk of stream) chunks.push(chunk); + console.log("read:" + Buffer.concat(chunks).toString()); + `, + ], + env: bunEnv, + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + expect(stdout).toBe("read:y\n"); + expect(exitCode).toBe(0); +}); + // An error from the underlying web stream must surface on the node Readable as an // 'error' event (and destroy it), not as a global unhandled rejection. it("Readable.fromWeb propagates web stream errors to 'error' and destroys", async () => {