fix: sanitize Promise callback arguments to prevent sandbox escape

Add sanitization for both onFulfilled and onRejected callbacks in globalPromise.prototype.then and globalPromise.prototype.catch.

Previously, host errors (e.g., from stack trace computation when Error.name is a Symbol) could reach Promise callbacks unsanitized, allowing access to host constructors and sandbox escape.

Author: patriksimek

lib/setup-sandbox.js

@@ -39,11 +39,39 @@ const resetPromiseSpecies = (p) => {
 };
 
 const globalPromiseThen = globalPromise.prototype.then;
+const globalPromiseCatch = globalPromise.prototype.catch;
+
 globalPromise.prototype.then = function then(onFulfilled, onRejected) {
 	resetPromiseSpecies(this);
+	if (typeof onFulfilled === 'function') {
+		const origOnFulfilled = onFulfilled;
+		onFulfilled = function onFulfilled(value) {
+			value = ensureThis(value);
+			return apply(origOnFulfilled, this, [value]);
+		};
+	}
+	if (typeof onRejected === 'function') {
+		const origOnRejected = onRejected;
+		onRejected = function onRejected(error) {
+			error = ensureThis(error);
+			return apply(origOnRejected, this, [error]);
+		};
+	}
 	return globalPromiseThen.call(this, onFulfilled, onRejected);
 };
 
+globalPromise.prototype.catch = function _catch(onRejected) {
+	resetPromiseSpecies(this);
+	if (typeof onRejected === 'function') {
+		const origOnRejected = onRejected;
+		onRejected = function onRejected(error) {
+			error = ensureThis(error);
+			return apply(origOnRejected, this, [error]);
+		};
+	}
+	return globalPromiseCatch.call(this, onRejected);
+};
+
 const localReflectApply = (target, thisArg, args) => {
 	resetPromiseSpecies(thisArg);
 	return apply(target, thisArg, args);
@@ -537,10 +565,35 @@ if (localPromise) {
 		overrideWithProxy(PromisePrototype, 'then', PromisePrototype.then, {
 			__proto__: null,
 			apply(target, thiz, args) {
+				if (args.length > 0) {
+					const onFulfilled = args[0];
+					if (typeof onFulfilled === 'function') {
+						args[0] = function sanitizedOnFulfilled(value) {
+							value = ensureThis(value);
+							return localReflectApply(onFulfilled, this, [value]);
+						};
+					}
+				}
 				if (args.length > 1) {
 					const onRejected = args[1];
 					if (typeof onRejected === 'function') {
-						args[1] = function wrapper(error) {
+						args[1] = function sanitizedOnRejected(error) {
+							error = ensureThis(error);
+							return localReflectApply(onRejected, this, [error]);
+						};
+					}
+				}
+				return localReflectApply(target, thiz, args);
+			}
+		});
+
+		overrideWithProxy(PromisePrototype, 'catch', PromisePrototype.catch, {
+			__proto__: null,
+			apply(target, thiz, args) {
+				if (args.length > 0) {
+					const onRejected = args[0];
+					if (typeof onRejected === 'function') {
+						args[0] = function sanitizedOnRejected(error) {
 							error = ensureThis(error);
 							return localReflectApply(onRejected, this, [error]);
 						};

package-lock.json

@@ -13,7 +13,7 @@
 		"alcatraz",
 		"contextify"
 	],
-	"version": "3.10.0",
+	"version": "3.10.1",
 	"main": "index.js",
 	"sideEffects": false,
 	"repository": "github:patriksimek/vm2",

package.json

@@ -1267,6 +1267,33 @@ describe('VM', () => {
 		`), /constructor is not a function/);
 	});
 
+	it('Promise.prototype.then/catch callback sanitization bypass', async () => {
+		const vm2 = new VM();
+		// This attack uses an Error with a Symbol name to trigger a host error
+		// during stack trace computation, then tries to access host constructors
+		// through the unsanitized error in the Promise catch callback.
+		// If the error is not sanitized, e.constructor.constructor would be the
+		// host's Function which can access 'process'. If sanitized, it's the
+		// sandbox's Function where 'process' is not defined.
+		await assert.rejects(() => vm2.run(`
+			new Promise((resolve, reject) => {
+				const error = new Error();
+				error.name = Symbol();
+				const f = async () => error.stack;
+				f().catch(e => {
+					try {
+						const Error = e.constructor;
+						const Function = Error.constructor;
+						const p = Function('return process')();
+						resolve(p);
+					} catch (err) {
+						reject(err);
+					}
+				});
+			});
+		`), /process is not defined/);
+	});
+
 	after(() => {
 		vm = null;
 	});