Skip to content

Commit 73ad72e

Browse files
80avin80avin
authored
fix(eval): improve security of safe-eval (#233)
* block reading properties 'constructor', '__proto__', '__defineGetter__', '__defineSetter__' if they are not owned by the object. * allow only expected variables in global scope ( removing constructor, __proto__, etc from global scope ) * Remove previous patches to fix security issues. Ensure no breakage by adding unit tests * chore: remove unnecessary changes and rebuild docs rebuild docs using `pnpm run license-badges && pnpm run build-docs && pnpm run lint && pnpm run test`, remove unnecessary changes in test/test.safe-eval.js and badges/license-badge-dev.svg
1 parent 93612a3 commit 73ad72e

22 files changed

+173
-172
lines changed

‎badges/coverage-badge.svg

Lines changed: 1 addition & 1 deletion
Loading

‎badges/licenses-badge-dev.svg

Lines changed: 1 addition & 1 deletion
Loading

‎badges/tests-badge.svg

Lines changed: 1 addition & 1 deletion
Loading

‎dist/index-browser-esm.js

Lines changed: 13 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1203,6 +1203,7 @@ jsep.plugins.register(index, plugin);
12031203
jsep.addUnaryOp('typeof');
12041204
jsep.addLiteral('null', null);
12051205
jsep.addLiteral('undefined', undefined);
1206+
const BLOCKED_PROTO_PROPERTIES = new Set(['constructor', '__proto__', '__defineGetter__', '__defineSetter__']);
12061207
const SafeEval = {
12071208
/**
12081209
* @param {jsep.Expression} ast
@@ -1285,7 +1286,7 @@ const SafeEval = {
12851286
return SafeEval.evalAst(ast.alternate, subs);
12861287
},
12871288
evalIdentifier(ast, subs) {
1288-
if (ast.name in subs) {
1289+
if (Object.hasOwn(subs, ast.name)) {
12891290
return subs[ast.name];
12901291
}
12911292
throw ReferenceError(`${ast.name} is not defined`);
@@ -1294,23 +1295,17 @@ const SafeEval = {
12941295
return ast.value;
12951296
},
12961297
evalMemberExpression(ast, subs) {
1297-
if (ast.property.type === 'Identifier' && ast.property.name === 'constructor' || ast.object.type === 'Identifier' && ast.object.name === 'constructor') {
1298-
throw new Error("'constructor' property is disabled");
1299-
}
13001298
const prop = ast.computed ? SafeEval.evalAst(ast.property) // `object[property]`
13011299
: ast.property.name; // `object.property` property is Identifier
13021300
const obj = SafeEval.evalAst(ast.object, subs);
1301+
if (obj === undefined || obj === null) {
1302+
throw TypeError(`Cannot read properties of ${obj} (reading '${prop}')`);
1303+
}
1304+
if (!Object.hasOwn(obj, prop) && BLOCKED_PROTO_PROPERTIES.has(prop)) {
1305+
throw TypeError(`Cannot read properties of ${obj} (reading '${prop}')`);
1306+
}
13031307
const result = obj[prop];
13041308
if (typeof result === 'function') {
1305-
if (obj === Function && prop === 'bind') {
1306-
throw new Error('Function.prototype.bind is disabled');
1307-
}
1308-
if (obj === Function && (prop === 'call' || prop === 'apply')) {
1309-
throw new Error('Function.prototype.call and ' + 'Function.prototype.apply are disabled');
1310-
}
1311-
if (result === Function) {
1312-
return result; // Don't bind so can identify and throw later
1313-
}
13141309
return result.bind(obj); // arrow functions aren't affected by bind.
13151310
}
13161311
return result;
@@ -1332,19 +1327,16 @@ const SafeEval = {
13321327
evalCallExpression(ast, subs) {
13331328
const args = ast.arguments.map(arg => SafeEval.evalAst(arg, subs));
13341329
const func = SafeEval.evalAst(ast.callee, subs);
1335-
if (func === Function) {
1336-
throw new Error('Function constructor is disabled');
1337-
}
1330+
// if (func === Function) {
1331+
// throw new Error('Function constructor is disabled');
1332+
// }
13381333
return func(...args);
13391334
},
13401335
evalAssignmentExpression(ast, subs) {
13411336
if (ast.left.type !== 'Identifier') {
13421337
throw SyntaxError('Invalid left-hand side in assignment');
13431338
}
13441339
const id = ast.left.name;
1345-
if (id === '__proto__') {
1346-
throw new Error('Assignment to __proto__ is disabled');
1347-
}
13481340
const value = SafeEval.evalAst(ast.right, subs);
13491341
subs[id] = value;
13501342
return subs[id];
@@ -1369,9 +1361,8 @@ class SafeScript {
13691361
* @returns {EvaluatedResult} Result of evaluated code
13701362
*/
13711363
runInNewContext(context) {
1372-
const keyMap = {
1373-
...context
1374-
};
1364+
// `Object.create(null)` creates a prototypeless object
1365+
const keyMap = Object.assign(Object.create(null), context);
13751366
return SafeEval.evalAst(this.ast, keyMap);
13761367
}
13771368
}

‎dist/index-browser-esm.min.js

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎dist/index-browser-esm.min.js.map

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎dist/index-browser-umd.cjs

Lines changed: 13 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1209,6 +1209,7 @@
12091209
jsep.addUnaryOp('typeof');
12101210
jsep.addLiteral('null', null);
12111211
jsep.addLiteral('undefined', undefined);
1212+
const BLOCKED_PROTO_PROPERTIES = new Set(['constructor', '__proto__', '__defineGetter__', '__defineSetter__']);
12121213
const SafeEval = {
12131214
/**
12141215
* @param {jsep.Expression} ast
@@ -1291,7 +1292,7 @@
12911292
return SafeEval.evalAst(ast.alternate, subs);
12921293
},
12931294
evalIdentifier(ast, subs) {
1294-
if (ast.name in subs) {
1295+
if (Object.hasOwn(subs, ast.name)) {
12951296
return subs[ast.name];
12961297
}
12971298
throw ReferenceError(`${ast.name} is not defined`);
@@ -1300,23 +1301,17 @@
13001301
return ast.value;
13011302
},
13021303
evalMemberExpression(ast, subs) {
1303-
if (ast.property.type === 'Identifier' && ast.property.name === 'constructor' || ast.object.type === 'Identifier' && ast.object.name === 'constructor') {
1304-
throw new Error("'constructor' property is disabled");
1305-
}
13061304
const prop = ast.computed ? SafeEval.evalAst(ast.property) // `object[property]`
13071305
: ast.property.name; // `object.property` property is Identifier
13081306
const obj = SafeEval.evalAst(ast.object, subs);
1307+
if (obj === undefined || obj === null) {
1308+
throw TypeError(`Cannot read properties of ${obj} (reading '${prop}')`);
1309+
}
1310+
if (!Object.hasOwn(obj, prop) && BLOCKED_PROTO_PROPERTIES.has(prop)) {
1311+
throw TypeError(`Cannot read properties of ${obj} (reading '${prop}')`);
1312+
}
13091313
const result = obj[prop];
13101314
if (typeof result === 'function') {
1311-
if (obj === Function && prop === 'bind') {
1312-
throw new Error('Function.prototype.bind is disabled');
1313-
}
1314-
if (obj === Function && (prop === 'call' || prop === 'apply')) {
1315-
throw new Error('Function.prototype.call and ' + 'Function.prototype.apply are disabled');
1316-
}
1317-
if (result === Function) {
1318-
return result; // Don't bind so can identify and throw later
1319-
}
13201315
return result.bind(obj); // arrow functions aren't affected by bind.
13211316
}
13221317
return result;
@@ -1338,19 +1333,16 @@
13381333
evalCallExpression(ast, subs) {
13391334
const args = ast.arguments.map(arg => SafeEval.evalAst(arg, subs));
13401335
const func = SafeEval.evalAst(ast.callee, subs);
1341-
if (func === Function) {
1342-
throw new Error('Function constructor is disabled');
1343-
}
1336+
// if (func === Function) {
1337+
// throw new Error('Function constructor is disabled');
1338+
// }
13441339
return func(...args);
13451340
},
13461341
evalAssignmentExpression(ast, subs) {
13471342
if (ast.left.type !== 'Identifier') {
13481343
throw SyntaxError('Invalid left-hand side in assignment');
13491344
}
13501345
const id = ast.left.name;
1351-
if (id === '__proto__') {
1352-
throw new Error('Assignment to __proto__ is disabled');
1353-
}
13541346
const value = SafeEval.evalAst(ast.right, subs);
13551347
subs[id] = value;
13561348
return subs[id];
@@ -1375,9 +1367,8 @@
13751367
* @returns {EvaluatedResult} Result of evaluated code
13761368
*/
13771369
runInNewContext(context) {
1378-
const keyMap = {
1379-
...context
1380-
};
1370+
// `Object.create(null)` creates a prototypeless object
1371+
const keyMap = Object.assign(Object.create(null), context);
13811372
return SafeEval.evalAst(this.ast, keyMap);
13821373
}
13831374
}

‎dist/index-browser-umd.min.cjs

Lines changed: 1 addition & 1 deletion
Large diffs are not rendered by default.

‎dist/index-browser-umd.min.cjs.map

Lines changed: 1 addition & 1 deletion
Large diffs are not rendered by default.

‎dist/index-node-cjs.cjs

Lines changed: 13 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1204,6 +1204,7 @@ jsep.plugins.register(index, plugin);
12041204
jsep.addUnaryOp('typeof');
12051205
jsep.addLiteral('null', null);
12061206
jsep.addLiteral('undefined', undefined);
1207+
const BLOCKED_PROTO_PROPERTIES = new Set(['constructor', '__proto__', '__defineGetter__', '__defineSetter__']);
12071208
const SafeEval = {
12081209
/**
12091210
* @param {jsep.Expression} ast
@@ -1286,7 +1287,7 @@ const SafeEval = {
12861287
return SafeEval.evalAst(ast.alternate, subs);
12871288
},
12881289
evalIdentifier(ast, subs) {
1289-
if (ast.name in subs) {
1290+
if (Object.hasOwn(subs, ast.name)) {
12901291
return subs[ast.name];
12911292
}
12921293
throw ReferenceError(`${ast.name} is not defined`);
@@ -1295,23 +1296,17 @@ const SafeEval = {
12951296
return ast.value;
12961297
},
12971298
evalMemberExpression(ast, subs) {
1298-
if (ast.property.type === 'Identifier' && ast.property.name === 'constructor' || ast.object.type === 'Identifier' && ast.object.name === 'constructor') {
1299-
throw new Error("'constructor' property is disabled");
1300-
}
13011299
const prop = ast.computed ? SafeEval.evalAst(ast.property) // `object[property]`
13021300
: ast.property.name; // `object.property` property is Identifier
13031301
const obj = SafeEval.evalAst(ast.object, subs);
1302+
if (obj === undefined || obj === null) {
1303+
throw TypeError(`Cannot read properties of ${obj} (reading '${prop}')`);
1304+
}
1305+
if (!Object.hasOwn(obj, prop) && BLOCKED_PROTO_PROPERTIES.has(prop)) {
1306+
throw TypeError(`Cannot read properties of ${obj} (reading '${prop}')`);
1307+
}
13041308
const result = obj[prop];
13051309
if (typeof result === 'function') {
1306-
if (obj === Function && prop === 'bind') {
1307-
throw new Error('Function.prototype.bind is disabled');
1308-
}
1309-
if (obj === Function && (prop === 'call' || prop === 'apply')) {
1310-
throw new Error('Function.prototype.call and ' + 'Function.prototype.apply are disabled');
1311-
}
1312-
if (result === Function) {
1313-
return result; // Don't bind so can identify and throw later
1314-
}
13151310
return result.bind(obj); // arrow functions aren't affected by bind.
13161311
}
13171312
return result;
@@ -1333,19 +1328,16 @@ const SafeEval = {
13331328
evalCallExpression(ast, subs) {
13341329
const args = ast.arguments.map(arg => SafeEval.evalAst(arg, subs));
13351330
const func = SafeEval.evalAst(ast.callee, subs);
1336-
if (func === Function) {
1337-
throw new Error('Function constructor is disabled');
1338-
}
1331+
// if (func === Function) {
1332+
// throw new Error('Function constructor is disabled');
1333+
// }
13391334
return func(...args);
13401335
},
13411336
evalAssignmentExpression(ast, subs) {
13421337
if (ast.left.type !== 'Identifier') {
13431338
throw SyntaxError('Invalid left-hand side in assignment');
13441339
}
13451340
const id = ast.left.name;
1346-
if (id === '__proto__') {
1347-
throw new Error('Assignment to __proto__ is disabled');
1348-
}
13491341
const value = SafeEval.evalAst(ast.right, subs);
13501342
subs[id] = value;
13511343
return subs[id];
@@ -1370,9 +1362,8 @@ class SafeScript {
13701362
* @returns {EvaluatedResult} Result of evaluated code
13711363
*/
13721364
runInNewContext(context) {
1373-
const keyMap = {
1374-
...context
1375-
};
1365+
// `Object.create(null)` creates a prototypeless object
1366+
const keyMap = Object.assign(Object.create(null), context);
13761367
return SafeEval.evalAst(this.ast, keyMap);
13771368
}
13781369
}

0 commit comments

Comments
 (0)