Fix missing sandbox escape defenses (#562)

lib/bridge.js

@@ -150,9 +150,16 @@ const thisErrorCaptureStackTrace = Error.captureStackTrace;
 const thisSymbolToString = Symbol.prototype.toString;
 const thisSymbolToStringTag = Symbol.toStringTag;
 const thisSymbolIterator = Symbol.iterator;
+const thisSymbolSpecies = Symbol.species;
 const thisSymbolNodeJSUtilInspectCustom = Symbol.for('nodejs.util.inspect.custom');
 const thisSymbolNodeJSRejection = Symbol.for('nodejs.rejection');
 
+// Sentinel value used to detect species self-return attacks on arrays.
+// When neutralizeArraySpecies sets constructor = undefined, if a subsequent
+// check finds constructor has been restored (e.g. via Object.assign bypass),
+// this sentinel marks the array as tampered.
+const SPECIES_ATTACK_SENTINEL = Symbol('SPECIES_ATTACK_SENTINEL');
+
 function isDangerousCrossRealmSymbol(key) {
 	return key === thisSymbolNodeJSUtilInspectCustom || key === thisSymbolNodeJSRejection;
 }
@@ -364,6 +371,59 @@ function createBridge(otherInit, registerProxy) {
 		if (!thisReflectPreventExtensions(target)) throw thisUnexpected();
 	}
 
+	/**
+	 * Neutralize Array species on a host-realm value.
+	 *
+	 * V8's ArraySpeciesCreate algorithm reads `obj.constructor[Symbol.species]` on
+	 * the raw host object, bypassing proxy traps. If an attacker sets constructor
+	 * to a function that returns the same array (species self-return), map/filter/etc.
+	 * store raw host values directly into that array, bypassing bridge sanitization.
+	 *
+	 * This function sets `constructor = undefined` as an own data property on any
+	 * host array. With `constructor` undefined, ArraySpeciesCreate falls back to
+	 * the default Array constructor, which is safe.
+	 *
+	 * Called before and after every host function call in the apply trap.
+	 */
+	function neutralizeArraySpecies(value) {
+		// Only process non-null objects (arrays are objects)
+		if (value === null || typeof value !== 'object') return;
+		try {
+			// Array.isArray works cross-realm — identifies host arrays correctly
+			if (!thisArrayIsArray(value)) return;
+
+			// Set constructor = undefined as own data property.
+			// This shadows any inherited or attacker-set constructor.
+			const success = otherReflectDefineProperty(value, 'constructor', {
+				__proto__: null,
+				value: undefined,
+				writable: true,
+				configurable: true
+			});
+			if (!success) {
+				// If defineProperty failed, the array may be non-extensible or
+				// constructor is non-configurable (attacker froze it).
+				// Either way, throw to prevent the call from proceeding.
+				throw new VMError('Cannot neutralize array species: constructor is non-configurable or array is non-extensible');
+			}
+		} catch (e) {
+			if (e instanceof VMError) throw e;
+			// Swallow other errors (e.g., from Proxy traps on exotic objects)
+		}
+	}
+
+	/**
+	 * Neutralize Array species on all arguments and context before/after a host call.
+	 */
+	function neutralizeArraySpeciesArgs(context, args) {
+		neutralizeArraySpecies(context);
+		if (args) {
+			for (let i = 0; i < args.length; i++) {
+				neutralizeArraySpecies(args[i]);
+			}
+		}
+	}
+
 	function thisAddProtoMapping(proto, other, name) {
 		// Note: proto@this(unsafe) other@other(unsafe) name@this(unsafe) throws@this(unsafe)
 		thisReflectApply(thisMapSet, protoMappings, [proto, thisIdMapping]);
@@ -644,6 +704,16 @@ function createBridge(otherInit, registerProxy) {
 			if (key === '__proto__' && !thisOtherHasOwnProperty(object, key)) {
 				return this.setPrototypeOf(target, value);
 			}
+			// Intercept constructor writes to host arrays.
+			// V8's ArraySpeciesCreate reads constructor[Symbol.species] on the raw
+			// host object, bypassing proxy traps. If an attacker sets constructor
+			// to a species-returning function, map/filter/etc. store raw host values
+			// directly, bypassing bridge sanitization.
+			// Store the value on the proxy target (this-realm) instead of the host array.
+			if (key === 'constructor' && thisArrayIsArray(object)) {
+				thisReflectSet(target, key, value);
+				return true;
+			}
 			try {
 				value = otherFromThis(value);
 				return otherReflectSet(object, key, value) === true;
@@ -669,7 +739,14 @@ function createBridge(otherInit, registerProxy) {
 			try {
 				context = otherFromThis(context);
 				args = otherFromThisArguments(args);
+				// Neutralize Array species before the host call.
+				// V8's ArraySpeciesCreate reads constructor[Symbol.species] on raw host
+				// arrays, bypassing proxy traps. Setting constructor = undefined forces
+				// the default Array constructor, which is safe.
+				neutralizeArraySpeciesArgs(context, args);
 				ret = otherReflectApply(object, context, args);
+				// Re-neutralize after the call in case the host function restored constructor.
+				neutralizeArraySpeciesArgs(context, args);
 			} catch (e) { // @other(unsafe)
 				throw thisFromOtherForThrow(e);
 			}
@@ -755,6 +832,13 @@ function createBridge(otherInit, registerProxy) {
 			const object = getHandlerObject(this); // @other(unsafe)
 			if (!thisReflectSetPrototypeOf(desc, null)) throw thisUnexpected();
 
+			// Intercept defineProperty for constructor on host arrays.
+			// Same rationale as the set trap: prevent ArraySpeciesCreate manipulation.
+			if (prop === 'constructor' && thisArrayIsArray(object)) {
+				thisReflectDefineProperty(target, prop, desc);
+				return true;
+			}
+
 			desc = this.definePropertyDesc(target, prop, desc);
 
 			if (!desc) return false;

lib/setup-sandbox.js

@@ -393,11 +393,79 @@ connect(localObject.prototype.__lookupSetter__, host.Object.prototype.__lookupSe
 
 const oldPrepareStackTraceDesc = localReflectGetOwnPropertyDescriptor(LocalError, 'prepareStackTrace');
 
+/*
+ * Safe default prepareStackTrace function.
+ *
+ * When Error.prepareStackTrace is undefined in the sandbox, V8 falls back to
+ * Node.js's host-side prepareStackTraceCallback (from node:internal/errors).
+ * If that host code throws (e.g., when error.name is a Symbol), the TypeError
+ * is a host-realm error, which can be used for sandbox escape.
+ *
+ * This function ensures V8 never falls back to the host formatter. It safely
+ * handles Symbol names, Proxy objects, and other exotic types without throwing.
+ */
+function defaultSandboxPrepareStackTrace(error, callSites) {
+	// Safely convert error to a header string, handling Symbol names,
+	// Proxy objects, and other exotic types that would throw during coercion.
+	let header;
+	try {
+		let name;
+		try {
+			name = error.name;
+		} catch (e) {
+			name = 'Error';
+		}
+		// If name is a Symbol or other non-string, safely coerce it
+		if (typeof name === 'symbol') {
+			try {
+				name = name.toString();
+			} catch (e) {
+				name = 'Error';
+			}
+		} else if (typeof name !== 'string') {
+			try {
+				name = '' + name;
+			} catch (e) {
+				name = 'Error';
+			}
+		}
+		let message;
+		try {
+			message = error.message;
+		} catch (e) {
+			message = '';
+		}
+		if (typeof message !== 'string') {
+			try {
+				message = '' + message;
+			} catch (e) {
+				message = '';
+			}
+		}
+		header = message ? name + ': ' + message : name;
+	} catch (e) {
+		header = 'Error';
+	}
+
+	// Format each call site safely
+	const lines = [header];
+	for (let i = 0; i < callSites.length; i++) {
+		try {
+			lines[lines.length] = '    at ' + callSites[i];
+		} catch (e) {
+			lines[lines.length] = '    at <error formatting frame>';
+		}
+	}
+	return lines.join('\n');
+}
+
 let currentPrepareStackTrace = LocalError.prepareStackTrace;
 const wrappedPrepareStackTrace = new LocalWeakMap();
 if (typeof currentPrepareStackTrace === 'function') {
 	wrappedPrepareStackTrace.set(currentPrepareStackTrace, currentPrepareStackTrace);
 }
+// Register the safe default in the WeakMap so it wraps itself (identity).
+localReflectApply(localWeakMapSet, wrappedPrepareStackTrace, [defaultSandboxPrepareStackTrace, defaultSandboxPrepareStackTrace]);
 
 let OriginalCallSite;
 LocalError.prepareStackTrace = (e, sst) => {
@@ -490,7 +558,12 @@ if (typeof OriginalCallSite === 'function') {
 		},
 		set(value) {
 			if (typeof(value) !== 'function') {
-				currentPrepareStackTrace = value;
+				// When user sets prepareStackTrace to undefined/null/non-function,
+				// reset to the safe default instead of allowing undefined.
+				// If undefined were stored, V8 would fall back to Node.js's host-side
+				// prepareStackTraceCallback, which runs in the host context and can
+				// throw host-realm TypeErrors (e.g., when error.name is a Symbol).
+				currentPrepareStackTrace = defaultSandboxPrepareStackTrace;
 				return;
 			}
 			const wrapped = localReflectApply(localWeakMapGet, wrappedPrepareStackTrace, [value]);

test/vm.js

@@ -2648,6 +2648,125 @@ describe('VM', () => {
 	});
 });
 
+describe('missing sandbox defenses (issue #562)', () => {
+	it('Array species self-return attack via constructor manipulation', () => {
+		const vm2 = new VM();
+		// Setting constructor on a host array to a species-returning function
+		// should be intercepted by the proxy set trap and stored locally,
+		// not on the underlying host array.
+		const result = vm2.run(`
+			const arr = Buffer.from([1,2,3]);
+			arr.constructor = function x() { return arr; };
+			// Even if constructor was set, map should produce a normal array
+			const mapped = arr.map(x => x * 2);
+			// Verify map didn't store into the original array (species self-return)
+			mapped !== arr;
+		`);
+		assert.strictEqual(result, true);
+	});
+
+	it('Array constructor write via defineProperty is intercepted', () => {
+		const vm2 = new VM();
+		// Object.defineProperty for constructor on host arrays should be
+		// intercepted and stored locally on the proxy target.
+		const result = vm2.run(`
+			const arr = Buffer.from([1,2,3]);
+			Object.defineProperty(arr, 'constructor', {
+				value: function x() { return arr; },
+				writable: true,
+				configurable: true
+			});
+			const mapped = arr.map(x => x * 2);
+			mapped !== arr;
+		`);
+		assert.strictEqual(result, true);
+	});
+
+	it('Error.prepareStackTrace safe default prevents host fallback', () => {
+		const vm2 = new VM();
+		// When user sets Error.prepareStackTrace = undefined, V8 should NOT
+		// fall back to the host's prepareStackTraceCallback. Instead, a safe
+		// default should be used. Verify setting to undefined doesn't crash
+		// and still produces valid stack traces.
+		const result = vm2.run(`
+			Error.prepareStackTrace = undefined;
+			const e = new Error('test');
+			typeof e.stack === 'string' && e.stack.includes('test');
+		`);
+		assert.strictEqual(result, true);
+	});
+
+	it('Error.prepareStackTrace safe default handles Symbol names without throwing', () => {
+		const vm2 = new VM();
+		// After clearing prepareStackTrace, accessing .stack with a Symbol name
+		// should NOT throw a host TypeError (which would be a host-realm error).
+		// The safe default should handle it gracefully.
+		const result = vm2.run(`
+			Error.prepareStackTrace = undefined;
+			const e = new Error('msg');
+			e.name = Symbol('test');
+			let threw = false;
+			try {
+				const s = e.stack;
+				// Stack should be a string containing the Symbol name
+				threw = typeof s !== 'string';
+			} catch(ex) {
+				threw = true;
+			}
+			threw;
+		`);
+		assert.strictEqual(result, false);
+	});
+
+	it('Error.prepareStackTrace = undefined does not enable host escape', () => {
+		const vm2 = new VM({allowAsync: true});
+		// The Category 19 attack: clear prepareStackTrace, use Symbol name trick
+		// to generate host TypeError, catch via host promise. With the safe default,
+		// no host error should be generated.
+		assert.strictEqual(vm2.run(`
+			Error.prepareStackTrace = undefined;
+			const e = new Error();
+			e.name = Symbol();
+			let hostError = false;
+			try {
+				const s = e.stack;
+			} catch(ex) {
+				// If we get here, check if it's a host error
+				try {
+					const F = ex.constructor.constructor;
+					const p = F('return process')();
+					if (p && p.version) hostError = true;
+				} catch(inner) {}
+			}
+			hostError;
+		`), false);
+	});
+
+	it('neutralizeArraySpecies prevents species attack in apply trap', () => {
+		const vm2 = new VM();
+		// Host function calls through the apply trap should neutralize
+		// array species by setting constructor = undefined on host arrays
+		// passed as arguments. Use Object.entries to get a host array.
+		const result = vm2.run(`
+			const g = ({}).__lookupGetter__;
+			const a = Buffer.apply;
+			const p = a.apply(g, [Buffer, ['__proto__']]);
+			const op = p.call(p.call(p.call(p.call(Buffer.of()))));
+			const ho = op.constructor;
+			// ho.entries({}) creates a host array
+			const arr = ho.entries({a: 1, b: 2});
+			// Setting constructor for species should be blocked
+			function x() { return arr; }
+			x[Symbol.species] = x;
+			arr.constructor = x;
+			const mapped = arr.map(v => v);
+			// If species was neutralized, mapped should be a different array
+			mapped !== arr;
+		`);
+		assert.strictEqual(result, true);
+	});
+});
+
 describe('precompiled scripts', () => {
 	it('VM', () => {
 		const vm = new VM();