Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

Commit 89ca859

Browse files
thejhIgorMinar
authored andcommitted
fix($parse): forbid __{define,lookup}{Getter,Setter}__ properties
It was possible to use `{}.__defineGetter__.call(null, 'alert', (0).valueOf.bind(0))` to set `window.alert` to a false-ish value, thereby breaking the `isWindow` check, which might lead to arbitrary code execution in browsers that let you obtain the window object using Array methods. Prevent that by blacklisting the nasty __{define,lookup}{Getter,Setter}__ properties. BREAKING CHANGE: This prevents the use of __{define,lookup}{Getter,Setter}__ inside angular expressions. If you really need them for some reason, please wrap/bind them to make them less dangerous, then make them available through the scope object.
1 parent bc6fb7c commit 89ca859

File tree

2 files changed

+85
-0
lines changed

2 files changed

+85
-0
lines changed

src/ng/parse.js

+10
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,11 @@ function ensureSafeMemberName(name, fullExpression) {
3838
throw $parseMinErr('isecfld',
3939
'Referencing "constructor" field in Angular expressions is disallowed! Expression: {0}',
4040
fullExpression);
41+
} else if (name === "__defineGetter__" || name === "__defineSetter__"
42+
|| name === "__lookupGetter__" || name === "__lookupSetter__") {
43+
throw $parseMinErr('isecgetset',
44+
'Defining and looking up getters and setters in Angular expressions is disallowed! '
45+
+'Expression: {0}', fullExpression);
4146
}
4247
return name;
4348
}
@@ -64,6 +69,11 @@ function ensureSafeObject(obj, fullExpression) {
6469
throw $parseMinErr('isecobj',
6570
'Referencing Object in Angular expressions is disallowed! Expression: {0}',
6671
fullExpression);
72+
} else if (obj === ({}).__defineGetter__ || obj === ({}).__defineSetter__
73+
|| obj === ({}).__lookupGetter__ || obj === ({}).__lookupSetter__) {
74+
throw $parseMinErr('isecgetset',
75+
'Defining and looking up getters and setters in Angular expressions is disallowed! '
76+
+'Expression: {0}', fullExpression);
6777
}
6878
}
6979
return obj;

test/ng/parseSpec.js

+75
Original file line numberDiff line numberDiff line change
@@ -1032,6 +1032,81 @@ describe('parser', function() {
10321032
}));
10331033
});
10341034

1035+
1036+
describe('getters and setters', function() {
1037+
it('should NOT allow invocation of __defineGetter__', function() {
1038+
expect(function() {
1039+
scope.$eval('{}.__defineGetter__("a", "".charAt)');
1040+
}).toThrowMinErr(
1041+
'$parse', 'isecgetset', 'Defining and looking up getters and setters in '+
1042+
'Angular expressions is disallowed! Expression: '+
1043+
'{}.__defineGetter__("a", "".charAt)');
1044+
1045+
expect(function() {
1046+
scope.$eval('{}.__defineGetter__.call({}, "a", "".charAt)');
1047+
}).toThrowMinErr(
1048+
'$parse', 'isecgetset', 'Defining and looking up getters and setters in '+
1049+
'Angular expressions is disallowed! Expression: '+
1050+
'{}.__defineGetter__.call({}, "a", "".charAt)');
1051+
1052+
expect(function() {
1053+
scope.$eval('{}["__defineGetter__"].call({}, "a", "".charAt)');
1054+
}).toThrowMinErr(
1055+
'$parse', 'isecgetset', 'Defining and looking up getters and setters in '+
1056+
'Angular expressions is disallowed! Expression: '+
1057+
'{}["__defineGetter__"].call({}, "a", "".charAt)');
1058+
});
1059+
1060+
it('should NOT allow invocation of __defineSetter__', function() {
1061+
expect(function() {
1062+
scope.$eval('{}.__defineSetter__("a", "".charAt)');
1063+
}).toThrowMinErr(
1064+
'$parse', 'isecgetset', 'Defining and looking up getters and setters in '+
1065+
'Angular expressions is disallowed! Expression: '+
1066+
'{}.__defineSetter__("a", "".charAt)');
1067+
1068+
expect(function() {
1069+
scope.$eval('{}.__defineSetter__.call({}, "a", "".charAt)');
1070+
}).toThrowMinErr(
1071+
'$parse', 'isecgetset', 'Defining and looking up getters and setters in '+
1072+
'Angular expressions is disallowed! Expression: '+
1073+
'{}.__defineSetter__.call({}, "a", "".charAt)');
1074+
});
1075+
1076+
it('should NOT allow invocation of __lookupGetter__', function() {
1077+
expect(function() {
1078+
scope.$eval('{}.__lookupGetter__("a")');
1079+
}).toThrowMinErr(
1080+
'$parse', 'isecgetset', 'Defining and looking up getters and setters in '+
1081+
'Angular expressions is disallowed! Expression: '+
1082+
'{}.__lookupGetter__("a")');
1083+
1084+
expect(function() {
1085+
scope.$eval('{}.__lookupGetter__.call({}, "a")');
1086+
}).toThrowMinErr(
1087+
'$parse', 'isecgetset', 'Defining and looking up getters and setters in '+
1088+
'Angular expressions is disallowed! Expression: '+
1089+
'{}.__lookupGetter__.call({}, "a")');
1090+
});
1091+
1092+
it('should NOT allow invocation of __lookupSetter__', function() {
1093+
expect(function() {
1094+
scope.$eval('{}.__lookupSetter__("a")');
1095+
}).toThrowMinErr(
1096+
'$parse', 'isecgetset', 'Defining and looking up getters and setters in '+
1097+
'Angular expressions is disallowed! Expression: '+
1098+
'{}.__lookupSetter__("a")');
1099+
1100+
expect(function() {
1101+
scope.$eval('{}.__lookupSetter__.call({}, "a")');
1102+
}).toThrowMinErr(
1103+
'$parse', 'isecgetset', 'Defining and looking up getters and setters in '+
1104+
'Angular expressions is disallowed! Expression: '+
1105+
'{}.__lookupSetter__.call({}, "a")');
1106+
});
1107+
});
1108+
1109+
10351110
describe('constant', function() {
10361111
it('should mark scalar value expressions as constant', inject(function($parse) {
10371112
expect($parse('12.3').constant).toBe(true);

0 commit comments

Comments
 (0)