Skip to content

Commit

Permalink
fix(eval): improve security of safe-eval
Browse files Browse the repository at this point in the history 
* 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
  • Loading branch information
@80avin
80avin committed Nov 15, 2024
1 parent b70aa71 commit 17eadab
Show file tree
Hide file tree
Showing 30 changed files with 676 additions and 506 deletions.
2 changes: 1 addition & 1 deletion badges/coverage-badge.svg
View file
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 1 addition & 1 deletion badges/licenses-badge-dev.svg
View file
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 1 addition & 1 deletion badges/tests-badge.svg
View file
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
35 changes: 13 additions & 22 deletions dist/index-browser-esm.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1200,6 +1200,7 @@ const plugin = {

// register plugins
jsep.plugins.register(index, plugin);
const BLOCKED_PROTO_PROPERTIES = new Set(['constructor', '__proto__', '__defineGetter__', '__defineSetter__', '__lookupGetter__', '__lookupSetter__']);
const SafeEval = {
/**
* @param {jsep.Expression} ast
Expand Down Expand Up @@ -1282,7 +1283,7 @@ const SafeEval = {
return SafeEval.evalAst(ast.alternate, subs);
},
evalIdentifier(ast, subs) {
if (ast.name in subs) {
if (Object.hasOwn(subs, ast.name)) {
return subs[ast.name];
}
throw ReferenceError(`${ast.name} is not defined`);
Expand All @@ -1291,23 +1292,17 @@ const SafeEval = {
return ast.value;
},
evalMemberExpression(ast, subs) {
if (ast.property.type === 'Identifier' && ast.property.name === 'constructor' || ast.object.type === 'Identifier' && ast.object.name === 'constructor') {
throw new Error("'constructor' property is disabled");
}
const prop = ast.computed ? SafeEval.evalAst(ast.property) // `object[property]`
: ast.property.name; // `object.property` property is Identifier
const obj = SafeEval.evalAst(ast.object, subs);
if (obj === undefined || obj === null) {
throw TypeError(`Cannot read properties of ${obj} (reading '${prop}')`);
}
if (!Object.hasOwn(obj, prop) && BLOCKED_PROTO_PROPERTIES.has(prop)) {
throw TypeError(`Cannot read properties of ${obj} (reading '${prop}')`);
}
const result = obj[prop];
if (typeof result === 'function') {
if (obj === Function && prop === 'bind') {
throw new Error('Function.prototype.bind is disabled');
}
if (obj === Function && (prop === 'call' || prop === 'apply')) {
throw new Error('Function.prototype.call and ' + 'Function.prototype.apply are disabled');
}
if (result === Function) {
return result; // Don't bind so can identify and throw later
}
return result.bind(obj); // arrow functions aren't affected by bind.
}
return result;
Expand All @@ -1328,19 +1323,16 @@ const SafeEval = {
evalCallExpression(ast, subs) {
const args = ast.arguments.map(arg => SafeEval.evalAst(arg, subs));
const func = SafeEval.evalAst(ast.callee, subs);
if (func === Function) {
throw new Error('Function constructor is disabled');
}
// if (func === Function) {
// throw new Error('Function constructor is disabled');
// }
return func(...args);
},
evalAssignmentExpression(ast, subs) {
if (ast.left.type !== 'Identifier') {
throw SyntaxError('Invalid left-hand side in assignment');
}
const id = ast.left.name;
if (id === '__proto__') {
throw new Error('Assignment to __proto__ is disabled');
}
const value = SafeEval.evalAst(ast.right, subs);
subs[id] = value;
return subs[id];
Expand All @@ -1365,9 +1357,8 @@ class SafeScript {
* @returns {EvaluatedResult} Result of evaluated code
*/
runInNewContext(context) {
const keyMap = {
...context
};
// `Object.create(null)` creates a prototypeless object
const keyMap = Object.assign(Object.create(null), context);
return SafeEval.evalAst(this.ast, keyMap);
}
}
Expand Down
2 changes: 1 addition & 1 deletion dist/index-browser-esm.min.js
View file

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion dist/index-browser-esm.min.js.map
View file

Large diffs are not rendered by default.

35 changes: 13 additions & 22 deletions dist/index-browser-umd.cjs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1206,6 +1206,7 @@

// register plugins
jsep.plugins.register(index, plugin);
const BLOCKED_PROTO_PROPERTIES = new Set(['constructor', '__proto__', '__defineGetter__', '__defineSetter__', '__lookupGetter__', '__lookupSetter__']);
const SafeEval = {
/**
* @param {jsep.Expression} ast
Expand Down Expand Up @@ -1288,7 +1289,7 @@
return SafeEval.evalAst(ast.alternate, subs);
},
evalIdentifier(ast, subs) {
if (ast.name in subs) {
if (Object.hasOwn(subs, ast.name)) {
return subs[ast.name];
}
throw ReferenceError(`${ast.name} is not defined`);
Expand All @@ -1297,23 +1298,17 @@
return ast.value;
},
evalMemberExpression(ast, subs) {
if (ast.property.type === 'Identifier' && ast.property.name === 'constructor' || ast.object.type === 'Identifier' && ast.object.name === 'constructor') {
throw new Error("'constructor' property is disabled");
}
const prop = ast.computed ? SafeEval.evalAst(ast.property) // `object[property]`
: ast.property.name; // `object.property` property is Identifier
const obj = SafeEval.evalAst(ast.object, subs);
if (obj === undefined || obj === null) {
throw TypeError(`Cannot read properties of ${obj} (reading '${prop}')`);
}
if (!Object.hasOwn(obj, prop) && BLOCKED_PROTO_PROPERTIES.has(prop)) {
throw TypeError(`Cannot read properties of ${obj} (reading '${prop}')`);
}
const result = obj[prop];
if (typeof result === 'function') {
if (obj === Function && prop === 'bind') {
throw new Error('Function.prototype.bind is disabled');
}
if (obj === Function && (prop === 'call' || prop === 'apply')) {
throw new Error('Function.prototype.call and ' + 'Function.prototype.apply are disabled');
}
if (result === Function) {
return result; // Don't bind so can identify and throw later
}
return result.bind(obj); // arrow functions aren't affected by bind.
}
return result;
Expand All @@ -1334,19 +1329,16 @@
evalCallExpression(ast, subs) {
const args = ast.arguments.map(arg => SafeEval.evalAst(arg, subs));
const func = SafeEval.evalAst(ast.callee, subs);
if (func === Function) {
throw new Error('Function constructor is disabled');
}
// if (func === Function) {
// throw new Error('Function constructor is disabled');
// }
return func(...args);
},
evalAssignmentExpression(ast, subs) {
if (ast.left.type !== 'Identifier') {
throw SyntaxError('Invalid left-hand side in assignment');
}
const id = ast.left.name;
if (id === '__proto__') {
throw new Error('Assignment to __proto__ is disabled');
}
const value = SafeEval.evalAst(ast.right, subs);
subs[id] = value;
return subs[id];
Expand All @@ -1371,9 +1363,8 @@
* @returns {EvaluatedResult} Result of evaluated code
*/
runInNewContext(context) {
const keyMap = {
...context
};
// `Object.create(null)` creates a prototypeless object
const keyMap = Object.assign(Object.create(null), context);
return SafeEval.evalAst(this.ast, keyMap);
}
}
Expand Down
2 changes: 1 addition & 1 deletion dist/index-browser-umd.min.cjs
View file

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion dist/index-browser-umd.min.cjs.map
View file

Large diffs are not rendered by default.

35 changes: 13 additions & 22 deletions dist/index-node-cjs.cjs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1201,6 +1201,7 @@ const plugin = {

// register plugins
jsep.plugins.register(index, plugin);
const BLOCKED_PROTO_PROPERTIES = new Set(['constructor', '__proto__', '__defineGetter__', '__defineSetter__', '__lookupGetter__', '__lookupSetter__']);
const SafeEval = {
/**
* @param {jsep.Expression} ast
Expand Down Expand Up @@ -1283,7 +1284,7 @@ const SafeEval = {
return SafeEval.evalAst(ast.alternate, subs);
},
evalIdentifier(ast, subs) {
if (ast.name in subs) {
if (Object.hasOwn(subs, ast.name)) {
return subs[ast.name];
}
throw ReferenceError(`${ast.name} is not defined`);
Expand All @@ -1292,23 +1293,17 @@ const SafeEval = {
return ast.value;
},
evalMemberExpression(ast, subs) {
if (ast.property.type === 'Identifier' && ast.property.name === 'constructor' || ast.object.type === 'Identifier' && ast.object.name === 'constructor') {
throw new Error("'constructor' property is disabled");
}
const prop = ast.computed ? SafeEval.evalAst(ast.property) // `object[property]`
: ast.property.name; // `object.property` property is Identifier
const obj = SafeEval.evalAst(ast.object, subs);
if (obj === undefined || obj === null) {
throw TypeError(`Cannot read properties of ${obj} (reading '${prop}')`);
}
if (!Object.hasOwn(obj, prop) && BLOCKED_PROTO_PROPERTIES.has(prop)) {
throw TypeError(`Cannot read properties of ${obj} (reading '${prop}')`);
}
const result = obj[prop];
if (typeof result === 'function') {
if (obj === Function && prop === 'bind') {
throw new Error('Function.prototype.bind is disabled');
}
if (obj === Function && (prop === 'call' || prop === 'apply')) {
throw new Error('Function.prototype.call and ' + 'Function.prototype.apply are disabled');
}
if (result === Function) {
return result; // Don't bind so can identify and throw later
}
return result.bind(obj); // arrow functions aren't affected by bind.
}
return result;
Expand All @@ -1329,19 +1324,16 @@ const SafeEval = {
evalCallExpression(ast, subs) {
const args = ast.arguments.map(arg => SafeEval.evalAst(arg, subs));
const func = SafeEval.evalAst(ast.callee, subs);
if (func === Function) {
throw new Error('Function constructor is disabled');
}
// if (func === Function) {
// throw new Error('Function constructor is disabled');
// }
return func(...args);
},
evalAssignmentExpression(ast, subs) {
if (ast.left.type !== 'Identifier') {
throw SyntaxError('Invalid left-hand side in assignment');
}
const id = ast.left.name;
if (id === '__proto__') {
throw new Error('Assignment to __proto__ is disabled');
}
const value = SafeEval.evalAst(ast.right, subs);
subs[id] = value;
return subs[id];
Expand All @@ -1366,9 +1358,8 @@ class SafeScript {
* @returns {EvaluatedResult} Result of evaluated code
*/
runInNewContext(context) {
const keyMap = {
...context
};
// `Object.create(null)` creates a prototypeless object
const keyMap = Object.assign(Object.create(null), context);
return SafeEval.evalAst(this.ast, keyMap);
}
}
Expand Down
Loading

0 comments on commit 17eadab

Please sign in to comment.