Skip to content

Commit 7161a80

Browse files
feat: enhance security by preventing raw references in property descriptors and reflection methods
1 parent e72d335 commit 7161a80

3 files changed

Lines changed: 123 additions & 3 deletions

File tree

libs/core/src/__tests__/secure-proxy.spec.ts

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -632,6 +632,62 @@ describe('createSafeReflect', () => {
632632
// When calling undefined as a function, it throws TypeError
633633
expect(safeReflect?.setPrototypeOf).toBeUndefined();
634634
});
635+
636+
// Regression: createSafeReflect used to hand back the RAW native Reflect methods, whose
637+
// prototype chain reaches the host Function constructor, and Reflect.get was an unrestricted
638+
// reflection primitive (string-literal key evades the AST guards).
639+
it('does not hand back a raw Reflect.get that can reach the Function constructor', () => {
640+
const safeReflect = createSafeReflect('STANDARD') as any;
641+
const get = safeReflect.get;
642+
expect(typeof get).toBe('function');
643+
// The returned method is behind the membrane: its constructor is blocked.
644+
expect(get.constructor).toBeUndefined();
645+
// Reflect.get(Reflect.get, 'constructor') must not yield a usable Function constructor.
646+
expect(get(get, 'constructor')).toBeUndefined();
647+
});
648+
649+
it('still performs reflection correctly after hardening', () => {
650+
const safeReflect = createSafeReflect('STANDARD') as any;
651+
const obj = { a: 1, b: 2 };
652+
expect(safeReflect.get(obj, 'a')).toBe(1);
653+
expect(safeReflect.has(obj, 'b')).toBe(true);
654+
expect([...safeReflect.ownKeys(obj)]).toEqual(expect.arrayContaining(['a', 'b']));
655+
});
656+
});
657+
658+
describe('getOwnPropertyDescriptor trap must not leak raw references (review finding)', () => {
659+
it('refuses an object-valued non-configurable, non-writable pinned property', () => {
660+
const host: Record<string, unknown> = {};
661+
Object.defineProperty(host, 'pinned', {
662+
value: { leadsToHost: {} },
663+
configurable: false,
664+
writable: false,
665+
enumerable: true,
666+
});
667+
const proxy = createSecureProxy(host);
668+
expect(() => Object.getOwnPropertyDescriptor(proxy, 'pinned')).toThrow(/blocked|cannot be exposed/i);
669+
});
670+
671+
it('still reports a primitive-valued pinned property via its descriptor', () => {
672+
const host: Record<string, unknown> = {};
673+
Object.defineProperty(host, 'VERSION', {
674+
value: 7,
675+
configurable: false,
676+
writable: false,
677+
enumerable: true,
678+
});
679+
const proxy = createSecureProxy(host);
680+
expect(Object.getOwnPropertyDescriptor(proxy, 'VERSION')?.value).toBe(7);
681+
});
682+
683+
it('wraps a configurable object value so its descriptor cannot leak a raw reference', () => {
684+
const obj = { child: { nested: 1 } };
685+
const proxy = createSecureProxy(obj) as any;
686+
const descriptor = Object.getOwnPropertyDescriptor(proxy, 'child');
687+
// The descriptor value is itself a secure proxy: constructor is blocked.
688+
expect(descriptor?.value.constructor).toBeUndefined();
689+
expect(descriptor?.value.nested).toBe(1);
690+
});
635691
});
636692

637693
describe('BLOCKED_PROPERTY_CATEGORIES', () => {

libs/core/src/double-vm/parent-vm-bootstrap.ts

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -984,7 +984,23 @@ ${stackTraceHardeningCode}
984984
var propName = String(property);
985985
var descriptor = Reflect.getOwnPropertyDescriptor(target, property);
986986
987-
// Must return actual descriptor for non-configurable properties (proxy invariant)
987+
// Mirror the get trap's host-owned pinned-value refusal: a non-configurable, non-writable
988+
// data property must be reported with its exact value (proxy invariant), so on a HOST-owned
989+
// value an object/function would cross the barrier unwrapped via descriptor.value and its
990+
// prototype chain reaches the host Function constructor. Deny the read instead (throwing
991+
// satisfies the invariant); primitives stay safe to report.
992+
if (host && descriptor && !descriptor.configurable && descriptor.writable === false && 'value' in descriptor) {
993+
var exactValue = descriptor.value;
994+
if (exactValue !== null && (typeof exactValue === 'object' || typeof exactValue === 'function')) {
995+
throw createSafeError(
996+
"Security violation: Access to property descriptor for '" + propName + "' is blocked. " +
997+
"This property cannot be exposed without breaking the sandbox barrier."
998+
);
999+
}
1000+
return descriptor;
1001+
}
1002+
1003+
// Must return actual descriptor for other non-configurable properties (proxy invariant)
9881004
if (descriptor && !descriptor.configurable) {
9891005
return descriptor;
9901006
}
@@ -999,6 +1015,18 @@ ${stackTraceHardeningCode}
9991015
}
10001016
return undefined;
10011017
}
1018+
1019+
// Wrap object/function values so a descriptor read cannot hand back a raw reference that
1020+
// the get trap would otherwise have proxied.
1021+
if (descriptor && 'value' in descriptor) {
1022+
var value = descriptor.value;
1023+
if (value !== null && (typeof value === 'object' || typeof value === 'function')) {
1024+
var wrapped = {};
1025+
for (var dk in descriptor) { if (Object.prototype.hasOwnProperty.call(descriptor, dk)) wrapped[dk] = descriptor[dk]; }
1026+
wrapped.value = createSecureProxy(value, depth + 1, host);
1027+
return wrapped;
1028+
}
1029+
}
10021030
return descriptor;
10031031
},
10041032
ownKeys: function(target) {

libs/core/src/secure-proxy.ts

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,7 @@ export function createSafeReflect(securityLevel: SecurityLevel): typeof Reflect
219219

220220
// Wrap Reflect.construct to block Function constructors
221221
if (typeof value === 'function' && prop === 'construct') {
222-
return function (ctorTarget: unknown, args: unknown[], newTarget?: unknown) {
222+
const safeConstruct = function (ctorTarget: unknown, args: unknown[], newTarget?: unknown) {
223223
// Block Function, AsyncFunction, GeneratorFunction, AsyncGeneratorFunction constructors
224224
// Intentional empty functions to obtain constructor references
225225
// eslint-disable-next-line @typescript-eslint/no-empty-function
@@ -243,6 +243,17 @@ export function createSafeReflect(securityLevel: SecurityLevel): typeof Reflect
243243
newTarget as new (...args: unknown[]) => unknown,
244244
);
245245
};
246+
// Wrap so the returned function cannot itself be walked to the host Function constructor.
247+
return createSecureProxy(safeConstruct);
248+
}
249+
250+
// SECURITY: never hand back the RAW native Reflect methods. Each is a host-realm function
251+
// whose prototype chain reaches the host Function constructor, and Reflect.get /
252+
// Reflect.getOwnPropertyDescriptor are reflection primitives whose STRING-LITERAL key
253+
// argument bypasses the AST computed-key guard. Wrapping in a secure proxy blocks
254+
// `.constructor` on the method and keeps every result behind the membrane.
255+
if (typeof value === 'function') {
256+
return createSecureProxy(value);
246257
}
247258

248259
return value;
@@ -663,7 +674,23 @@ export function createSecureProxy<T extends object>(target: T, options: SecurePr
663674
const propName = typeof property === 'symbol' ? property.toString() : property;
664675
const descriptor = Reflect.getOwnPropertyDescriptor(target, property);
665676

666-
// Must return actual descriptor for non-configurable properties (proxy invariant)
677+
// Mirror the get trap: a non-configurable, non-writable data property must be reported
678+
// with its exact value (proxy invariant), so an object/function value cannot be wrapped
679+
// or hidden and would cross the barrier unwrapped via `descriptor.value`. Deny the read
680+
// instead (throwing satisfies the invariant); primitives are inert and safe to report.
681+
if (descriptor && !descriptor.configurable && descriptor.writable === false && 'value' in descriptor) {
682+
const exactValue: unknown = descriptor.value;
683+
if (exactValue !== null && (typeof exactValue === 'object' || typeof exactValue === 'function')) {
684+
throw createSafeError(
685+
`Security violation: Access to property descriptor for '${String(propName)}' is blocked. ` +
686+
`This property cannot be exposed without breaking the sandbox barrier.`,
687+
'SecurityError',
688+
);
689+
}
690+
return descriptor;
691+
}
692+
693+
// Must return actual descriptor for other non-configurable properties (proxy invariant)
667694
if (descriptor && !descriptor.configurable) {
668695
return descriptor;
669696
}
@@ -683,6 +710,15 @@ export function createSecureProxy<T extends object>(target: T, options: SecurePr
683710
return undefined;
684711
}
685712

713+
// Wrap object/function values so a descriptor read cannot hand back a raw reference that
714+
// the get trap would have proxied (a configurable property's value may be safely wrapped).
715+
if (descriptor && 'value' in descriptor) {
716+
const value: unknown = descriptor.value;
717+
if (value !== null && (typeof value === 'object' || typeof value === 'function')) {
718+
return { ...descriptor, value: proxyWithDepth(value as object, depth + 1) };
719+
}
720+
}
721+
686722
return descriptor;
687723
},
688724

0 commit comments

Comments
 (0)