Skip to content

Commit 4aee666

Browse files
feat: enhance security by blocking dangerous static destructuring keys to prevent host RCE vulnerabilities (#78)
* feat: enhance security by blocking dangerous static destructuring keys to prevent host RCE vulnerabilities (GHSA-3279) * feat: enhance security by blocking computed destructuring and dangerous static keys to prevent property name attacks * feat: enhance security by pinning Reflect methods to prevent shared-realm tampering * feat: enhance security by preventing mutations to the Reflect global in the sandbox * feat: enhance security by preventing mutations to the Reflect global in the sandbox
1 parent 203cb69 commit 4aee666

9 files changed

Lines changed: 920 additions & 81 deletions

libs/ast/src/__tests__/no-computed-destructuring.spec.ts

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,47 @@ describe('NoComputedDestructuringRule', () => {
107107
});
108108
});
109109

110+
describe('Static Dangerous Keys (GHSA-3279)', () => {
111+
const dangerous = [
112+
'constructor',
113+
'prototype',
114+
'__proto__',
115+
'__defineGetter__',
116+
'__defineSetter__',
117+
'__lookupGetter__',
118+
'__lookupSetter__',
119+
];
120+
121+
for (const key of dangerous) {
122+
it(`blocks identifier key { ${key}: x }`, async () => {
123+
const validator = new JSAstValidator([new NoComputedDestructuringRule()]);
124+
const result = await validator.validate(`const { ${key}: x } = obj;`);
125+
expect(result.valid).toBe(false);
126+
expect(result.issues[0].code).toBe('NO_DANGEROUS_DESTRUCTURING');
127+
});
128+
129+
it(`blocks string-literal key { "${key}": x }`, async () => {
130+
const validator = new JSAstValidator([new NoComputedDestructuringRule()]);
131+
const result = await validator.validate(`const { "${key}": x } = obj;`);
132+
expect(result.valid).toBe(false);
133+
expect(result.issues[0].code).toBe('NO_DANGEROUS_DESTRUCTURING');
134+
});
135+
}
136+
137+
it('blocks a dangerous key nested inside a valid pattern', async () => {
138+
const validator = new JSAstValidator([new NoComputedDestructuringRule()]);
139+
const result = await validator.validate(`const { user: { "constructor": c } } = obj;`);
140+
expect(result.valid).toBe(false);
141+
expect(result.issues[0].code).toBe('NO_DANGEROUS_DESTRUCTURING');
142+
});
143+
144+
it('does not flag a benign key that merely contains a dangerous substring', async () => {
145+
const validator = new JSAstValidator([new NoComputedDestructuringRule()]);
146+
const result = await validator.validate(`const { constructorName, prototypeId } = obj;`);
147+
expect(result.valid).toBe(true);
148+
});
149+
});
150+
110151
describe('Valid Patterns (Should Allow)', () => {
111152
it('should allow static property names', async () => {
112153
const validator = new JSAstValidator([new NoComputedDestructuringRule()]);

libs/ast/src/rules/no-computed-destructuring.rule.ts

Lines changed: 75 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,23 @@ import type { ValidationRule, ValidationContext } from '../interfaces';
22
import { ValidationSeverity } from '../interfaces';
33
import * as walk from 'acorn-walk';
44

5+
/**
6+
* Property keys that must never be read through destructuring: their prototype chain reaches host
7+
* intrinsics (constructor -> Function). Blocking them statically closes GHSA-3279, where
8+
* `const { "constructor": C } = {}` and `const { prototype: p } = callTool` slipped past the
9+
* computed-key guard because the key is a static Identifier/Literal. Mirrors the runtime membrane
10+
* and interpreter blocked-key sets.
11+
*/
12+
const DANGEROUS_DESTRUCTURING_KEYS = new Set([
13+
'constructor',
14+
'prototype',
15+
'__proto__',
16+
'__defineGetter__',
17+
'__defineSetter__',
18+
'__lookupGetter__',
19+
'__lookupSetter__',
20+
]);
21+
522
/**
623
* Configuration options for NoComputedDestructuringRule
724
*/
@@ -52,7 +69,8 @@ export interface NoComputedDestructuringOptions {
5269
export class NoComputedDestructuringRule implements ValidationRule {
5370
readonly name = 'no-computed-destructuring';
5471
readonly description =
55-
'Blocks computed property names in destructuring patterns to prevent runtime property name attacks';
72+
'Blocks computed property names and dangerous static keys (constructor, prototype, __proto__, ...) ' +
73+
'in destructuring patterns to prevent property name attacks';
5674
readonly defaultSeverity = ValidationSeverity.ERROR;
5775
readonly enabledByDefault = true;
5876

@@ -71,8 +89,10 @@ export class NoComputedDestructuringRule implements ValidationRule {
7189
if (!node.properties) return;
7290

7391
for (const prop of node.properties) {
74-
// Check for computed property: { [expr]: binding }
75-
if (prop.type === 'Property' && prop.computed === true) {
92+
if (prop.type !== 'Property') continue;
93+
94+
// Computed key: { [expr]: binding } — the name is unknowable at analysis time.
95+
if (prop.computed === true) {
7696
const keyDescription = this.describeKey(prop.key);
7797

7898
report({
@@ -82,24 +102,32 @@ export class NoComputedDestructuringRule implements ValidationRule {
82102
`Computed property names in destructuring are not allowed: { [${keyDescription}]: ... }. ` +
83103
`This pattern can be used to bypass security checks by constructing dangerous property names at runtime. ` +
84104
`Use static property names instead.`,
85-
location: prop.key?.loc
86-
? {
87-
line: prop.key.loc.start.line,
88-
column: prop.key.loc.start.column,
89-
endLine: prop.key.loc.end.line,
90-
endColumn: prop.key.loc.end.column,
91-
}
92-
: node.loc
93-
? {
94-
line: node.loc.start.line,
95-
column: node.loc.start.column,
96-
}
97-
: undefined,
105+
location: this.keyLocation(prop, node),
98106
data: {
99107
keyType: prop.key?.type,
100108
keyDescription,
101109
},
102110
});
111+
continue;
112+
}
113+
114+
// Static key: { "constructor": binding } / { constructor: binding }. These are not
115+
// computed, so the guard above never fired — block dangerous names explicitly.
116+
const staticName = this.staticKeyName(prop.key);
117+
if (staticName !== undefined && DANGEROUS_DESTRUCTURING_KEYS.has(staticName)) {
118+
report({
119+
code: 'NO_DANGEROUS_DESTRUCTURING',
120+
message:
121+
this.customMessage ||
122+
`Destructuring the '${staticName}' property is not allowed: { ${this.describeKey(prop.key)}: ... }. ` +
123+
`This property's prototype chain can reach the Function constructor for sandbox escape. ` +
124+
`Remove the '${staticName}' binding.`,
125+
location: this.keyLocation(prop, node),
126+
data: {
127+
keyType: prop.key?.type,
128+
key: staticName,
129+
},
130+
});
103131
}
104132
}
105133
},
@@ -110,6 +138,37 @@ export class NoComputedDestructuringRule implements ValidationRule {
110138
});
111139
}
112140

141+
/**
142+
* Resolve the name of a NON-computed destructuring key (Identifier or Literal), or undefined
143+
* when it has no statically-known string name.
144+
*/
145+
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- acorn doesn't export specific node types
146+
private staticKeyName(key: any): string | undefined {
147+
if (!key) return undefined;
148+
if (key.type === 'Identifier') return key.name;
149+
if (key.type === 'Literal') return typeof key.value === 'string' ? key.value : String(key.value);
150+
return undefined;
151+
}
152+
153+
/**
154+
* Build the issue location from the offending key, falling back to the pattern node.
155+
*/
156+
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- acorn doesn't export specific node types
157+
private keyLocation(prop: any, node: any) {
158+
if (prop.key?.loc) {
159+
return {
160+
line: prop.key.loc.start.line,
161+
column: prop.key.loc.start.column,
162+
endLine: prop.key.loc.end.line,
163+
endColumn: prop.key.loc.end.column,
164+
};
165+
}
166+
if (node.loc) {
167+
return { line: node.loc.start.line, column: node.loc.start.column };
168+
}
169+
return undefined;
170+
}
171+
113172
/**
114173
* Generate a human-readable description of the computed key
115174
*/

0 commit comments

Comments
 (0)