Skip to content
Open
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 82 additions & 6 deletions src/jsc/bindings/NodeVM.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@
#include "JavaScriptCore/JSGlobalProxyInlines.h"
#include "GCDefferalContext.h"
#include "JSBuffer.h"
#include "xxhash3.h"

#include <wtf/MallocSpan.h>
#include <JavaScriptCore/JSCBytecodeCacheVersion.h>

#include <JavaScriptCore/DOMJITAbstractHeap.h>
#include <JavaScriptCore/DFGAbstractHeap.h>
Expand Down Expand Up @@ -125,6 +129,77 @@ bool extractCachedData(JSValue cachedDataValue, WTF::Vector<uint8_t>& outCachedD
return false;
}

// Integrity header verified by unwrapCachedData before decodeCodeBlock sees the payload; same role as V8's SerializedCodeData header.
struct CachedDataHeader {
uint32_t magic;
uint32_t payloadLength;
uint32_t sourceHash;
uint32_t jscVersion;
uint64_t payloadHash;
};
static_assert(sizeof(CachedDataHeader) == 24, "header layout is part of the cachedData format");

static constexpr uint32_t cachedDataMagic = 0x436E7542; // "BunC"

static uint64_t hashCachedDataPayload(std::span<const uint8_t> payload)
{
return highway_xxhash3_64(payload.data(), payload.size(), 0);
}

JSC::JSUint8Array* createCachedDataBuffer(JSGlobalObject* globalObject, const JSC::SourceCode& source, std::span<const uint8_t> bytecode)
{
JSC::JSUint8Array* buffer = WebCore::createUninitializedBuffer(globalObject, sizeof(CachedDataHeader) + bytecode.size());
if (!buffer) [[unlikely]] {
return nullptr;
}

const CachedDataHeader header {
.magic = cachedDataMagic,
.payloadLength = static_cast<uint32_t>(bytecode.size()),
.sourceHash = source.hash(),
.jscVersion = JSC::computeJSCBytecodeCacheVersion(),
.payloadHash = hashCachedDataPayload(bytecode),
};
uint8_t* data = buffer->typedVector();
memcpy(data, &header, sizeof(header));
if (!bytecode.empty()) {
memcpy(data + sizeof(header), bytecode.data(), bytecode.size());
}
return buffer;
}

RefPtr<JSC::CachedBytecode> unwrapCachedData(const JSC::SourceCode& source, std::span<const uint8_t> cachedData)
{
if (cachedData.size() < sizeof(CachedDataHeader)) {
return nullptr;
}

CachedDataHeader header;
memcpy(&header, cachedData.data(), sizeof(header));
if (header.magic != cachedDataMagic) {
return nullptr;
}

std::span<const uint8_t> payload = cachedData.subspan(sizeof(CachedDataHeader));
if (payload.size() != header.payloadLength) {
return nullptr;
}
if (header.sourceHash != source.hash()) {
return nullptr;
}
if (header.jscVersion != JSC::computeJSCBytecodeCacheVersion()) {
return nullptr;
}
if (header.payloadHash != hashCachedDataPayload(payload)) {
return nullptr;
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}

// Copy: decoded functions retain the Decoder for lazy code block decoding, so a borrowed span would dangle.
auto payloadCopy = WTF::MallocSpan<uint8_t, JSC::VMMalloc>::malloc(payload.size());
memcpySpan(payloadCopy.mutableSpan(), payload);
return JSC::CachedBytecode::create(WTF::move(payloadCopy), {});
}

JSC::JSFunction* constructAnonymousFunction(JSC::JSGlobalObject* globalObject, const ArgList& args, const SourceOrigin& sourceOrigin, CompileFunctionOptions&& options, JSC::SourceTaintedOrigin sourceTaintOrigin, JSC::JSScope* scope)
{
ASSERT(scope);
Expand Down Expand Up @@ -210,9 +285,11 @@ JSC::JSFunction* constructAnonymousFunction(JSC::JSGlobalObject* globalObject, c
TriState bytecodeAccepted = TriState::Indeterminate;

if (!options.cachedData.isEmpty()) {
cachedBytecode = CachedBytecode::create(std::span(options.cachedData), nullptr, {});
SourceCodeKey key(sourceCode, {}, JSC::SourceCodeType::ProgramType, lexicallyScopedFeatures, JSC::JSParserScriptMode::Classic, JSC::DerivedContextType::None, JSC::EvalContextType::None, false, {}, std::nullopt);
unlinkedProgramCodeBlock = JSC::decodeCodeBlock<UnlinkedProgramCodeBlock>(vm, key, *cachedBytecode);
cachedBytecode = unwrapCachedData(sourceCode, std::span(options.cachedData));
if (cachedBytecode) {
SourceCodeKey key(sourceCode, {}, JSC::SourceCodeType::ProgramType, lexicallyScopedFeatures, JSC::JSParserScriptMode::Classic, JSC::DerivedContextType::None, JSC::EvalContextType::None, false, {}, std::nullopt);
unlinkedProgramCodeBlock = JSC::decodeCodeBlock<UnlinkedProgramCodeBlock>(vm, key, *cachedBytecode);
}
if (unlinkedProgramCodeBlock == nullptr) {
bytecodeAccepted = TriState::False;
} else {
Expand Down Expand Up @@ -253,7 +330,7 @@ JSC::JSFunction* constructAnonymousFunction(JSC::JSGlobalObject* globalObject, c
if (options.produceCachedData) {
RefPtr<JSC::CachedBytecode> producedBytecode = getBytecode(globalObject, programExecutable, sourceCode);
if (producedBytecode) {
JSC::JSUint8Array* buffer = WebCore::createBuffer(globalObject, producedBytecode->span());
JSC::JSUint8Array* buffer = createCachedDataBuffer(globalObject, sourceCode, producedBytecode->span());
RETURN_IF_EXCEPTION(throwScope, nullptr);
function->putDirect(vm, JSC::Identifier::fromString(vm, "cachedData"_s), buffer);
function->putDirect(vm, JSC::Identifier::fromString(vm, "cachedDataProduced"_s), jsBoolean(true));
Expand Down Expand Up @@ -476,8 +553,7 @@ JSC::EncodedJSValue createCachedData(JSGlobalObject* globalObject, const JSC::So
return throwVMError(globalObject, scope, "createCachedData failed"_s);
}

std::span<const uint8_t> bytes = bytecode->span();
JSC::JSUint8Array* buffer = WebCore::createBuffer(globalObject, bytes);
JSC::JSUint8Array* buffer = createCachedDataBuffer(globalObject, source, bytecode->span());
RETURN_IF_EXCEPTION(scope, {});

if (!buffer) {
Expand Down
2 changes: 2 additions & 0 deletions src/jsc/bindings/NodeVM.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ namespace NodeVM {
RefPtr<JSC::CachedBytecode> getBytecode(JSGlobalObject* globalObject, JSC::ProgramExecutable* executable, const JSC::SourceCode& source);
RefPtr<JSC::CachedBytecode> getBytecode(JSGlobalObject* globalObject, JSC::ModuleProgramExecutable* executable, const JSC::SourceCode& source);
bool extractCachedData(JSValue cachedDataValue, WTF::Vector<uint8_t>& outCachedData);
JSC::JSUint8Array* createCachedDataBuffer(JSGlobalObject* globalObject, const JSC::SourceCode& source, std::span<const uint8_t> bytecode);
RefPtr<JSC::CachedBytecode> unwrapCachedData(const JSC::SourceCode& source, std::span<const uint8_t> cachedData);
String stringifyAnonymousFunction(JSGlobalObject* globalObject, const ArgList& args, ThrowScope& scope, int* outOffset);
JSC::EncodedJSValue createCachedData(JSGlobalObject* globalObject, const JSC::SourceCode& source);
bool handleException(JSGlobalObject* globalObject, VM& vm, NakedPtr<JSC::Exception> exception, ThrowScope& throwScope);
Expand Down
19 changes: 10 additions & 9 deletions src/jsc/bindings/NodeVMScript.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -173,10 +173,12 @@
}
ASSERT(executable);

JSC::LexicallyScopedFeatures lexicallyScopedFeatures = globalObject->globalScopeExtension() ? JSC::TaintedByWithScopeLexicallyScopedFeature : JSC::NoLexicallyScopedFeatures;
JSC::SourceCodeKey key(script->source(), {}, JSC::SourceCodeType::ProgramType, lexicallyScopedFeatures, JSC::JSParserScriptMode::Classic, JSC::DerivedContextType::None, JSC::EvalContextType::None, false, {}, std::nullopt);
Ref<JSC::CachedBytecode> cachedBytecode = JSC::CachedBytecode::create(std::span(cachedData), nullptr, {});
JSC::UnlinkedProgramCodeBlock* unlinkedBlock = JSC::decodeCodeBlock<UnlinkedProgramCodeBlock>(vm, key, WTF::move(cachedBytecode));
JSC::UnlinkedProgramCodeBlock* unlinkedBlock = nullptr;
if (RefPtr<JSC::CachedBytecode> cachedBytecode = unwrapCachedData(script->source(), std::span(cachedData))) {
JSC::LexicallyScopedFeatures lexicallyScopedFeatures = globalObject->globalScopeExtension() ? JSC::TaintedByWithScopeLexicallyScopedFeature : JSC::NoLexicallyScopedFeatures;
JSC::SourceCodeKey key(script->source(), {}, JSC::SourceCodeType::ProgramType, lexicallyScopedFeatures, JSC::JSParserScriptMode::Classic, JSC::DerivedContextType::None, JSC::EvalContextType::None, false, {}, std::nullopt);
unlinkedBlock = JSC::decodeCodeBlock<UnlinkedProgramCodeBlock>(vm, key, cachedBytecode.releaseNonNull());
}

Check warning on line 181 in src/jsc/bindings/NodeVMScript.cpp

View check run for this annotation

Claude / Claude Code Review

NodeVMScript retains a redundant copy of cachedData after unwrapCachedData() copies it

Now that `unwrapCachedData()` copies the payload into its own `MallocSpan`, `m_options.cachedData` on `NodeVMScript` is dead storage after `constructScript` returns — its only reader is line 167 — but it's still retained for the script's lifetime, so an accepted-`cachedData` `vm.Script` now holds ~2× the payload bytes (Vector + MallocSpan copy) where before it held 1×. Consider `cachedData = {};` at the end of the `if (!cachedData.isEmpty())` block, or drop the field from `ScriptOptions` entirel
Comment thread
claude[bot] marked this conversation as resolved.
Comment thread
robobun marked this conversation as resolved.

if (!unlinkedBlock) {
script->cachedDataRejected(TriState::True);
Expand All @@ -199,8 +201,6 @@
}
} else if (produceCachedData) {
script->cacheBytecode();
// TODO(@heimskr): is there ever a case where bytecode production fails?
script->cachedDataProduced(true);
}

return JSValue::encode(script);
Expand Down Expand Up @@ -244,10 +244,11 @@
cacheBytecode();
}

ASSERT(m_cachedBytecode);
if (!m_cachedBytecode) [[unlikely]] {
return nullptr;
}

std::span<const uint8_t> bytes = m_cachedBytecode->span();
m_cachedBytecodeBuffer.set(vm(), this, WebCore::createBuffer(globalObject(), bytes));
m_cachedBytecodeBuffer.set(vm(), this, createCachedDataBuffer(globalObject(), m_source, m_cachedBytecode->span()));
if (!m_cachedBytecodeBuffer) {
return nullptr;
}
Expand Down
1 change: 0 additions & 1 deletion src/jsc/bindings/NodeVMScript.h
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,6 @@ class NodeVMScript final : public JSC::JSDestructibleObject, public SigintReceiv
RefPtr<JSC::CachedBytecode> cachedBytecode() const { return m_cachedBytecode; }
JSC::ProgramExecutable* cachedExecutable() const { return m_cachedExecutable.get(); }
bool cachedDataProduced() const { return m_cachedDataProduced; }
void cachedDataProduced(bool value) { m_cachedDataProduced = value; }
TriState cachedDataRejected() const { return m_cachedDataRejected; }
void cachedDataRejected(TriState value) { m_cachedDataRejected = value; }
bool sourceMapURLParsed() const { return m_sourceMapURLParsed; }
Expand Down
21 changes: 13 additions & 8 deletions src/jsc/bindings/NodeVMSourceTextModule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -124,12 +124,13 @@ NodeVMSourceTextModule* NodeVMSourceTextModule::create(VM& vm, JSGlobalObject* g
}

ptr->m_cachedExecutable.set(vm, ptr, executable);
LexicallyScopedFeatures lexicallyScopedFeatures = globalObject->globalScopeExtension() ? TaintedByWithScopeLexicallyScopedFeature : NoLexicallyScopedFeatures;
SourceCodeKey key(ptr->sourceCode(), {}, SourceCodeType::ProgramType, lexicallyScopedFeatures, JSParserScriptMode::Classic, DerivedContextType::None, EvalContextType::None, false, {}, std::nullopt);
Ref<CachedBytecode> cachedBytecode = CachedBytecode::create(std::span(cachedData), nullptr, {});
RETURN_IF_EXCEPTION(scope, nullptr);
UnlinkedModuleProgramCodeBlock* unlinkedBlock = decodeCodeBlock<UnlinkedModuleProgramCodeBlock>(vm, key, WTF::move(cachedBytecode));
RETURN_IF_EXCEPTION(scope, nullptr);
UnlinkedModuleProgramCodeBlock* unlinkedBlock = nullptr;
if (RefPtr<CachedBytecode> cachedBytecode = unwrapCachedData(ptr->sourceCode(), std::span(cachedData))) {
LexicallyScopedFeatures lexicallyScopedFeatures = globalObject->globalScopeExtension() ? TaintedByWithScopeLexicallyScopedFeature : NoLexicallyScopedFeatures;
SourceCodeKey key(ptr->sourceCode(), {}, SourceCodeType::ProgramType, lexicallyScopedFeatures, JSParserScriptMode::Classic, DerivedContextType::None, EvalContextType::None, false, {}, std::nullopt);
unlinkedBlock = decodeCodeBlock<UnlinkedModuleProgramCodeBlock>(vm, key, cachedBytecode.releaseNonNull());
RETURN_IF_EXCEPTION(scope, nullptr);
}

if (unlinkedBlock) {
JSScope* jsScope = globalObject->globalScope();
Expand Down Expand Up @@ -516,8 +517,12 @@ JSUint8Array* NodeVMSourceTextModule::cachedData(JSGlobalObject* globalObject)
if (!m_cachedBytecodeBuffer) {
RefPtr<CachedBytecode> cachedBytecode = bytecode(globalObject);
RETURN_IF_EXCEPTION(scope, nullptr);
std::span<const uint8_t> bytes = cachedBytecode->span();
JSUint8Array* buffer = WebCore::createBuffer(globalObject, bytes);
// getBytecode can return null without throwing (serialization failure).
if (!cachedBytecode) [[unlikely]] {
throwVMError(globalObject, scope, "createCachedData failed"_s);
return nullptr;
}
JSUint8Array* buffer = createCachedDataBuffer(globalObject, m_sourceCode, cachedBytecode->span());
RETURN_IF_EXCEPTION(scope, nullptr);
m_cachedBytecodeBuffer.set(vm, this, buffer);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
Expand Down
94 changes: 94 additions & 0 deletions test/js/node/vm/vm.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -892,6 +892,100 @@ test("can't use bytecode from a different script", () => {
expect(secondScript.runInThisContext()).toBe(4);
});

test("rejects corrupted cachedData instead of crashing", async () => {
// Corrupting, truncating, or extending createCachedData() output must set
// cachedDataRejected (or throw ERR_VM_MODULE_CACHED_DATA_REJECTED for modules)
// and recompile from source, like Node. Runs in a child process since the
// broken behavior is a process crash: a truncated tail leaves embedded
// self-relative offsets pointing past the copied buffer, which JSC's decoder
// dereferences without a bounds check.
const code = /* js */ `
const vm = require("node:vm");
const assert = require("node:assert");

function corruptions(good) {
const variants = [];
// Truncations: len-1, len/2, 16, 1. The len/2 case is the one that
// reliably reached the decoder's wild pointer arithmetic before the
// envelope check was added.
for (const cut of [good.length - 1, good.length >> 1, 16, 1]) {
variants.push(["truncate:" + cut, good.subarray(0, cut)]);
}
// 20 single-byte flips seeded deterministically across the buffer.
for (let i = 0; i < 20; i++) {
const copy = Buffer.from(good);
const off = Math.floor((i * (good.length - 1)) / 19);
copy[off] ^= 0xff;
variants.push(["flip@" + off, copy]);
}
variants.push(["extend", Buffer.concat([good, Buffer.from([0xde, 0xad])])]);
variants.push(["fill", Buffer.alloc(good.length, 0x2a)]);
variants.push(["random", Buffer.from("fhqwhgads")]);
return variants;
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}

{
const source = "function f(){return 1234} f()";
const good = new vm.Script(source).createCachedData();
for (const [label, cachedData] of corruptions(good)) {
const script = new vm.Script(source, { cachedData });
assert.strictEqual(script.cachedDataRejected, true, label);
assert.strictEqual(script.runInThisContext(), 1234, label);
}
const intact = new vm.Script(source, { cachedData: good });
assert.strictEqual(intact.cachedDataRejected, false);
assert.strictEqual(intact.runInThisContext(), 1234);
// Intact bytecode for a different source is also rejected (sourceHash).
const other = new vm.Script("1 + 1", { cachedData: good });
assert.strictEqual(other.cachedDataRejected, true);
assert.strictEqual(other.runInThisContext(), 2);
console.log("script ok");
}

{
const params = ["a"];
const source = "return a + 1234;";
const good = vm.compileFunction(source, params, { produceCachedData: true }).cachedData;
assert.ok(good.length > 0);
for (const [label, cachedData] of corruptions(good)) {
const fn = vm.compileFunction(source, params, { cachedData });
assert.strictEqual(fn.cachedDataRejected, true, label);
assert.strictEqual(fn(1), 1235, label);
}
const intact = vm.compileFunction(source, params, { cachedData: good });
assert.strictEqual(intact.cachedDataRejected, false);
assert.strictEqual(intact(1), 1235);
console.log("compileFunction ok");
}

{
const source = "export const value = 1234;";
const good = new vm.SourceTextModule(source).createCachedData();
for (const [label, cachedData] of corruptions(good)) {
assert.throws(() => new vm.SourceTextModule(source, { cachedData }), {
code: "ERR_VM_MODULE_CACHED_DATA_REJECTED",
}, label);
}
new vm.SourceTextModule(source, { cachedData: good });
console.log("module ok");
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
`;

await using proc = Bun.spawn({
cmd: [bunExe(), "-e", code],
env: bunEnv,
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect({ stdout, stderr, exitCode, signalCode: proc.signalCode }).toEqual({
stdout: "script ok\ncompileFunction ok\nmodule ok\n",
stderr: "",
exitCode: 0,
signalCode: null,
});
Comment thread
robobun marked this conversation as resolved.
});

describe("codeGeneration options", () => {
test("disabling codeGeneration.strings should block eval and Function constructor", () => {
const context = createContext(
Expand Down
Loading