fix: prevent cross-realm symbol extraction via Object.getOwnPropertySymbols

Author: patriksimek

lib/setup-sandbox.js

@@ -25,9 +25,14 @@ const {
 	has: localReflectHas,
 	defineProperty: localReflectDefineProperty,
 	setPrototypeOf: localReflectSetPrototypeOf,
-	getOwnPropertyDescriptor: localReflectGetOwnPropertyDescriptor
+	getOwnPropertyDescriptor: localReflectGetOwnPropertyDescriptor,
+	ownKeys: localReflectOwnKeys
 } = localReflect;
 
+const localObjectGetOwnPropertySymbols = localObject.getOwnPropertySymbols;
+const localObjectGetOwnPropertyDescriptors = localObject.getOwnPropertyDescriptors;
+const localObjectAssign = localObject.assign;
+
 const speciesSymbol = Symbol.species;
 const globalPromise = global.Promise;
 class localPromise extends globalPromise {}
@@ -64,6 +69,99 @@ Symbol.for = function(key) {
 	return originalSymbolFor(keyStr);
 };
 
+/*
+ * Cross-realm symbol extraction protection
+ *
+ * Even with Symbol.for overridden, cross-realm symbols can be extracted from
+ * host objects exposed to the sandbox (e.g., Buffer.prototype) via:
+ *   Object.getOwnPropertySymbols(Buffer.prototype).find(s => s.description === 'nodejs.util.inspect.custom')
+ *
+ * Fix: Override Object.getOwnPropertySymbols and Reflect.ownKeys to replace
+ * dangerous cross-realm symbols with sandbox-local equivalents in results.
+ */
+const realSymbolCustomInspect = originalSymbolFor('nodejs.util.inspect.custom');
+const realSymbolRejection = originalSymbolFor('nodejs.rejection');
+
+function isDangerousSymbol(sym) {
+	return sym === realSymbolCustomInspect || sym === realSymbolRejection;
+}
+
+localObject.getOwnPropertySymbols = function getOwnPropertySymbols(obj) {
+	const symbols = apply(localObjectGetOwnPropertySymbols, localObject, [obj]);
+	const result = [];
+	let j = 0;
+	for (let i = 0; i < symbols.length; i++) {
+		if (typeof symbols[i] !== 'symbol' || !isDangerousSymbol(symbols[i])) {
+			localReflectDefineProperty(result, j++, {
+				__proto__: null,
+				value: symbols[i],
+				writable: true,
+				enumerable: true,
+				configurable: true
+			});
+		}
+	}
+	return result;
+};
+
+localReflect.ownKeys = function ownKeys(obj) {
+	const keys = apply(localReflectOwnKeys, localReflect, [obj]);
+	const result = [];
+	let j = 0;
+	for (let i = 0; i < keys.length; i++) {
+		if (typeof keys[i] !== 'symbol' || !isDangerousSymbol(keys[i])) {
+			localReflectDefineProperty(result, j++, {
+				__proto__: null,
+				value: keys[i],
+				writable: true,
+				enumerable: true,
+				configurable: true
+			});
+		}
+	}
+	return result;
+};
+
+/*
+ * Object.getOwnPropertyDescriptors uses the internal [[OwnPropertyKeys]] which
+ * bypasses our Reflect.ownKeys override. The result object has dangerous symbols
+ * as property keys, which can then be leaked via Object.assign/Object.defineProperties
+ * to a Proxy whose set/defineProperty trap captures the key.
+ */
+localObject.getOwnPropertyDescriptors = function getOwnPropertyDescriptors(obj) {
+	const descs = apply(localObjectGetOwnPropertyDescriptors, localObject, [obj]);
+	localReflectDeleteProperty(descs, realSymbolCustomInspect);
+	localReflectDeleteProperty(descs, realSymbolRejection);
+	return descs;
+};
+
+/*
+ * Object.assign uses internal [[OwnPropertyKeys]] on source objects, bypassing our
+ * Reflect.ownKeys override. If a source (bridge proxy) has an enumerable dangerous-symbol
+ * property, the symbol is passed to the target's [[Set]] which could be a user Proxy trap.
+ */
+localObject.assign = function assign(target) {
+	if (target === null || target === undefined) {
+		throw new LocalError('Cannot convert undefined or null to object');
+	}
+	const to = localObject(target);
+	for (let s = 1; s < arguments.length; s++) {
+		const source = arguments[s];
+		if (source === null || source === undefined) continue;
+		const from = localObject(source);
+		const keys = apply(localReflectOwnKeys, localReflect, [from]);
+		for (let i = 0; i < keys.length; i++) {
+			const key = keys[i];
+			if (typeof key === 'symbol' && isDangerousSymbol(key)) continue;
+			const desc = apply(localReflectGetOwnPropertyDescriptor, localReflect, [from, key]);
+			if (desc && desc.enumerable === true) {
+				to[key] = from[key];
+			}
+		}
+	}
+	return to;
+};
+
 const resetPromiseSpecies = (p) => {
 	if (p instanceof globalPromise && ![globalPromise, localPromise].includes(p.constructor[speciesSymbol])) {
 		Object.defineProperty(p.constructor, speciesSymbol, { value: localPromise });

test/vm.js

@@ -1334,6 +1334,75 @@ describe('VM', () => {
 		}
 	});
 
+	it('Symbol extraction via Object.getOwnPropertySymbols on host objects', () => {
+		// The cross-realm symbol can be obtained from host objects like Buffer.prototype
+		// via Object.getOwnPropertySymbols, bypassing the Symbol.for override entirely.
+		const vm2 = new VM();
+
+		// Attempt to extract the real cross-realm symbol from Buffer.prototype
+		const extractedSymbol = vm2.run(`
+			const symbols = Object.getOwnPropertySymbols(Buffer.prototype);
+			symbols.find(s => s.description === 'nodejs.util.inspect.custom');
+		`);
+
+		assert.strictEqual(
+			extractedSymbol,
+			undefined,
+			'Dangerous cross-realm symbols should be filtered from Object.getOwnPropertySymbols results'
+		);
+
+		// Also verify Reflect.ownKeys doesn't leak the real symbol
+		const extractedViaReflect = vm2.run(`
+			const keys = Reflect.ownKeys(Buffer.prototype);
+			keys.find(k => typeof k === 'symbol' && k.description === 'nodejs.util.inspect.custom');
+		`);
+
+		assert.strictEqual(
+			extractedViaReflect,
+			undefined,
+			'Dangerous cross-realm symbols should be filtered from Reflect.ownKeys results'
+		);
+
+		// Verify Array.prototype monkey-patching can't bypass the filter
+		const vm3 = new VM();
+		const extractedWithSplicePatch = vm3.run(`
+			Array.prototype.splice = function() { /* no-op */ };
+			const symbols2 = Object.getOwnPropertySymbols(Buffer.prototype);
+			symbols2.find(s => typeof s === 'symbol' && s.description === 'nodejs.util.inspect.custom');
+		`);
+
+		assert.strictEqual(
+			extractedWithSplicePatch,
+			undefined,
+			'Array.prototype.splice override should not bypass symbol filtering'
+		);
+
+		// Verify Object.getOwnPropertyDescriptors filters dangerous symbols from result
+		const vm4 = new VM();
+		const descs = vm4.run(`Object.getOwnPropertyDescriptors(Buffer.prototype)`);
+		const hostInspectSymbol = Symbol.for('nodejs.util.inspect.custom');
+
+		assert.strictEqual(
+			hostInspectSymbol in descs,
+			false,
+			'Object.getOwnPropertyDescriptors should not include dangerous symbol keys in result'
+		);
+
+		// Verify Object.assign doesn't copy dangerous symbol-keyed properties
+		const vm5 = new VM();
+		const assigned = vm5.run(`
+			const target = {};
+			Object.assign(target, Buffer.prototype);
+			target;
+		`);
+
+		assert.strictEqual(
+			hostInspectSymbol in assigned,
+			false,
+			'Object.assign should not copy dangerous symbol-keyed properties'
+		);
+	});
+
 	it('Function.prototype.call attack via Promise', async () => {
 		const vm2 = new VM();
 		// This attack attempts to override Function.prototype.call to capture