Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lint to disallow SES polymorphic calls #827

Merged
merged 3 commits into from
Aug 28, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 52 additions & 0 deletions packages/eslint-plugin/lib/rules/no-polymorphic-call.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
"use strict"

module.exports = {
meta: {
docs: {
description:
"disallow polymorphic function calls e.g.: 'array.slice()'",
category: "Possible Security Errors",
recommended: true,
url:
"https://github.com/endojs/endo/blob/master/packages/eslint-plugin/lib/rules/no-polymorphic-call.js",
},
type: "problem",
fixable: null,
schema: [],
supported: true,
},
create (context) {
return {
CallExpression(node) {
if (node.callee.type !== 'MemberExpression') {
return
}
const reportHint = prepareMemberExpressionHint(node.callee)
context.report(node, `Polymorphic call: "${reportHint}". May be vulnerable to corruption or trap`)
}
}
},
}

function prepareMemberExpressionHint (node) {
const { object, property, computed } = node
let objectHint
let propertyHint
if (object.type === 'Identifier') {
objectHint = object.name
} else if (object.type === 'MemberExpression') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it cover a[b]?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This certainly did produce lint errors for a[b]() syntax.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes same member expression but with computed flag set to true

objectHint = prepareMemberExpressionHint(object)
} else {
objectHint = `[[${object.type}]]`
}
if (property.type === 'Identifier') {
if (computed) {
propertyHint = `[${property.name}]`
} else {
propertyHint = property.name
}
} else {
propertyHint = `[[${property.type}]]`
}
return `${objectHint}.${propertyHint}`
}
4 changes: 2 additions & 2 deletions packages/ses/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

import { globalThis, Error, assign } from './src/commons.js';
import { globalThis, TypeError, assign } from './src/commons.js';
import { tameFunctionToString } from './src/tame-function-tostring.js';
import { getGlobalIntrinsics } from './src/intrinsics.js';
import { getAnonymousIntrinsics } from './src/get-anonymous-intrinsics.js';
Expand All @@ -29,7 +29,7 @@ function getThis() {
}

if (getThis()) {
throw new Error(`SES failed to initialize, sloppy mode (SES_NO_SLOPPY)`);
throw new TypeError(`SES failed to initialize, sloppy mode (SES_NO_SLOPPY)`);
}

const markVirtualizedNativeFunction = tameFunctionToString();
Expand Down
9 changes: 6 additions & 3 deletions packages/ses/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -142,16 +142,19 @@
"parseFloat",
"parseInt",
"unescape"
]
],
"@endo/no-polymorphic-call": "error"
},
"overrides": [
{
"files": [
"test/**/*.js",
"demos/**/*.js"
"demos/**/*.js",
"scripts/**/*.js"
],
"rules": {
"no-restricted-globals": "off"
"no-restricted-globals": "off",
"@endo/no-polymorphic-call": "off"
}
}
]
Expand Down
51 changes: 43 additions & 8 deletions packages/ses/src/commons.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,18 @@ export const {
Promise,
Proxy,
Reflect,
RegExp,
RegExp: FERAL_REG_EXP,
Set,
String,
WeakMap,
WeakSet,
} = globalThis;

export const {
Error,
// The feral Error constructor is safe for internal use, but must not be
// revealed to post-lockdown code in any compartment including the start
// compartment since in V8 at least it bears stack inspection capabilities.
Error: FERAL_ERROR,
RangeError,
ReferenceError,
SyntaxError,
Expand Down Expand Up @@ -102,7 +105,7 @@ export const defineProperty = (object, prop, descriptor) => {
const result = originalDefineProperty(object, prop, descriptor);
if (result !== object) {
throw TypeError(
`Please report that the original defineProperty silently failed to set ${JSON.stringify(
`Please report that the original defineProperty silently failed to set ${stringifyJson(
String(prop),
)}. (SES_DEFINE_PROPERTY_FAILED_SILENTLY)`,
);
Expand Down Expand Up @@ -131,6 +134,7 @@ export const { prototype: stringPrototype } = String;
export const { prototype: weakmapPrototype } = WeakMap;
export const { prototype: weaksetPrototype } = WeakSet;
export const { prototype: functionPrototype } = Function;
export const { prototype: promisePrototype } = Promise;

/**
* uncurryThis()
Expand All @@ -151,40 +155,58 @@ export const uncurryThis = fn => (thisArg, ...args) => apply(fn, thisArg, args);

export const objectHasOwnProperty = uncurryThis(objectPrototype.hasOwnProperty);
//
export const arrayForEach = uncurryThis(arrayPrototype.forEach);
export const arrayFilter = uncurryThis(arrayPrototype.filter);
export const arrayForEach = uncurryThis(arrayPrototype.forEach);
export const arrayIncludes = uncurryThis(arrayPrototype.includes);
export const arrayJoin = uncurryThis(arrayPrototype.join);
export const arrayPush = uncurryThis(arrayPrototype.push);
export const arrayMap = uncurryThis(arrayPrototype.map);
export const arrayPop = uncurryThis(arrayPrototype.pop);
export const arrayIncludes = uncurryThis(arrayPrototype.includes);
export const arrayPush = uncurryThis(arrayPrototype.push);
export const arraySlice = uncurryThis(arrayPrototype.slice);
export const arraySome = uncurryThis(arrayPrototype.some);
export const arraySort = uncurryThis(arrayPrototype.sort);
export const iterateArray = uncurryThis(arrayPrototype[iteratorSymbol]);
//
export const mapSet = uncurryThis(mapPrototype.set);
export const mapGet = uncurryThis(mapPrototype.get);
export const mapHas = uncurryThis(mapPrototype.has);
export const iterateMap = uncurryThis(mapPrototype[iteratorSymbol]);
//
export const setAdd = uncurryThis(setPrototype.add);
export const setForEach = uncurryThis(setPrototype.forEach);
export const setHas = uncurryThis(setPrototype.has);
export const iterateSet = uncurryThis(setPrototype[iteratorSymbol]);
//
export const regexpTest = uncurryThis(regexpPrototype.test);
export const regexpExec = uncurryThis(regexpPrototype.exec);
export const matchAllRegExp = uncurryThis(regexpPrototype[matchAllSymbol]);
//
export const stringEndsWith = uncurryThis(stringPrototype.endsWith);
export const stringIncludes = uncurryThis(stringPrototype.includes);
export const stringIndexOf = uncurryThis(stringPrototype.indexOf);
export const stringMatch = uncurryThis(stringPrototype.match);
export const stringReplace = uncurryThis(stringPrototype.replace);
export const stringSearch = uncurryThis(stringPrototype.search);
export const stringSlice = uncurryThis(stringPrototype.slice);
export const stringSplit = uncurryThis(stringPrototype.split);
export const stringStartsWith = uncurryThis(stringPrototype.startsWith);
export const iterateString = uncurryThis(stringPrototype[iteratorSymbol]);
//
export const weakmapDelete = uncurryThis(weakmapPrototype.delete);
export const weakmapGet = uncurryThis(weakmapPrototype.get);
export const weakmapSet = uncurryThis(weakmapPrototype.set);
export const weakmapHas = uncurryThis(weakmapPrototype.has);
export const weakmapSet = uncurryThis(weakmapPrototype.set);
//
export const weaksetAdd = uncurryThis(weaksetPrototype.add);
export const weaksetSet = uncurryThis(weaksetPrototype.set);
export const weaksetGet = uncurryThis(weaksetPrototype.get);
export const weaksetHas = uncurryThis(weaksetPrototype.has);
//
export const functionToString = uncurryThis(functionPrototype.toString);
//
const { all } = Promise;
export const promiseAll = promises => apply(all, Promise, [promises]);
export const promiseCatch = uncurryThis(promisePrototype.catch);
export const promiseThen = uncurryThis(promisePrototype.then);

/**
* getConstructorOf()
Expand Down Expand Up @@ -220,6 +242,19 @@ export const immutableObject = freeze(create(null));
*/
export const isObject = value => Object(value) === value;

/**
* isError tests whether an object inherits from the intrinsic
* `Error.prototype`.
* We capture the original error constructor as FERAL_ERROR to provide a clear
* signal for reviewers that we are handling an object with excess authority,
* like stack trace inspection, that we are carefully hiding from client code.
* Checking instanceof happens to be safe, but to avoid uttering FERAL_ERROR
* for such a trivial case outside commons.js, we provide a utility function.
*
* @param {any} value
*/
export const isError = value => value instanceof FERAL_ERROR;

// The original unsafe untamed eval function, which must not escape.
// Sample at module initialization time, which is before lockdown can
// repair it. Use it only to build powerless abstractions.
Expand Down
72 changes: 72 additions & 0 deletions packages/ses/src/compartment-evaluate.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
/// <reference types="ses">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Compartment.evaluate implementation needed to be extracted into a stand-alone module so it could be used from both compartment-shim.js and module-instance.js without dynamic dispatch. The internals communicate purely through their captured private fields.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I (MarkM) did not review this refactoring.

import {
TypeError,
arrayPush,
create,
defineProperties,
getOwnPropertyDescriptors,
} from './commons.js';
import {
evadeHtmlCommentTest,
evadeImportExpressionTest,
rejectSomeDirectEvalExpressions,
} from './transforms.js';
import { performEval } from './evaluate.js';

export const compartmentEvaluate = (compartmentFields, source, options) => {
// Perform this check first to avoid unecessary sanitizing.
// TODO Maybe relax string check and coerce instead:
// https://github.com/tc39/proposal-dynamic-code-brand-checks
if (typeof source !== 'string') {
throw new TypeError('first argument of evaluate() must be a string');
}

// Extract options, and shallow-clone transforms.
const {
transforms = [],
sloppyGlobalsMode = false,
__moduleShimLexicals__ = undefined,
__evadeHtmlCommentTest__ = false,
__evadeImportExpressionTest__ = false,
__rejectSomeDirectEvalExpressions__ = true, // Note default on
} = options;
const localTransforms = [...transforms];
if (__evadeHtmlCommentTest__ === true) {
arrayPush(localTransforms, evadeHtmlCommentTest);
}
if (__evadeImportExpressionTest__ === true) {
arrayPush(localTransforms, evadeImportExpressionTest);
}
if (__rejectSomeDirectEvalExpressions__ === true) {
arrayPush(localTransforms, rejectSomeDirectEvalExpressions);
}

let { globalTransforms } = compartmentFields;
const { globalObject, globalLexicals, knownScopeProxies } = compartmentFields;

let localObject = globalLexicals;
if (__moduleShimLexicals__ !== undefined) {
// When using `evaluate` for ESM modules, as should only occur from the
// module-shim's module-instance.js, we do not reveal the SES-shim's
// module-to-program translation, as this is not standardizable behavior.
// However, the `localTransforms` will come from the `__shimTransforms__`
// Compartment option in this case, which is a non-standardizable escape
// hatch so programs designed specifically for the SES-shim
// implementation may opt-in to use the same transforms for `evaluate`
// and `import`, at the expense of being tightly coupled to SES-shim.
globalTransforms = undefined;

localObject = create(null, getOwnPropertyDescriptors(globalLexicals));
defineProperties(
localObject,
getOwnPropertyDescriptors(__moduleShimLexicals__),
);
}

return performEval(source, globalObject, localObject, {
globalTransforms,
localTransforms,
sloppyGlobalsMode,
knownScopeProxies,
});
};
Loading