Skip to content

Commit 39ee184

Browse files
committed
fix(eval): improve security of safe-eval
* 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
1 parent 93612a3 commit 39ee184

File tree

3 files changed

+188
-124
lines changed

3 files changed

+188
-124
lines changed

badges/licenses-badge-dev.svg

Lines changed: 1 addition & 1 deletion
Loading

src/Safe-Script.js

Lines changed: 24 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,13 @@ jsep.addUnaryOp('typeof');
99
jsep.addLiteral('null', null);
1010
jsep.addLiteral('undefined', undefined);
1111

12+
const BLOCKED_PROTO_PROPERTIES = new Set([
13+
'constructor',
14+
'__proto__',
15+
'__defineGetter__',
16+
'__defineSetter__'
17+
]);
18+
1219
const SafeEval = {
1320
/**
1421
* @param {jsep.Expression} ast
@@ -66,10 +73,7 @@ const SafeEval = {
6673
'*': (a, b) => a * b(),
6774
'/': (a, b) => a / b(),
6875
'%': (a, b) => a % b()
69-
}[ast.operator](
70-
SafeEval.evalAst(ast.left, subs),
71-
() => SafeEval.evalAst(ast.right, subs)
72-
);
76+
}[ast.operator](SafeEval.evalAst(ast.left, subs), () => SafeEval.evalAst(ast.right, subs));
7377
return result;
7478
},
7579
evalCompound (ast, subs) {
@@ -99,7 +103,7 @@ const SafeEval = {
99103
return SafeEval.evalAst(ast.alternate, subs);
100104
},
101105
evalIdentifier (ast, subs) {
102-
if (ast.name in subs) {
106+
if (Object.hasOwn(subs, ast.name)) {
103107
return subs[ast.name];
104108
}
105109
throw ReferenceError(`${ast.name} is not defined`);
@@ -108,33 +112,22 @@ const SafeEval = {
108112
return ast.value;
109113
},
110114
evalMemberExpression (ast, subs) {
111-
if (
112-
(ast.property.type === 'Identifier' &&
113-
ast.property.name === 'constructor') ||
114-
(ast.object.type === 'Identifier' &&
115-
ast.object.name === 'constructor')
116-
) {
117-
throw new Error("'constructor' property is disabled");
118-
}
119-
120115
const prop = ast.computed
121116
? SafeEval.evalAst(ast.property) // `object[property]`
122117
: ast.property.name; // `object.property` property is Identifier
123118
const obj = SafeEval.evalAst(ast.object, subs);
119+
if (obj === undefined || obj === null) {
120+
throw TypeError(
121+
`Cannot read properties of ${obj} (reading '${prop}')`
122+
);
123+
}
124+
if (!Object.hasOwn(obj, prop) && BLOCKED_PROTO_PROPERTIES.has(prop)) {
125+
throw TypeError(
126+
`Cannot read properties of ${obj} (reading '${prop}')`
127+
);
128+
}
124129
const result = obj[prop];
125130
if (typeof result === 'function') {
126-
if (obj === Function && prop === 'bind') {
127-
throw new Error('Function.prototype.bind is disabled');
128-
}
129-
if (obj === Function && (prop === 'call' || prop === 'apply')) {
130-
throw new Error(
131-
'Function.prototype.call and ' +
132-
'Function.prototype.apply are disabled'
133-
);
134-
}
135-
if (result === Function) {
136-
return result; // Don't bind so can identify and throw later
137-
}
138131
return result.bind(obj); // arrow functions aren't affected by bind.
139132
}
140133
return result;
@@ -156,19 +149,16 @@ const SafeEval = {
156149
evalCallExpression (ast, subs) {
157150
const args = ast.arguments.map((arg) => SafeEval.evalAst(arg, subs));
158151
const func = SafeEval.evalAst(ast.callee, subs);
159-
if (func === Function) {
160-
throw new Error('Function constructor is disabled');
161-
}
152+
// if (func === Function) {
153+
// throw new Error('Function constructor is disabled');
154+
// }
162155
return func(...args);
163156
},
164157
evalAssignmentExpression (ast, subs) {
165158
if (ast.left.type !== 'Identifier') {
166159
throw SyntaxError('Invalid left-hand side in assignment');
167160
}
168161
const id = ast.left.name;
169-
if (id === '__proto__') {
170-
throw new Error('Assignment to __proto__ is disabled');
171-
}
172162
const value = SafeEval.evalAst(ast.right, subs);
173163
subs[id] = value;
174164
return subs[id];
@@ -193,7 +183,8 @@ class SafeScript {
193183
* @returns {EvaluatedResult} Result of evaluated code
194184
*/
195185
runInNewContext (context) {
196-
const keyMap = {...context};
186+
// `Object.create(null)` creates a prototypeless object
187+
const keyMap = Object.assign(Object.create(null), context);
197188
return SafeEval.evalAst(this.ast, keyMap);
198189
}
199190
}

0 commit comments

Comments
 (0)