Skip to content

Commit 10aecc9

Browse files
committed
fix(sandbox): expand CodeSandbox hardening to block realm intrinsics
CodeRabbit review on the SandboxedToolForge -> CodeSandbox consolidation flagged the extraGlobals denylist as incomplete and the contextObj as missing five realm intrinsics that node:vm exposes by default but untrusted sandbox code has no legitimate need for. Expanded both surfaces in lockstep: - DANGEROUS_GLOBAL_KEYS now includes Reflect, Proxy, WebAssembly, SharedArrayBuffer, Atomics so extraGlobals callers cannot inject them - contextObj explicitly nulls the same five identifiers so the realm intrinsics resolve to undefined inside the sandbox Closes the Reflect.construct(Function, [...])() reflection escape that would otherwise sidestep codeGeneration: { strings: false }, blocks Proxy-based prototype-chain attacks, and removes the SharedArrayBuffer / Atomics Spectre side-channel surface. WebAssembly was already blocked via codeGeneration: { wasm: false } but nulled here for belt-and-suspenders. ICodeSandbox.ts JSDoc reorganized to match the categorized denylist. New test 'drops the expanded danger list' locks the behavior. All 74 CodeSandbox + 29 SandboxedToolForge tests green.
1 parent 9fae662 commit 10aecc9

3 files changed

Lines changed: 62 additions & 3 deletions

File tree

src/sandbox/executor/CodeSandbox.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,13 @@ const DEFAULT_CONFIG: SandboxConfig = {
5353
* we let extraGlobals re-bind them the entire isolation guarantee evaporates.
5454
* Filtered silently at merge time so a forge-style consumer that includes one
5555
* of these by accident still gets a working sandbox without a noisy error.
56+
*
57+
* Categorized:
58+
* - Host-state escape: process, global, globalThis, require
59+
* - Code-generation reflection: eval, Function
60+
* - Realm-reflection / introspection: Reflect, Proxy
61+
* - Memory side-channels (Spectre family): SharedArrayBuffer, Atomics
62+
* - Native compilation surface: WebAssembly
5663
*/
5764
const DANGEROUS_GLOBAL_KEYS: ReadonlySet<string> = new Set([
5865
'process',
@@ -61,6 +68,11 @@ const DANGEROUS_GLOBAL_KEYS: ReadonlySet<string> = new Set([
6168
'require',
6269
'eval',
6370
'Function',
71+
'Reflect',
72+
'Proxy',
73+
'WebAssembly',
74+
'SharedArrayBuffer',
75+
'Atomics',
6476
]);
6577

6678
/** Dangerous patterns by language */
@@ -307,6 +319,18 @@ export class CodeSandbox implements ICodeSandbox {
307319
clearInterval: undefined,
308320
clearImmediate: undefined,
309321
queueMicrotask: undefined,
322+
// Realm intrinsics that node:vm exposes by default but untrusted
323+
// sandbox code has no legitimate need for. Explicitly nulling them
324+
// blocks runtime reflection paths (Reflect.construct(Function,...)),
325+
// Proxy-based prototype-chain attacks, and the SharedArrayBuffer/
326+
// Atomics Spectre side-channel surface. WebAssembly is already
327+
// blocked via codeGeneration: { wasm: false } but nulled here for
328+
// belt-and-suspenders.
329+
Reflect: undefined,
330+
Proxy: undefined,
331+
WebAssembly: undefined,
332+
SharedArrayBuffer: undefined,
333+
Atomics: undefined,
310334
};
311335

312336
// Merge caller-supplied extras AFTER the hardened defaults so an explicit

src/sandbox/executor/ICodeSandbox.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -56,9 +56,13 @@ export interface SandboxConfig {
5656
* SandboxedToolForge) needs to expose allowlisted APIs (`fetch`, `fs`,
5757
* `crypto`) without forking a second sandbox implementation.
5858
*
59-
* Security-critical keys (`process`, `global`, `globalThis`, `require`,
60-
* `eval`, `Function`) are silently dropped from this map at merge time
61-
* so callers cannot accidentally undo the sandbox's hardenings.
59+
* Security-critical keys are silently dropped from this map at merge time
60+
* so callers cannot accidentally undo the sandbox's hardenings:
61+
* - Host-state escape: `process`, `global`, `globalThis`, `require`
62+
* - Code-generation reflection: `eval`, `Function`
63+
* - Realm-reflection / introspection: `Reflect`, `Proxy`
64+
* - Memory side-channels (Spectre family): `SharedArrayBuffer`, `Atomics`
65+
* - Native compilation surface: `WebAssembly`
6266
*/
6367
extraGlobals?: Record<string, unknown>;
6468
}

src/sandbox/executor/tests/CodeSandbox.spec.ts

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -317,6 +317,37 @@ describe('CodeSandbox — JavaScript execution', () => {
317317
* rather than passing because `process` is already undefined by
318318
* default in node:vm contexts.
319319
*/
320+
it('drops the expanded danger list (Reflect/Proxy/WebAssembly/SharedArrayBuffer/Atomics)', async () => {
321+
const result = await sandbox.execute({
322+
language: 'javascript',
323+
code: `
324+
const safe = typeof safeApi === 'function' ? 'ok' : 'missing';
325+
const reflect = typeof Reflect === 'undefined' ? 'blocked' : 'LEAKED';
326+
const proxy = typeof Proxy === 'undefined' ? 'blocked' : 'LEAKED';
327+
const wasm = typeof WebAssembly === 'undefined' ? 'blocked' : 'LEAKED';
328+
const sab = typeof SharedArrayBuffer === 'undefined' ? 'blocked' : 'LEAKED';
329+
const atomics = typeof Atomics === 'undefined' ? 'blocked' : 'LEAKED';
330+
return safe + ':' + reflect + ':' + proxy + ':' + wasm + ':' + sab + ':' + atomics;
331+
`,
332+
config: {
333+
extraGlobals: {
334+
// Caller tries to inject dangerous globals; all dropped.
335+
Reflect: { get: () => 'pwned' },
336+
Proxy: function () { return 'pwned'; },
337+
WebAssembly: { compile: () => 'pwned' },
338+
SharedArrayBuffer: function () { return 'pwned'; },
339+
Atomics: { load: () => 'pwned' },
340+
// Plus a safe one that should land.
341+
safeApi: () => 'ok',
342+
},
343+
},
344+
});
345+
346+
expect(result.status).toBe('success');
347+
expect(result.output?.stdout).toContain('ok:blocked:blocked:blocked:blocked:blocked');
348+
expect(result.output?.stdout).not.toContain('LEAKED');
349+
});
350+
320351
it('drops dangerous keys from extraGlobals while keeping safe ones', async () => {
321352
const fakeProcess = { env: { LEAKED: 'should-not-reach-sandbox' } };
322353
const result = await sandbox.execute({

0 commit comments

Comments
 (0)