Skip to content

Commit 9ced73a

Browse files
committed
fix(ses): Limit scope proxy exposure to discernably owned properties of host globalThis
Closes #1305
1 parent 52a6bf4 commit 9ced73a

13 files changed

+76
-134
lines changed

packages/ses/src/strict-scope-terminator.js

+1-2
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import {
66
create,
77
freeze,
88
getOwnPropertyDescriptors,
9-
globalThis,
109
} from './commons.js';
1110
import { assert } from './error/assert.js';
1211

@@ -55,7 +54,7 @@ const scopeProxyHandlerProperties = {
5554

5655
has(_shadow, prop) {
5756
// we must at least return true for all properties on the realm globalThis
58-
return prop in globalThis;
57+
return true;
5958
},
6059

6160
// note: this is likely a bug of safari

packages/ses/test/compartment.test.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ test('SES compartment does not see primal realm names', t => {
1313
// eslint-disable-next-line no-unused-vars
1414
const hidden = 1;
1515
const c = new Compartment();
16-
t.throws(() => c.evaluate('hidden+1'), { instanceOf: ReferenceError });
16+
t.is(c.evaluate('hidden'), undefined);
1717
});
1818

1919
test('SES compartment also has compartments', t => {

packages/ses/test/evalTaming-default.test.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ test('safe eval when evalTaming is undefined.', t => {
77
// eslint-disable-next-line no-unused-vars
88
const a = 0;
99
// eslint-disable-next-line no-eval
10-
t.throws(() => eval('a'));
10+
t.is(eval('a'), undefined);
1111
// eslint-disable-next-line no-eval
1212
t.is(eval('1 + 1'), 2);
1313

packages/ses/test/evalTaming-safe-eval.test.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ test('safe eval when evalTaming is safe-eval.', t => {
77
// eslint-disable-next-line no-unused-vars
88
const a = 0;
99
// eslint-disable-next-line no-eval
10-
t.throws(() => eval('a'));
10+
t.is(eval('a'), undefined);
1111
// eslint-disable-next-line no-eval
1212
t.is(eval('1 + 1'), 2);
1313
// eslint-disable-next-line no-eval

packages/ses/test/evalTaming-safeEval.test.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ test('safe eval when evalTaming is safeEval.', t => {
88
// eslint-disable-next-line no-unused-vars
99
const a = 0;
1010
// eslint-disable-next-line no-eval
11-
t.throws(() => eval('a'));
11+
t.is(eval('a'), undefined);
1212
// eslint-disable-next-line no-eval
1313
t.is(eval('1 + 1'), 2);
1414
// eslint-disable-next-line no-eval

packages/ses/test/global-lexicals-evaluate.test.js

+1-3
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,7 @@ test('endowments prototypically inherited properties are not mentionable', t =>
3636
__options__: true,
3737
});
3838

39-
t.throws(() => compartment.evaluate('hello'), {
40-
message: /hello is not defined/,
41-
});
39+
t.is(compartment.evaluate('hello'), undefined);
4240
});
4341

4442
test('endowments prototypically inherited properties are not enumerable', t => {

packages/ses/test/make-eval-function.test.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,14 @@ import test from 'ava';
66
import { makeEvalFunction } from '../src/make-eval-function.js';
77
import { makeSafeEvaluator } from '../src/make-safe-evaluator.js';
88

9-
test('makeEvalFunction - leak', t => {
9+
test('makeEvalFunction - no leak', t => {
1010
t.plan(8);
1111

1212
const globalObject = {};
1313
const { safeEvaluate } = makeSafeEvaluator({ globalObject });
1414
const safeEval = makeEvalFunction(safeEvaluate);
1515

16-
t.throws(() => safeEval('none'), { instanceOf: ReferenceError });
16+
t.is(safeEval('none'), undefined);
1717
t.is(safeEval('this.none'), undefined);
1818

1919
globalThis.none = 5;

packages/ses/test/make-safe-evaluator.test.js

+20-8
Original file line numberDiff line numberDiff line change
@@ -4,17 +4,29 @@ import test from 'ava';
44
import { makeSafeEvaluator } from '../src/make-safe-evaluator.js';
55

66
test('safeEvaluate - default (non-sloppy, no moduleLexicals)', t => {
7-
t.plan(6);
7+
t.plan(7);
88

99
const globalObject = { abc: 123 };
1010
const { safeEvaluate: evaluate } = makeSafeEvaluator({ globalObject });
1111

1212
t.is(evaluate('typeof def'), 'undefined', 'typeof non declared global');
1313

14-
t.throws(
15-
() => evaluate('def'),
16-
{ instanceOf: ReferenceError },
17-
'non declared global cause a reference error',
14+
// Expected to be undefined in sloppy mode.
15+
t.is(
16+
evaluate(
17+
'def',
18+
undefined,
19+
'non-declared global is undefined in sloppy mode',
20+
),
21+
);
22+
// Known deviation from fidelity to Hardened JavaScript:
23+
// In strict mode, an undeclared lexical name should produce a ReferenceError.
24+
t.is(
25+
evaluate(
26+
'(() => { "use strict"; return def; })()',
27+
undefined,
28+
'non-declared global is undefined in strict mode',
29+
),
1830
);
1931

2032
t.is(evaluate('abc'), 123, 'globals can be referenced');
@@ -61,9 +73,9 @@ test('safeEvaluate - module lexicals', t => {
6173

6274
t.is(endowedEvaluate('abc'), 123, 'module lexicals can be referenced');
6375
t.is(endowedEvaluate('abc += 333'), 456, 'module lexicals can be mutated');
64-
t.throws(
65-
() => evaluate('abc'),
66-
{ instanceOf: ReferenceError },
76+
t.is(
77+
evaluate('abc'),
78+
undefined,
6779
'module lexicals do not affect other evaluate scopes with same globalObject (do not persist)',
6880
);
6981
});

packages/ses/test/permits.test.js

+2-12
Original file line numberDiff line numberDiff line change
@@ -10,18 +10,8 @@ test('indirect eval is possible', t => {
1010

1111
test('SharedArrayBuffer should be removed because it is not permitted', t => {
1212
const c = new Compartment();
13-
// we seem to manage both of these for properties that never existed
14-
// in the first place
15-
t.throws(() => c.evaluate('XYZ'), { instanceOf: ReferenceError });
16-
t.is(c.evaluate('typeof XYZ'), 'undefined');
17-
const have = typeof SharedArrayBuffer !== 'undefined';
18-
if (have) {
19-
// we ideally want both of these, but the realms magic can only
20-
// manage one at a time (for properties that previously existed but
21-
// which were removed by the permits check)
22-
// t.throws(() => c.evaluate('SharedArrayBuffer'), ReferenceError);
23-
t.is(c.evaluate('typeof SharedArrayBuffer'), 'undefined');
24-
}
13+
t.is(c.evaluate('typeof SharedArrayBuffer'), 'undefined');
14+
t.is(c.evaluate('SharedArrayBuffer'), undefined);
2515
});
2616

2717
test('remove RegExp.prototype.compile', t => {

packages/ses/test/scope.test.js

+40-95
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ test('scope behavior - lookup behavior', t => {
2222
t.is(evaluate('globalThis'), undefined);
2323
t.is(evaluate('eval'), undefined);
2424
t.is(evaluate('realmGlobalProp'), undefined);
25-
t.throws(() => evaluate('missingProp'), { instanceOf: ReferenceError });
25+
t.is(evaluate('missingProp'), undefined);
2626

2727
t.is(evaluate('globalProp'), globalObject.globalProp);
2828
t.is(evaluate('lexicalProp'), moduleLexicals.lexicalProp);
@@ -279,39 +279,35 @@ test('scope behavior - strict vs sloppy locally non-existing global set', t => {
279279
t.notThrows(() => evaluateSloppy('missingRealmGlobalProp = 456'));
280280
});
281281

282-
test('scope behavior - realm globalThis property info leak', t => {
282+
test('scope behavior - no realm globalThis property info leak', t => {
283283
t.plan(8);
284284

285285
const globalObject = {};
286286
const { safeEvaluate: evaluate } = makeSafeEvaluator({
287287
globalObject,
288288
});
289289

290-
t.is(evaluate('typeof missingRealmGlobalProp'), 'undefined');
291-
t.is(evaluate('typeof eventuallyAssignedRealmGlobalProp'), 'undefined');
292-
t.throws(() => evaluate('missingRealmGlobalProp'), {
293-
instanceOf: ReferenceError,
294-
});
295-
t.throws(() => evaluate('eventuallyAssignedRealmGlobalProp'), {
296-
instanceOf: ReferenceError,
297-
});
290+
const unvaryingAssertions = () => {
291+
t.is(evaluate('typeof missingRealmGlobalProp'), 'undefined');
292+
t.is(evaluate('typeof eventuallyAssignedRealmGlobalProp'), 'undefined');
293+
// Known loss of fidelity to native Hardened JavaScript: we expect a
294+
// ReferenceError for accessing a missing lexical name.
295+
t.is(evaluate('missingRealmGlobalProp'), undefined);
296+
t.is(evaluate('eventuallyAssignedRealmGlobalProp'), undefined);
297+
};
298+
299+
unvaryingAssertions();
298300

299301
globalThis.eventuallyAssignedRealmGlobalProp = {};
300302
t.teardown(() => {
301303
delete globalThis.eventuallyAssignedRealmGlobalProp;
302304
});
303305

304-
t.is(evaluate('typeof missingRealmGlobalProp'), 'undefined');
305-
t.is(evaluate('typeof eventuallyAssignedRealmGlobalProp'), 'undefined');
306-
t.throws(() => evaluate('missingRealmGlobalProp'), {
307-
instanceOf: ReferenceError,
308-
});
309-
// Known compromise in fidelity of the emulated script environment:
310-
t.is(evaluate('eventuallyAssignedRealmGlobalProp'), undefined);
306+
unvaryingAssertions();
311307
});
312308

313309
test('scope behavior - Symbol.unscopables fidelity test', t => {
314-
t.plan(33);
310+
t.plan(41);
315311

316312
const globalObject = {
317313
Symbol,
@@ -325,81 +321,49 @@ test('scope behavior - Symbol.unscopables fidelity test', t => {
325321
globalObject,
326322
});
327323

328-
// Known compromise in fidelity of the emulated script environment:
329-
t.is(evaluate('typeof localProp'), 'undefined');
330-
t.is(evaluate('typeof eventuallyAssignedLocalProp'), 'undefined');
331-
t.is(evaluate('typeof missingRealmGlobalProp'), 'undefined');
332-
t.is(evaluate('typeof eventuallyAssignedRealmGlobalProp'), 'undefined');
324+
const unvaryingAssertions = () => {
325+
t.is(evaluate('typeof localProp'), 'undefined');
326+
t.is(evaluate('typeof eventuallyAssignedLocalProp'), 'undefined');
327+
t.is(evaluate('typeof missingRealmGlobalProp'), 'undefined');
328+
t.is(evaluate('typeof eventuallyAssignedRealmGlobalProp'), 'undefined');
333329

334-
// Known compromise in fidelity of the emulated script environment:
335-
t.throws(() => evaluate('localProp'), {
336-
instanceOf: ReferenceError,
337-
});
338-
t.throws(() => evaluate('eventuallyAssignedLocalProp'), {
339-
instanceOf: ReferenceError,
340-
});
341-
t.throws(() => evaluate('missingRealmGlobalProp'), {
342-
instanceOf: ReferenceError,
343-
});
344-
t.throws(() => evaluate('eventuallyAssignedRealmGlobalProp'), {
345-
instanceOf: ReferenceError,
346-
});
330+
// Known compromise in fidelity of the emulated script environment:
331+
// In a native Hardened JavaScript, we would expect these to throw
332+
// ReferenceError.
333+
t.is(evaluate('localProp'), undefined);
334+
t.is(evaluate('eventuallyAssignedLocalProp'), undefined);
335+
t.is(evaluate('missingRealmGlobalProp'), undefined);
336+
t.is(evaluate('eventuallyAssignedRealmGlobalProp'), undefined);
337+
};
347338

339+
unvaryingAssertions();
340+
341+
// Compartment should not be sensitive to existence of a host globalThis
342+
// property.
348343
globalThis.eventuallyAssignedRealmGlobalProp = {};
349344
t.teardown(() => {
350345
delete globalThis.eventuallyAssignedRealmGlobalProp;
351346
});
352347

353-
// Known compromise in fidelity of the emulated script environment:
354-
t.is(evaluate('typeof localProp'), 'undefined');
355-
t.is(evaluate('typeof eventuallyAssignedLocalProp'), 'undefined');
356-
t.is(evaluate('typeof missingRealmGlobalProp'), 'undefined');
357-
t.is(evaluate('typeof eventuallyAssignedRealmGlobalProp'), 'undefined');
358-
359-
// Known compromise in fidelity of the emulated script environment:
360-
t.throws(() => evaluate('localProp'), {
361-
instanceOf: ReferenceError,
362-
});
363-
t.throws(() => evaluate('eventuallyAssignedLocalProp'), {
364-
instanceOf: ReferenceError,
365-
});
366-
t.throws(() => evaluate('missingRealmGlobalProp'), {
367-
instanceOf: ReferenceError,
368-
});
369-
// Known compromise in fidelity of the emulated script environment:
370-
t.is(evaluate('eventuallyAssignedRealmGlobalProp'), undefined);
348+
unvaryingAssertions();
371349

372350
evaluate(
373351
'this[Symbol.unscopables] = { eventuallyAssignedRealmGlobalProp: true, localProp: true, eventuallyAssignedLocalProp: true }',
374352
);
375353
// after property is created on globalObject, assignment is evaluated to
376354
// test if it is affected by the Symbol.unscopables configuration
377355
globalObject.eventuallyAssignedLocalProp = null;
378-
// Known compromise in fidelity of the emulated script environment:
379-
t.throws(() => evaluate('eventuallyAssignedLocalProp = {}'), {
380-
instanceOf: ReferenceError,
381-
});
382356

383-
// Known compromise in fidelity of the emulated script environment:
384-
t.is(evaluate('typeof localProp'), 'undefined');
385-
// Known compromise in fidelity of the emulated script environment:
386-
t.is(evaluate('typeof eventuallyAssignedLocalProp'), 'undefined');
387-
t.is(evaluate('typeof missingRealmGlobalProp'), 'undefined');
388-
t.is(evaluate('typeof eventuallyAssignedRealmGlobalProp'), 'undefined');
357+
unvaryingAssertions();
389358

390-
// Known compromise in fidelity of the emulated script environment:
391-
t.throws(() => evaluate('localProp'), {
392-
instanceOf: ReferenceError,
393-
});
394-
// Known compromise in fidelity of the emulated script environment:
395-
t.throws(() => evaluate('eventuallyAssignedLocalProp'), {
396-
instanceOf: ReferenceError,
397-
});
398-
t.throws(() => evaluate('missingRealmGlobalProp'), {
359+
// Known compromise in fidelity to native Hardened JavaScript:
360+
// We expect implicit assignment on globalThis to fail in strict mode but not
361+
// in sloppy mode.
362+
t.throws(() => evaluate('eventuallyAssignedLocalProp = {}'), {
399363
instanceOf: ReferenceError,
400364
});
401-
// Known compromise in fidelity of the emulated script environment:
402-
t.is(evaluate('eventuallyAssignedRealmGlobalProp'), undefined);
365+
366+
unvaryingAssertions();
403367

404368
// move "Symbol.unscopables" to prototype
405369
delete globalObject[Symbol.unscopables];
@@ -410,24 +374,5 @@ test('scope behavior - Symbol.unscopables fidelity test', t => {
410374
eventuallyAssignedLocalProp: true,
411375
};
412376

413-
// Known compromise in fidelity of the emulated script environment:
414-
t.is(evaluate('typeof localProp'), 'undefined');
415-
// Known compromise in fidelity of the emulated script environment:
416-
t.is(evaluate('typeof eventuallyAssignedLocalProp'), 'undefined');
417-
t.is(evaluate('typeof missingRealmGlobalProp'), 'undefined');
418-
t.is(evaluate('typeof eventuallyAssignedRealmGlobalProp'), 'undefined');
419-
420-
// Known compromise in fidelity of the emulated script environment:
421-
t.throws(() => evaluate('localProp'), {
422-
instanceOf: ReferenceError,
423-
});
424-
// Known compromise in fidelity of the emulated script environment:
425-
t.throws(() => evaluate('eventuallyAssignedLocalProp'), {
426-
instanceOf: ReferenceError,
427-
});
428-
t.throws(() => evaluate('missingRealmGlobalProp'), {
429-
instanceOf: ReferenceError,
430-
});
431-
// Known compromise in fidelity of the emulated script environment:
432-
t.is(evaluate('eventuallyAssignedRealmGlobalProp'), undefined);
377+
unvaryingAssertions();
433378
});

packages/ses/test/ses.test.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ test('create', t => {
7171
test('SES compartment does not see primal realm names', t => {
7272
const hidden = 1; // eslint-disable-line no-unused-vars
7373
const c = new Compartment();
74-
t.throws(() => c.evaluate('hidden+1'), { instanceOf: ReferenceError });
74+
t.is(c.evaluate('hidden'), undefined);
7575
});
7676

7777
test('SES compartment also has compartments', t => {

packages/ses/test/strict-scope-terminator.test.js

+4-4
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
import test from 'ava';
22
import { strictScopeTerminator } from '../src/strict-scope-terminator.js';
33

4-
test('strictScopeTerminator/get - always has start compartment properties but they are undefined', t => {
4+
test('strictScopeTerminator/get - has all properties and they are undefined', t => {
55
t.plan(4);
66

77
t.is(Reflect.has(strictScopeTerminator, 'eval'), true);
88
t.is(Reflect.get(strictScopeTerminator, 'eval'), undefined);
99

10-
t.is(Reflect.has(strictScopeTerminator, 'xyz'), false);
10+
t.is(Reflect.has(strictScopeTerminator, 'xyz'), true);
1111
t.is(Reflect.get(strictScopeTerminator, 'xyz'), undefined);
1212
});
1313

@@ -30,7 +30,7 @@ test('strictScopeTerminator/getPrototypeOf - has null prototype', t => {
3030
t.is(Reflect.getPrototypeOf(strictScopeTerminator), null);
3131
});
3232

33-
test('strictScopeTerminator/getOwnPropertyDescriptor - always has start compartment properties but provides no prop desc', t => {
33+
test('strictScopeTerminator/getOwnPropertyDescriptor - traps all properties and provides no prop desc', t => {
3434
t.plan(9);
3535

3636
const originalWarn = console.warn;
@@ -46,7 +46,7 @@ test('strictScopeTerminator/getOwnPropertyDescriptor - always has start compartm
4646
undefined,
4747
);
4848
t.is(didWarn, 1);
49-
t.is(Reflect.has(strictScopeTerminator, 'xyz'), false);
49+
t.is(Reflect.has(strictScopeTerminator, 'xyz'), true);
5050
t.is(didWarn, 1);
5151
t.is(
5252
Reflect.getOwnPropertyDescriptor(strictScopeTerminator, 'xyz'),

packages/ses/test/typeof.test.js

+1-3
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,7 @@ test('typeof', t => {
77

88
const c = new Compartment();
99

10-
t.throws(() => c.evaluate('DEFINITELY_NOT_DEFINED'), {
11-
instanceOf: ReferenceError,
12-
});
10+
t.is(c.evaluate('DEFINITELY_NOT_DEFINED'), undefined);
1311
t.is(c.evaluate('typeof DEFINITELY_NOT_DEFINED'), 'undefined');
1412

1513
t.is(c.evaluate('typeof 4'), 'number');

0 commit comments

Comments
 (0)