Skip to content

Commit

Permalink
fix(ses): handle properties already override protected
Browse files Browse the repository at this point in the history
  • Loading branch information
erights committed Jan 16, 2024
1 parent 4a4f9fe commit d0de868
Show file tree
Hide file tree
Showing 5 changed files with 295 additions and 36 deletions.
72 changes: 36 additions & 36 deletions packages/ses/src/enable-property-overrides.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import {
defineProperty,
getOwnPropertyDescriptor,
getOwnPropertyDescriptors,
getOwnPropertyNames,
isObject,
objectHasOwnProperty,
ownKeys,
Expand Down Expand Up @@ -88,42 +87,46 @@ export default function enablePropertyOverrides(
if ('value' in desc && desc.configurable) {
const { value } = desc;

function getter() {
return value;
}
const isDebug = setHas(debugProperties, prop);

// We use concise method syntax to be `this` sensitive but still
// omit a prototype property of [[Construct]] behavior.
const { getter, setter } = {
getter() {
return value;
},
setter(newValue) {
if (obj === this) {
throw TypeError(
`Cannot assign to read only property '${String(
prop,
)}' of '${path}'`,
);
}
if (objectHasOwnProperty(this, prop)) {
this[prop] = newValue;
} else {
if (isDebug) {
// eslint-disable-next-line @endo/no-polymorphic-call
console.error(TypeError(`Override property ${prop}`));
}
defineProperty(this, prop, {
value: newValue,
writable: true,
enumerable: true,
configurable: true,
});
}
},
};

defineProperty(getter, 'originalValue', {
value,
writable: false,
enumerable: false,
configurable: false,
});

const isDebug = setHas(debugProperties, prop);

function setter(newValue) {
if (obj === this) {
throw TypeError(
`Cannot assign to read only property '${String(
prop,
)}' of '${path}'`,
);
}
if (objectHasOwnProperty(this, prop)) {
this[prop] = newValue;
} else {
if (isDebug) {
// eslint-disable-next-line @endo/no-polymorphic-call
console.error(TypeError(`Override property ${prop}`));
}
defineProperty(this, prop, {
value: newValue,
writable: true,
enumerable: true,
configurable: true,
});
}
}

defineProperty(obj, prop, {
get: getter,
set: setter,
Expand All @@ -148,23 +151,20 @@ export default function enablePropertyOverrides(
}
// TypeScript does not allow symbols to be used as indexes because it
// cannot recokon types of symbolized properties.
// @ts-ignore
arrayForEach(ownKeys(descs), prop => enable(path, obj, prop, descs[prop]));
}

function enableProperties(path, obj, plan) {
for (const prop of getOwnPropertyNames(plan)) {
for (const prop of ownKeys(plan)) {
const desc = getOwnPropertyDescriptor(obj, prop);
if (!desc || desc.get || desc.set) {
// No not a value property, nothing to do.
// eslint-disable-next-line no-continue
continue;
}

// Plan has no symbol keys and we use getOwnPropertyNames()
// so `prop` cannot only be a string, not a symbol. We coerce it in place
// with `String(..)` anyway just as good hygiene, since these paths are just
// for diagnostic purposes.
// In case `prop` is a symbol, we first coerce it with `String`,
// purely for diagnostic purposes.
const subPath = `${path}.${String(prop)}`;
const subPlan = plan[prop];

Expand Down
13 changes: 13 additions & 0 deletions packages/ses/src/enablements.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { toStringTagSymbol } from './commons.js';

/**
* @file Exports {@code enablements}, a recursively defined
* JSON record defining the optimum set of intrinsics properties
Expand Down Expand Up @@ -74,6 +76,13 @@ export const minEnablements = {
'%ErrorPrototype%': {
name: true, // set by "precond", "ava", "node-fetch"
},
'%IteratorPrototype%': {
toString: true,
// https://github.com/tc39/proposal-iterator-helpers
constructor: true,
// https://github.com/tc39/proposal-iterator-helpers
[toStringTagSymbol]: true,
},
};

/**
Expand Down Expand Up @@ -152,6 +161,10 @@ export const moderateEnablements = {

'%IteratorPrototype%': {
toString: true,
// https://github.com/tc39/proposal-iterator-helpers
constructor: true,
// https://github.com/tc39/proposal-iterator-helpers
[toStringTagSymbol]: true,
},
};

Expand Down
3 changes: 3 additions & 0 deletions packages/ses/src/lockdown.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ import { getAnonymousIntrinsics } from './get-anonymous-intrinsics.js';
import { makeCompartmentConstructor } from './compartment.js';
import { tameHarden } from './tame-harden.js';
import { tameSymbolConstructor } from './tame-symbol-constructor.js';
import { tameFauxDataProperties } from './tame-faux-data-properties.js';

/** @typedef {import('../types.js').LockdownOptions} LockdownOptions */

Expand Down Expand Up @@ -335,6 +336,8 @@ export const repairIntrinsics = (options = {}) => {
// Replace *Locale* methods with their non-locale equivalents
tameLocaleMethods(intrinsics, localeTaming);

tameFauxDataProperties(intrinsics);

/**
* 2. WHITELIST to standardize the environment.
*/
Expand Down
131 changes: 131 additions & 0 deletions packages/ses/src/tame-faux-data-properties.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
import {
getOwnPropertyDescriptor,
apply,
defineProperty,
toStringTagSymbol,
} from './commons.js';

const throws = thunk => {
try {
thunk();
} catch (er) {
return true;
}
return false;
};

/**
* Exported for convenience of unit testing. Harmless, but not expected
* to be useful by itself.
*
* @param {any} obj
* @param {string|symbol} prop
* @param {any} expectedValue
* @returns {boolean}
*/
export const tameFauxDataProperty = (obj, prop, expectedValue) => {
if (obj === undefined) {
return false;
}
const desc = getOwnPropertyDescriptor(obj, prop);
if (!desc || 'value' in desc) {
return false;
}
const { get, set } = desc;
if (typeof get !== 'function') {
return false;
}
if (typeof set !== 'function') {
return false;
}
const observedValue = get();
if (observedValue !== expectedValue) {
return false;
}
if (apply(get, obj, []) !== expectedValue) {
return false;
}
const testValue = 'Seems to be a setter';
const subject1 = { __proto__: null };
apply(set, subject1, [testValue]);
if (subject1[prop] !== testValue) {
return false;
}
const subject2 = { __proto__: obj };
apply(set, subject2, [testValue]);
if (subject2[prop] !== testValue) {
return false;
}
if (!throws(() => apply(set, obj, [testValue]))) {
return false;
}
if ('originalValue' in get) {
return false;
}

// We assume that this code runs before any untrusted code runs, so
// we do not need to worry about the above conditions passing because of
// malicious intent. In fact, it runs even before vetted shims are supposed
// to run, between repair and hardening. Given that, after all these tests
// pass, we have adequately validated that the property in question is
// an accessor function whose purpose is suppressing the override mistake,
// i.e., enabling a non-writable property to be overridden by assignment.
// In that case, here we *temporarily* turn it into the data property
// it seems to emulate, but writable so that it does not trigger the
// override mistake while in this temporary state.

// For those properties that are also listed in `enablements.js`,
// that phase will re-enable override for these properties, but
// via accessor functions that ses controls, so we know what they are
// doing. In addition, the getter functions installed by
// `enable-property-overrides.js` have an `originalValue` field
// enabling meta-traversal code like harden to visit the original value
// without calling the getter.

if (desc.configurable === false) {
// Even though it seems to be an accessor, we're unable to fix it.
return false;
}

// Many of the `return false;` cases above plausibly should be turned into
// errors, or an least generate warnings. However, for those, the checks
// following this phase are likely to signal an error anyway.

defineProperty(obj, prop, {
value: expectedValue,
writable: true,
enumerable: desc.enumerable,
configurable: true,
});

return true;
};

/**
* For each, see it the property is an accessor property that
* seems to emulate a data property with this expectedValue,
* and seems to be an accessor whose purpose is to protect against
* the override mistake, i.e., enable these properties to be overridden
* by assignment. If all these expected conditions are met, then
* *temporarily* turn it into the data property it emulated.
*
* Coordinate with `enablements.js` so the appropriate ones are
* turned back to accessor that protect against override mistake,
* but accessors we know.
*
* @param {Record<any,any>} intrinsics
*/
export const tameFauxDataProperties = intrinsics => {
// https://github.com/tc39/proposal-iterator-helpers
tameFauxDataProperty(
intrinsics['%IteratorPrototype%'],
'constructor',
intrinsics.Iterator,
);
// https://github.com/tc39/proposal-iterator-helpers
tameFauxDataProperty(
intrinsics['%IteratorPrototype%'],
toStringTagSymbol,
'Iterator',
);
};
Loading

0 comments on commit d0de868

Please sign in to comment.