Skip to content

Commit 6cf582a

Browse files
patriksimekpatriksimek
authored
fix: prevent sandbox escape via Symbol.for cross-realm symbols (#553)
* fix: prevent sandbox escape via Symbol.for cross-realm symbols * fix: prevent Symbol.for bypass via hasOwnProperty override * fix: prevent Symbol.for bypass via object key coercion * fix: prevent cross-realm symbol extraction via Object.getOwnPropertySymbols * fix: prevent cross-realm symbol extraction via spread operator on bridge proxies
1 parent 4b009c2 commit 6cf582a

File tree

3 files changed

+311
-2
lines changed

3 files changed

+311
-2
lines changed

‎lib/bridge.js‎

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,11 @@ const thisSymbolToString = Symbol.prototype.toString;
104104
const thisSymbolToStringTag = Symbol.toStringTag;
105105
const thisSymbolIterator = Symbol.iterator;
106106
const thisSymbolNodeJSUtilInspectCustom = Symbol.for('nodejs.util.inspect.custom');
107+
const thisSymbolNodeJSRejection = Symbol.for('nodejs.rejection');
108+
109+
function isDangerousCrossRealmSymbol(key) {
110+
return key === thisSymbolNodeJSUtilInspectCustom || key === thisSymbolNodeJSRejection;
111+
}
107112

108113
/**
109114
* VMError.
@@ -380,6 +385,8 @@ function createBridge(otherInit, registerProxy) {
380385
}
381386
for (let i = 0; i < keys.length; i++) {
382387
const key = keys[i]; // @prim
388+
// Skip dangerous cross-realm symbols
389+
if (isDangerousCrossRealmSymbol(key)) continue;
383390
let desc;
384391
try {
385392
desc = otherSafeGetOwnPropertyDescriptor(object, key);
@@ -519,6 +526,8 @@ function createBridge(otherInit, registerProxy) {
519526

520527
getOwnPropertyDescriptor(target, prop) {
521528
// Note: target@this(unsafe) prop@prim throws@this(unsafe)
529+
// Filter dangerous cross-realm symbols to prevent extraction
530+
if (isDangerousCrossRealmSymbol(prop)) return undefined;
522531
const object = this.getObject(); // @other(unsafe)
523532
let desc; // @other(safe)
524533
try {
@@ -628,6 +637,8 @@ function createBridge(otherInit, registerProxy) {
628637

629638
has(target, key) {
630639
// Note: target@this(unsafe) key@prim throws@this(unsafe)
640+
// Filter dangerous cross-realm symbols
641+
if (isDangerousCrossRealmSymbol(key)) return false;
631642
const object = this.getObject(); // @other(unsafe)
632643
try {
633644
return otherReflectHas(object, key) === true;
@@ -659,7 +670,22 @@ function createBridge(otherInit, registerProxy) {
659670
} catch (e) { // @other(unsafe)
660671
throw thisFromOtherForThrow(e);
661672
}
662-
return thisFromOther(res);
673+
// Filter dangerous cross-realm symbols to prevent extraction via spread operator
674+
const keys = thisFromOther(res);
675+
const filtered = [];
676+
for (let i = 0; i < keys.length; i++) {
677+
const key = keys[i];
678+
if (!isDangerousCrossRealmSymbol(key)) {
679+
thisReflectDefineProperty(filtered, filtered.length, {
680+
__proto__: null,
681+
value: key,
682+
writable: true,
683+
enumerable: true,
684+
configurable: true
685+
});
686+
}
687+
}
688+
return filtered;
663689
}
664690

665691
preventExtensions(target) {

‎lib/setup-sandbox.js‎

Lines changed: 131 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,143 @@ const {
2525
has: localReflectHas,
2626
defineProperty: localReflectDefineProperty,
2727
setPrototypeOf: localReflectSetPrototypeOf,
28-
getOwnPropertyDescriptor: localReflectGetOwnPropertyDescriptor
28+
getOwnPropertyDescriptor: localReflectGetOwnPropertyDescriptor,
29+
ownKeys: localReflectOwnKeys
2930
} = localReflect;
3031

32+
const localObjectGetOwnPropertySymbols = localObject.getOwnPropertySymbols;
33+
const localObjectGetOwnPropertyDescriptors = localObject.getOwnPropertyDescriptors;
34+
const localObjectAssign = localObject.assign;
35+
3136
const speciesSymbol = Symbol.species;
3237
const globalPromise = global.Promise;
3338
class localPromise extends globalPromise {}
3439

40+
/*
41+
* Symbol.for protection
42+
*
43+
* Certain Node.js cross-realm symbols can be exploited for sandbox escapes:
44+
*
45+
* - 'nodejs.util.inspect.custom': Called by util.inspect with host's inspect function as argument.
46+
* If sandbox defines this on an object passed to host APIs (e.g., WebAssembly.compileStreaming),
47+
* Node's error handling calls the custom function with host context, enabling escape.
48+
*
49+
* - 'nodejs.rejection': Called by EventEmitter on promise rejection with captureRejections enabled.
50+
* The handler receives error objects that could potentially leak host context.
51+
*
52+
* Fix: Override Symbol.for to return sandbox-local symbols for dangerous keys instead of cross-realm
53+
* symbols. This prevents Node.js internals from recognizing sandbox-defined symbol properties while
54+
* preserving cross-realm behavior for other symbols.
55+
*/
56+
const originalSymbolFor = Symbol.for;
57+
const blockedSymbolCustomInspect = Symbol('nodejs.util.inspect.custom');
58+
const blockedSymbolRejection = Symbol('nodejs.rejection');
59+
60+
Symbol.for = function(key) {
61+
// Convert to string once to prevent toString/toPrimitive bypass and TOCTOU attacks
62+
const keyStr = '' + key;
63+
if (keyStr === 'nodejs.util.inspect.custom') {
64+
return blockedSymbolCustomInspect;
65+
}
66+
if (keyStr === 'nodejs.rejection') {
67+
return blockedSymbolRejection;
68+
}
69+
return originalSymbolFor(keyStr);
70+
};
71+
72+
/*
73+
* Cross-realm symbol extraction protection
74+
*
75+
* Even with Symbol.for overridden, cross-realm symbols can be extracted from
76+
* host objects exposed to the sandbox (e.g., Buffer.prototype) via:
77+
* Object.getOwnPropertySymbols(Buffer.prototype).find(s => s.description === 'nodejs.util.inspect.custom')
78+
*
79+
* Fix: Override Object.getOwnPropertySymbols and Reflect.ownKeys to replace
80+
* dangerous cross-realm symbols with sandbox-local equivalents in results.
81+
*/
82+
const realSymbolCustomInspect = originalSymbolFor('nodejs.util.inspect.custom');
83+
const realSymbolRejection = originalSymbolFor('nodejs.rejection');
84+
85+
function isDangerousSymbol(sym) {
86+
return sym === realSymbolCustomInspect || sym === realSymbolRejection;
87+
}
88+
89+
localObject.getOwnPropertySymbols = function getOwnPropertySymbols(obj) {
90+
const symbols = apply(localObjectGetOwnPropertySymbols, localObject, [obj]);
91+
const result = [];
92+
let j = 0;
93+
for (let i = 0; i < symbols.length; i++) {
94+
if (typeof symbols[i] !== 'symbol' || !isDangerousSymbol(symbols[i])) {
95+
localReflectDefineProperty(result, j++, {
96+
__proto__: null,
97+
value: symbols[i],
98+
writable: true,
99+
enumerable: true,
100+
configurable: true
101+
});
102+
}
103+
}
104+
return result;
105+
};
106+
107+
localReflect.ownKeys = function ownKeys(obj) {
108+
const keys = apply(localReflectOwnKeys, localReflect, [obj]);
109+
const result = [];
110+
let j = 0;
111+
for (let i = 0; i < keys.length; i++) {
112+
if (typeof keys[i] !== 'symbol' || !isDangerousSymbol(keys[i])) {
113+
localReflectDefineProperty(result, j++, {
114+
__proto__: null,
115+
value: keys[i],
116+
writable: true,
117+
enumerable: true,
118+
configurable: true
119+
});
120+
}
121+
}
122+
return result;
123+
};
124+
125+
/*
126+
* Object.getOwnPropertyDescriptors uses the internal [[OwnPropertyKeys]] which
127+
* bypasses our Reflect.ownKeys override. The result object has dangerous symbols
128+
* as property keys, which can then be leaked via Object.assign/Object.defineProperties
129+
* to a Proxy whose set/defineProperty trap captures the key.
130+
*/
131+
localObject.getOwnPropertyDescriptors = function getOwnPropertyDescriptors(obj) {
132+
const descs = apply(localObjectGetOwnPropertyDescriptors, localObject, [obj]);
133+
localReflectDeleteProperty(descs, realSymbolCustomInspect);
134+
localReflectDeleteProperty(descs, realSymbolRejection);
135+
return descs;
136+
};
137+
138+
/*
139+
* Object.assign uses internal [[OwnPropertyKeys]] on source objects, bypassing our
140+
* Reflect.ownKeys override. If a source (bridge proxy) has an enumerable dangerous-symbol
141+
* property, the symbol is passed to the target's [[Set]] which could be a user Proxy trap.
142+
*/
143+
localObject.assign = function assign(target) {
144+
if (target === null || target === undefined) {
145+
throw new LocalError('Cannot convert undefined or null to object');
146+
}
147+
const to = localObject(target);
148+
for (let s = 1; s < arguments.length; s++) {
149+
const source = arguments[s];
150+
if (source === null || source === undefined) continue;
151+
const from = localObject(source);
152+
const keys = apply(localReflectOwnKeys, localReflect, [from]);
153+
for (let i = 0; i < keys.length; i++) {
154+
const key = keys[i];
155+
if (typeof key === 'symbol' && isDangerousSymbol(key)) continue;
156+
const desc = apply(localReflectGetOwnPropertyDescriptor, localReflect, [from, key]);
157+
if (desc && desc.enumerable === true) {
158+
to[key] = from[key];
159+
}
160+
}
161+
}
162+
return to;
163+
};
164+
35165
const resetPromiseSpecies = (p) => {
36166
if (p instanceof globalPromise && ![globalPromise, localPromise].includes(p.constructor[speciesSymbol])) {
37167
Object.defineProperty(p.constructor, speciesSymbol, { value: localPromise });

‎test/vm.js‎

Lines changed: 153 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1294,6 +1294,159 @@ describe('VM', () => {
12941294
`), /process is not defined/);
12951295
});
12961296

1297+
it('Symbol.for dangerous Node.js symbols isolation', () => {
1298+
// Certain Node.js cross-realm symbols can be exploited for sandbox escapes:
1299+
// - 'nodejs.util.inspect.custom': Called by util.inspect with host's inspect function
1300+
// - 'nodejs.rejection': Called by EventEmitter on promise rejection
1301+
//
1302+
// Fix: These symbols return sandbox-local versions instead of cross-realm symbols,
1303+
// so Node.js internals won't recognize sandbox-defined symbol properties.
1304+
const vm2 = new VM();
1305+
1306+
// These dangerous symbols should be isolated (sandbox gets different symbol than host)
1307+
const dangerousSymbols = [
1308+
'nodejs.util.inspect.custom',
1309+
'nodejs.rejection'
1310+
];
1311+
1312+
for (const key of dangerousSymbols) {
1313+
const hostSymbol = Symbol.for(key);
1314+
const sandboxSymbol = vm2.run(`Symbol.for('${key}')`);
1315+
1316+
assert.notStrictEqual(
1317+
sandboxSymbol,
1318+
hostSymbol,
1319+
`Sandbox Symbol.for("${key}") should return a different symbol than host`
1320+
);
1321+
}
1322+
1323+
// Other symbols should still work cross-realm (backwards compatibility)
1324+
const safeSymbols = ['foo', 'bar', 'some.random.key'];
1325+
for (const key of safeSymbols) {
1326+
const hostSymbol = Symbol.for(key);
1327+
const sandboxSymbol = vm2.run(`Symbol.for('${key}')`);
1328+
1329+
assert.strictEqual(
1330+
sandboxSymbol,
1331+
hostSymbol,
1332+
`Symbol.for("${key}") should still work cross-realm`
1333+
);
1334+
}
1335+
});
1336+
1337+
it('Symbol extraction via Object.getOwnPropertySymbols on host objects', () => {
1338+
// The cross-realm symbol can be obtained from host objects like Buffer.prototype
1339+
// via Object.getOwnPropertySymbols, bypassing the Symbol.for override entirely.
1340+
const vm2 = new VM();
1341+
1342+
// Attempt to extract the real cross-realm symbol from Buffer.prototype
1343+
const extractedSymbol = vm2.run(`
1344+
const symbols = Object.getOwnPropertySymbols(Buffer.prototype);
1345+
symbols.find(s => s.description === 'nodejs.util.inspect.custom');
1346+
`);
1347+
1348+
assert.strictEqual(
1349+
extractedSymbol,
1350+
undefined,
1351+
'Dangerous cross-realm symbols should be filtered from Object.getOwnPropertySymbols results'
1352+
);
1353+
1354+
// Also verify Reflect.ownKeys doesn't leak the real symbol
1355+
const extractedViaReflect = vm2.run(`
1356+
const keys = Reflect.ownKeys(Buffer.prototype);
1357+
keys.find(k => typeof k === 'symbol' && k.description === 'nodejs.util.inspect.custom');
1358+
`);
1359+
1360+
assert.strictEqual(
1361+
extractedViaReflect,
1362+
undefined,
1363+
'Dangerous cross-realm symbols should be filtered from Reflect.ownKeys results'
1364+
);
1365+
1366+
// Verify Array.prototype monkey-patching can't bypass the filter
1367+
const vm3 = new VM();
1368+
const extractedWithSplicePatch = vm3.run(`
1369+
Array.prototype.splice = function() { /* no-op */ };
1370+
const symbols2 = Object.getOwnPropertySymbols(Buffer.prototype);
1371+
symbols2.find(s => typeof s === 'symbol' && s.description === 'nodejs.util.inspect.custom');
1372+
`);
1373+
1374+
assert.strictEqual(
1375+
extractedWithSplicePatch,
1376+
undefined,
1377+
'Array.prototype.splice override should not bypass symbol filtering'
1378+
);
1379+
1380+
// Verify Object.getOwnPropertyDescriptors filters dangerous symbols from result
1381+
const vm4 = new VM();
1382+
const descs = vm4.run(`Object.getOwnPropertyDescriptors(Buffer.prototype)`);
1383+
const hostInspectSymbol = Symbol.for('nodejs.util.inspect.custom');
1384+
1385+
assert.strictEqual(
1386+
hostInspectSymbol in descs,
1387+
false,
1388+
'Object.getOwnPropertyDescriptors should not include dangerous symbol keys in result'
1389+
);
1390+
1391+
// Verify Object.assign doesn't copy dangerous symbol-keyed properties
1392+
const vm5 = new VM();
1393+
const assigned = vm5.run(`
1394+
const target = {};
1395+
Object.assign(target, Buffer.prototype);
1396+
target;
1397+
`);
1398+
1399+
assert.strictEqual(
1400+
hostInspectSymbol in assigned,
1401+
false,
1402+
'Object.assign should not copy dangerous symbol-keyed properties'
1403+
);
1404+
});
1405+
1406+
it('Symbol extraction via spread operator on host objects', () => {
1407+
// The spread operator {...obj} calls [[OwnPropertyKeys]] internally,
1408+
// which invokes the proxy's ownKeys trap directly, bypassing any
1409+
// Reflect.ownKeys override in the sandbox.
1410+
const vm2 = new VM();
1411+
const hostInspectSymbol = Symbol.for('nodejs.util.inspect.custom');
1412+
1413+
// Verify spread operator doesn't copy dangerous symbol-keyed properties
1414+
const spread = vm2.run(`
1415+
const spread = {...Buffer.prototype};
1416+
spread;
1417+
`);
1418+
1419+
assert.strictEqual(
1420+
hostInspectSymbol in spread,
1421+
false,
1422+
'Spread operator should not copy dangerous symbol-keyed properties from host objects'
1423+
);
1424+
1425+
// Verify the full attack doesn't work
1426+
const vm3 = new VM();
1427+
const attackResult = vm3.run(`
1428+
const {...inspectDesc} = Buffer.prototype;
1429+
for (const k in inspectDesc) delete inspectDesc[k];
1430+
1431+
// If the dangerous symbol leaked, inspectDesc would have a symbol key
1432+
// with a function value that Object.defineProperties would interpret
1433+
let hasSymbolKey = false;
1434+
const symbols = Object.getOwnPropertySymbols(inspectDesc);
1435+
for (let i = 0; i < symbols.length; i++) {
1436+
if (symbols[i].description === 'nodejs.util.inspect.custom') {
1437+
hasSymbolKey = true;
1438+
}
1439+
}
1440+
hasSymbolKey;
1441+
`);
1442+
1443+
assert.strictEqual(
1444+
attackResult,
1445+
false,
1446+
'Dangerous symbol should not be extractable via spread operator'
1447+
);
1448+
});
1449+
12971450
it('Function.prototype.call attack via Promise', async () => {
12981451
const vm2 = new VM();
12991452
// This attack attempts to override Function.prototype.call to capture

0 commit comments

Comments
 (0)