Skip to content

Commit

Permalink
Fix Security Vulnerability by using hasOwnProperty defensively
Browse files Browse the repository at this point in the history
  • Loading branch information
edi9999 committed Jan 29, 2021
1 parent 171f699 commit 07edb62
Show file tree
Hide file tree
Showing 3 changed files with 90 additions and 48 deletions.
6 changes: 5 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
### 1.1.2

- Disallow access to prototype chain (CVE-2021-21277)

### 1.1.1

Previous version was published with ES6 feature, now the published JS uses ES5 only
Expand All @@ -18,7 +22,7 @@ function validChars(ch) {
}
evaluate = compile("être_embarassé", {
isIdentifierStart: validChars,
isIdentifierContinue: validChars
isIdentifierContinue: validChars,
});

evaluate({ être_embarassé: "Ping" });
Expand Down
101 changes: 57 additions & 44 deletions lib/parse.js
Original file line number Diff line number Diff line change
Expand Up @@ -1694,44 +1694,6 @@ var isAutoBootstrapAllowed = allowAutoBootstrap(window.document);
</file>
</example>
*/
function angularInit(element, bootstrap) {
var appElement,
module,
config = {};

// The element `element` has priority over any other element.
forEach(ngAttrPrefixes, function (prefix) {
var name = prefix + "app";

if (!appElement && element.hasAttribute && element.hasAttribute(name)) {
appElement = element;
module = element.getAttribute(name);
}
});
forEach(ngAttrPrefixes, function (prefix) {
var name = prefix + "app";
var candidate;

if (
!appElement &&
(candidate = element.querySelector("[" + name.replace(":", "\\:") + "]"))
) {
appElement = candidate;
module = candidate.getAttribute(name);
}
});
if (appElement) {
if (!isAutoBootstrapAllowed) {
window.console.error(
"Angular: disabling automatic bootstrap. <script> protocol indicates " +
"an extension, document.location.href does not match."
);
return;
}
config.strictDi = getNgAttribute(appElement, "strict-di") !== null;
bootstrap(appElement, module ? [module] : [], config);
}
}

/**
* @ngdoc function
Expand Down Expand Up @@ -3232,11 +3194,12 @@ ASTCompiler.prototype = {
intoId = intoId || this.nextId();
this.if_(
"i",
this.lazyAssign(intoId, this.computedMember("i", ast.watchId)),
this.lazyAssign(intoId, this.unsafeComputedMember("i", ast.watchId)),
this.lazyRecurse(ast, intoId, nameId, recursionFn, create, true)
);
return;
}

switch (ast.type) {
case AST.Program:
forEach(ast.body, function (expression, pos) {
Expand Down Expand Up @@ -3356,11 +3319,20 @@ ASTCompiler.prototype = {
undefined,
function () {
var member = null;
const inAssignment = self.current().inAssignment;
if (ast.computed) {
right = self.nextId();
member = self.computedMember(left, right);
if (inAssignment || self.state.computing === "assign") {
member = self.unsafeComputedMember(left, right);
} else {
member = self.computedMember(left, right);
}
} else {
member = self.nonComputedMember(left, ast.property.name);
if (inAssignment || self.state.computing === "assign") {
member = self.unsafeNonComputedMember(left, ast.property.name);
} else {
member = self.nonComputedMember(left, ast.property.name);
}
right = ast.property.name;
}

Expand Down Expand Up @@ -3447,7 +3419,13 @@ ASTCompiler.prototype = {
if (left.name) {
var x = self.member(left.context, left.name, left.computed);
expression =
x + ".call(" + [left.context].concat(args).join(",") + ")";
"(" +
x +
" === null ? null : " +
self.unsafeMember(left.context, left.name, left.computed) +
".call(" +
[left.context].concat(args).join(",") +
"))";
} else {
expression = right + "(" + args.join(",") + ")";
}
Expand All @@ -3464,6 +3442,7 @@ ASTCompiler.prototype = {
case AST.AssignmentExpression:
right = this.nextId();
left = {};
self.current().inAssignment = true;
this.recurse(
ast.left,
undefined,
Expand All @@ -3489,9 +3468,13 @@ ASTCompiler.prototype = {
recursionFn(intoId || expression);
}
);
self.current().inAssignment = false;
self.recurse(ast.right, right);
self.current().inAssignment = true;
},
1
);
self.current().inAssignment = false;
break;
case AST.ArrayExpression:
args = [];
Expand Down Expand Up @@ -3532,7 +3515,10 @@ ASTCompiler.prototype = {
}
right = self.nextId();
self.recurse(property.value, right);
self.assign(self.member(intoId, left, property.computed), right);
self.assign(
self.unsafeMember(intoId, left, property.computed),
right
);
});
} else {
forEach(ast.properties, function (property) {
Expand Down Expand Up @@ -3666,9 +3652,35 @@ ASTCompiler.prototype = {
return expr;
},

computedMember: function (left, right) {
unsafeComputedMember: function (left, right) {
return left + "[" + right + "]";
},
unsafeNonComputedMember: function (left, right) {
return this.nonComputedMember(left, right);
},

computedMember: function (left, right) {
if (this.state.computing === "assign") {
return this.unsafeComputedMember(left, right);
}
// return left + "[" + right + "]";
return (
"(" +
left +
".hasOwnProperty(" +
right +
") ? " +
left +
"[" +
right +
"] : null)"
);
},

unsafeMember: function (left, right, computed) {
if (computed) return this.unsafeComputedMember(left, right);
return this.unsafeNonComputedMember(left, right);
},

member: function (left, right, computed) {
if (computed) return this.computedMember(left, right);
Expand Down Expand Up @@ -3827,6 +3839,7 @@ ASTInterpreter.prototype = {
right = ast.property.name;
}
if (ast.computed) right = this.recurse(ast.property);

return ast.computed
? this.computedMember(left, right, context, create)
: this.nonComputedMember(left, right, context, create);
Expand Down
31 changes: 28 additions & 3 deletions test/main.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,32 @@ describe("expressions", function () {
});
});

describe("Security", function () {
it("should not leak", function () {
evaluate = compile(
"''['c'+'onstructor']['c'+'onstructor']('return process;')()"
);
const result = evaluate({});
expect(result).to.equal(undefined);
});

it("should not leak indirectly with string concatenation", function () {
evaluate = compile(
"a = null; a = ''['c'+'onstructor']['c'+'onstructor']; a = a('return process;'); a();"
);
const result = evaluate({});
expect(result).to.equal(undefined);
});

it("should not leak indirectly with literal string", function () {
evaluate = compile(
"a = null; a = ''['constructor']['constructor']; a = a('return process;'); a();"
);
const result = evaluate({});
expect(result).to.equal(undefined);
});
});

describe("when evaluating dot-notated assignments", function () {
it("should set the new value on scope", function () {
evaluate = compile("island.pirate.name = 'Störtebeker'");
Expand Down Expand Up @@ -468,7 +494,7 @@ describe("expressions", function () {

it("should not leak with computed prop", function () {
evaluate = compile("a['split']");
expect(evaluate({ a: "" })).to.eql(undefined);
expect(evaluate({ a: "" })).to.eql(null);
});

it("should allow to read string length", function () {
Expand All @@ -487,8 +513,7 @@ describe("expressions", function () {
// evaluate(scope);
// expect(scope.name.split).to.be.a("function");
// });
//
//

it("should work with __proto__", function () {
evaluate = compile("__proto__");
expect(evaluate({})).to.eql(undefined);
Expand Down

0 comments on commit 07edb62

Please sign in to comment.