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

Commit 756640f

Browse files
chirayukchirayuk
committed
fix($parse): add quick check for Function constructor in fast path
1 parent d87b791 commit 756640f

File tree

2 files changed

+79
-34
lines changed

2 files changed

+79
-34
lines changed

‎src/ng/parse.js

+61-33
Original file line numberDiff line numberDiff line change
@@ -878,6 +878,10 @@ function setter(obj, path, setValue, fullExp, options) {
878878

879879
var getterFnCache = {};
880880

881+
function isPossiblyDangerousMemberName(name) {
882+
return name == 'constructor';
883+
}
884+
881885
/**
882886
* Implementation of the "Black Hole" variant from:
883887
* - http://jsperf.com/angularjs-parse-getter/4
@@ -889,29 +893,37 @@ function cspSafeGetterFn(key0, key1, key2, key3, key4, fullExp, options) {
889893
ensureSafeMemberName(key2, fullExp);
890894
ensureSafeMemberName(key3, fullExp);
891895
ensureSafeMemberName(key4, fullExp);
896+
var eso = function(o) {
897+
return ensureSafeObject(o, fullExp);
898+
};
899+
var eso0 = isPossiblyDangerousMemberName(key0) ? eso : identity;
900+
var eso1 = isPossiblyDangerousMemberName(key1) ? eso : identity;
901+
var eso2 = isPossiblyDangerousMemberName(key2) ? eso : identity;
902+
var eso3 = isPossiblyDangerousMemberName(key3) ? eso : identity;
903+
var eso4 = isPossiblyDangerousMemberName(key4) ? eso : identity;
892904

893905
return !options.unwrapPromises
894906
? function cspSafeGetter(scope, locals) {
895907
var pathVal = (locals && locals.hasOwnProperty(key0)) ? locals : scope;
896908

897909
if (pathVal == null) return pathVal;
898-
pathVal = pathVal[key0];
910+
pathVal = eso0(pathVal[key0]);
899911

900912
if (!key1) return pathVal;
901913
if (pathVal == null) return undefined;
902-
pathVal = pathVal[key1];
914+
pathVal = eso1(pathVal[key1]);
903915

904916
if (!key2) return pathVal;
905917
if (pathVal == null) return undefined;
906-
pathVal = pathVal[key2];
918+
pathVal = eso2(pathVal[key2]);
907919

908920
if (!key3) return pathVal;
909921
if (pathVal == null) return undefined;
910-
pathVal = pathVal[key3];
922+
pathVal = eso3(pathVal[key3]);
911923

912924
if (!key4) return pathVal;
913925
if (pathVal == null) return undefined;
914-
pathVal = pathVal[key4];
926+
pathVal = eso4(pathVal[key4]);
915927

916928
return pathVal;
917929
}
@@ -921,72 +933,78 @@ function cspSafeGetterFn(key0, key1, key2, key3, key4, fullExp, options) {
921933

922934
if (pathVal == null) return pathVal;
923935

924-
pathVal = pathVal[key0];
936+
pathVal = eso0(pathVal[key0]);
925937
if (pathVal && pathVal.then) {
926938
promiseWarning(fullExp);
927939
if (!("$$v" in pathVal)) {
928940
promise = pathVal;
929941
promise.$$v = undefined;
930-
promise.then(function(val) { promise.$$v = val; });
942+
promise.then(function(val) { promise.$$v = eso0(val); });
931943
}
932-
pathVal = pathVal.$$v;
944+
pathVal = eso0(pathVal.$$v);
933945
}
934946

935947
if (!key1) return pathVal;
936948
if (pathVal == null) return undefined;
937-
pathVal = pathVal[key1];
949+
pathVal = eso1(pathVal[key1]);
938950
if (pathVal && pathVal.then) {
939951
promiseWarning(fullExp);
940952
if (!("$$v" in pathVal)) {
941953
promise = pathVal;
942954
promise.$$v = undefined;
943-
promise.then(function(val) { promise.$$v = val; });
955+
promise.then(function(val) { promise.$$v = eso1(val); });
944956
}
945-
pathVal = pathVal.$$v;
957+
pathVal = eso1(pathVal.$$v);
946958
}
947959

948960
if (!key2) return pathVal;
949961
if (pathVal == null) return undefined;
950-
pathVal = pathVal[key2];
962+
pathVal = eso2(pathVal[key2]);
951963
if (pathVal && pathVal.then) {
952964
promiseWarning(fullExp);
953965
if (!("$$v" in pathVal)) {
954966
promise = pathVal;
955967
promise.$$v = undefined;
956-
promise.then(function(val) { promise.$$v = val; });
968+
promise.then(function(val) { promise.$$v = eso2(val); });
957969
}
958-
pathVal = pathVal.$$v;
970+
pathVal = eso2(pathVal.$$v);
959971
}
960972

961973
if (!key3) return pathVal;
962974
if (pathVal == null) return undefined;
963-
pathVal = pathVal[key3];
975+
pathVal = eso3(pathVal[key3]);
964976
if (pathVal && pathVal.then) {
965977
promiseWarning(fullExp);
966978
if (!("$$v" in pathVal)) {
967979
promise = pathVal;
968980
promise.$$v = undefined;
969-
promise.then(function(val) { promise.$$v = val; });
981+
promise.then(function(val) { promise.$$v = eso3(val); });
970982
}
971-
pathVal = pathVal.$$v;
983+
pathVal = eso3(pathVal.$$v);
972984
}
973985

974986
if (!key4) return pathVal;
975987
if (pathVal == null) return undefined;
976-
pathVal = pathVal[key4];
988+
pathVal = eso4(pathVal[key4]);
977989
if (pathVal && pathVal.then) {
978990
promiseWarning(fullExp);
979991
if (!("$$v" in pathVal)) {
980992
promise = pathVal;
981993
promise.$$v = undefined;
982-
promise.then(function(val) { promise.$$v = val; });
994+
promise.then(function(val) { promise.$$v = eso4(val); });
983995
}
984-
pathVal = pathVal.$$v;
996+
pathVal = eso4(pathVal.$$v);
985997
}
986998
return pathVal;
987999
};
9881000
}
9891001

1002+
function getterFnWithExtraArgs(fn, fullExpression) {
1003+
return function(s, l) {
1004+
return fn(s, l, promiseWarning, ensureSafeObject, fullExpression);
1005+
};
1006+
}
1007+
9901008
function getterFn(path, options, fullExp) {
9911009
// Check whether the cache has this getter already.
9921010
// We can use hasOwnProperty directly on the cache because we ensure,
@@ -1019,35 +1037,45 @@ function getterFn(path, options, fullExp) {
10191037
}
10201038
} else {
10211039
var code = 'var p;\n';
1040+
var needsEnsureSafeObject = false;
10221041
forEach(pathKeys, function(key, index) {
10231042
ensureSafeMemberName(key, fullExp);
1024-
code += 'if(s == null) return undefined;\n' +
1025-
's='+ (index
1043+
var lookupJs = (index
10261044
// we simply dereference 's' on any .dot notation
10271045
? 's'
10281046
// but if we are first then we check locals first, and if so read it first
1029-
: '((k&&k.hasOwnProperty("' + key + '"))?k:s)') + '["' + key + '"]' + ';\n' +
1030-
(options.unwrapPromises
1031-
? 'if (s && s.then) {\n' +
1047+
: '((l&&l.hasOwnProperty("' + key + '"))?l:s)') + '["' + key + '"]';
1048+
var wrapWithEso = isPossiblyDangerousMemberName(key);
1049+
if (wrapWithEso) {
1050+
lookupJs = 'eso(' + lookupJs + ', fe)';
1051+
needsEnsureSafeObject = true;
1052+
}
1053+
code += 'if(s == null) return undefined;\n' +
1054+
's=' + lookupJs + ';\n';
1055+
if (options.unwrapPromises) {
1056+
code += 'if (s && s.then) {\n' +
10321057
' pw("' + fullExp.replace(/(["\r\n])/g, '\\$1') + '");\n' +
10331058
' if (!("$$v" in s)) {\n' +
10341059
' p=s;\n' +
10351060
' p.$$v = undefined;\n' +
1036-
' p.then(function(v) {p.$$v=v;});\n' +
1061+
' p.then(function(v) {p.$$v=' + (wrapWithEso ? 'eso(v)' : 'v') + ';});\n' +
10371062
'}\n' +
1038-
' s=s.$$v\n' +
1039-
'}\n'
1040-
: '');
1063+
' s=' + (wrapWithEso ? 'eso(s.$$v)' : 's.$$v') + '\n' +
1064+
'}\n';
1065+
1066+
}
10411067
});
10421068
code += 'return s;';
10431069

10441070
/* jshint -W054 */
1045-
var evaledFnGetter = new Function('s', 'k', 'pw', code); // s=scope, k=locals, pw=promiseWarning
1071+
// s=scope, l=locals, pw=promiseWarning, eso=ensureSafeObject, fe=fullExpression
1072+
var evaledFnGetter = new Function('s', 'l', 'pw', 'eso', 'fe', code);
10461073
/* jshint +W054 */
10471074
evaledFnGetter.toString = valueFn(code);
1048-
fn = options.unwrapPromises ? function(scope, locals) {
1049-
return evaledFnGetter(scope, locals, promiseWarning);
1050-
} : evaledFnGetter;
1075+
if (needsEnsureSafeObject || options.unwrapPromises) {
1076+
evaledFnGetter = getterFnWithExtraArgs(evaledFnGetter, fullExp);
1077+
}
1078+
fn = evaledFnGetter;
10511079
}
10521080

10531081
// Only cache the value if it's not going to mess up the cache object

‎test/ng/parseSpec.js

+18-1
Original file line numberDiff line numberDiff line change
@@ -649,9 +649,26 @@ describe('parser', function() {
649649
expect(scope.$eval('a + \n b.c + \r "\td" + \t \r\n\r "\r\n\n"')).toEqual("abc\td\r\n\n");
650650
});
651651

652-
653652
describe('sandboxing', function() {
654653
describe('Function constructor', function() {
654+
it('should not tranverse the Function constructor in the getter', function() {
655+
expect(function() {
656+
scope.$eval('{}.toString.constructor');
657+
}).toThrowMinErr(
658+
'$parse', 'isecfn', 'Referencing Function in Angular expressions is disallowed! ' +
659+
'Expression: {}.toString.constructor');
660+
661+
});
662+
663+
it('should not allow access to the Function prototype in the getter', function() {
664+
expect(function() {
665+
scope.$eval('toString.constructor.prototype');
666+
}).toThrowMinErr(
667+
'$parse', 'isecfn', 'Referencing Function in Angular expressions is disallowed! ' +
668+
'Expression: toString.constructor.prototype');
669+
670+
});
671+
655672
it('should NOT allow access to Function constructor in getter', function() {
656673

657674
expect(function() {

0 commit comments

Comments
 (0)