Skip to content

Commit

Permalink
<IMPORTANT> Fix security issue reported by Isiah Meadows.
Browse files Browse the repository at this point in the history
  • Loading branch information
@doodadjs
doodadjs committed Jun 22, 2018
1 parent e895e88 commit e6b05a4
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 25 deletions.
2 changes: 1 addition & 1 deletion make.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
//
{
stage: "b",
version: "4.1.1",
version: "4.1.2",
dependencies: [
{
name: "@doodad-js/core",
Expand Down
35 changes: 20 additions & 15 deletions src/common/Tools_SafeEval.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ exports.add = function add(modules) {
deniedTokens: ['eval', 'arguments', 'this'],
constants: ['true', 'false', 'null', 'undefined', 'NaN', 'Infinity'],
allDigitsRegEx: /^([0-9]+[.]?[0-9]*([e][-+]?[0-9]+)?|0[xX]([0-9a-fA-F])+|0[bB]([01])+|0[oO]([0-7])+)$/,
newLineChars: ['\n', '\r', '\u2028', '\u2029'],
};


Expand Down Expand Up @@ -176,6 +177,13 @@ exports.add = function add(modules) {
isRegExpFlags = true;
noPrevChr = true;
};
} else if (isComment) {
if (tools.indexOf(__Internal__.newLineChars, chr.chr) >= 0) { // New line
isComment = false;
validateToken();
checkDenied();
functionArgs = [];
};
} else if (isCommentBlock) {
// Comment block
if ((prevChr === '*') && (chr.chr === '/')) {
Expand Down Expand Up @@ -203,17 +211,17 @@ exports.add = function add(modules) {
throw new types.AccessDenied("Assignment is not allowed.");
};
checkDenied();
} else if (isComment && (chr.chr !== '\n') && (chr.chr !== '\r')) {
// Statement comment
} else if ((prevChr === '/') && (chr.chr === '/')) {
// Begin statement comment
validateToken();
isComment = true;
noPrevChr = true;
} else if ((prevChr === '/') && (chr.chr === '*')) {
// Begin comment block
validateToken();
isCommentBlock = true;
} else if (isGlobal && !tokenName && (prevChr === '/')) {
noPrevChr = true;
} else if (isGlobal && !tokenName && (prevChr === '/') && (chr.chr !== '/')) {
// Begin RegExp
if (!allowRegExp) {
// For simplicity
Expand All @@ -223,25 +231,21 @@ exports.add = function add(modules) {
if (chr.chr === '\\') {
isEscape = true;
};
noPrevChr = true;
} else if (unicode.isSpace(chr.chr, curLocale)) {
// Space
validateToken();
isDot = false;
isGlobal = true;
isRegExpFlags = false;
} else if ((chr.chr === ';') || (chr.chr === '\n') || (chr.chr === '\r')) { // End of statement
if (isComment && (chr.chr === ';')) {
// ';' is part of the comment
} else {
isComment = false;
isRegExpFlags = false;
validateToken();
checkDenied();
if (!isDot || (chr.chr === ';')) {
isGlobal = true;
};
functionArgs = [];
} else if ((chr.chr === ';') || (tools.indexOf(__Internal__.newLineChars, chr.chr) >= 0)) { // End of statement
isRegExpFlags = false;
validateToken();
checkDenied();
if (!isDot || (chr.chr === ';')) {
isGlobal = true;
};
functionArgs = [];
} else if ((chr.chr === '$') || (chr.chr === '_') || unicode.isAlnum(chr.chr, curLocale)) {
if (!isRegExpFlags) {
// Token
Expand Down Expand Up @@ -279,6 +283,7 @@ exports.add = function add(modules) {
if (preventAssignment) {
throw new types.AccessDenied("Increment operators are not allowed.");
};
noPrevChr = true;
};
} else if (((chr.chr === '<') || (chr.chr === '>')) && (prevChr === chr.chr)) {
// Potential shift assignment
Expand Down
24 changes: 15 additions & 9 deletions src/test/Unit_Tools_SafeEval.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ exports.add = function add(modules) {

// Unit
priority: null,

proto: {
run: function run(root, /*optional*/options) {
"use strict";
Expand All @@ -47,12 +47,12 @@ exports.add = function add(modules) {
io = doodad.IO,
safeEval = tools.SafeEval;


if (!options) {
options = {};
};


test.runCommand(safeEval.eval, "Doodad.Tools.SafeEval.eval", function(command, options) {
const hasA = types.has(global, 'a'),
oldA = global.a,
Expand Down Expand Up @@ -120,7 +120,7 @@ exports.add = function add(modules) {
group.runStep(NaN, {}, /**/ "/hello/*/*hello*/1", null, null, {allowRegExp: true});
group.runStep(NaN, {}, /**/ "/\\//*/*hello*/1", null, null, {allowRegExp: true});
});

command.runGroup("Denied", function(group, options) {
group.runStep(types.AccessDenied, {mode: 'isinstance'}, /**/ "a=1", null, ['a']); // assignment denied
group.runStep(types.AccessDenied, {mode: 'isinstance'}, /**/ "a='hello'", null, ['a']); // assignment denied
Expand Down Expand Up @@ -151,31 +151,37 @@ exports.add = function add(modules) {

group.runStep(types.AccessDenied, {mode: 'isinstance'}, /**/ "`Hi !`"); // Templates are denied
group.runStep(types.AccessDenied, {mode: 'isinstance'}, /**/ "`Hi ${'you'} !`"); // Templates are denied

group.runStep(types.AccessDenied, {mode: 'isinstance'}, /**/ "/*comment*/eval"); // eval is denied
group.runStep(types.AccessDenied, {mode: 'isinstance'}, /**/ "eval/*comment*/"); // eval is denied
group.runStep(types.AccessDenied, {mode: 'isinstance'}, /**/ "eval//comment"); // eval is denied
group.runStep(types.AccessDenied, {mode: 'isinstance'}, /**/ "//\neval('1')"); // eval is denied
group.runStep(types.AccessDenied, {mode: 'isinstance'}, /**/ "//\reval('1')"); // eval is denied
group.runStep(types.AccessDenied, {mode: 'isinstance'}, /**/ "//\n\reval('1')"); // eval is denied
group.runStep(types.AccessDenied, {mode: 'isinstance'}, /**/ "//\u2028eval('1')"); // eval is denied
group.runStep(types.AccessDenied, {mode: 'isinstance'}, /**/ "//\u2029eval('1')"); // eval is denied
group.runStep(types.AccessDenied, {mode: 'isinstance'}, /**/ "//\u2028\u2029eval('1')"); // eval is denied

group.runStep(types.AccessDenied, {mode: 'isinstance'}, /**/ "/hello/"); // RegExp are denied
group.runStep(types.AccessDenied, {mode: 'isinstance'}, /**/ "/hello/*/*hello*/a", null, null, {allowRegExp: true}); // Access to "a" denied
group.runStep(types.AccessDenied, {mode: 'isinstance'}, /**/ "a=/hello/", null, ['a'], {allowRegExp: true}); // assignment denied
});

command.finalize(function(err, dummy) {
if (hasA) {
global.a = oldA;
} else {
delete global.a;
};

if (hasB) {
global.b = oldB;
} else {
delete global.b;
};
});
});

},
},
};
Expand Down

0 comments on commit e6b05a4

Please sign in to comment.