diff options
| author | Chirayu Krishnappa | 2013-06-21 13:03:56 -0700 |
|---|---|---|
| committer | Chirayu Krishnappa | 2013-06-21 17:37:44 -0700 |
| commit | 39841f2ec9b17b3b2920fd1eb548d444251f4f56 (patch) | |
| tree | 0776f7918d3b3bfbab22507a2ace7dc5fa43bc1c | |
| parent | 1adf29af13890d61286840177607edd552a9df97 (diff) | |
| download | angular.js-39841f2ec9b17b3b2920fd1eb548d444251f4f56.tar.bz2 | |
fix($compile): disallow interpolations for DOM event handlers
BREAKING CHANGE: Interpolations inside DOM event handlers are
disallowed. DOM event handlers execute arbitrary Javascript code.
Using an interpolation for such handlers means that the interpolated
value is a JS string that is evaluated. Storing or generating such
strings is error prone and likely leads to an XSS if you're not
super careful. On the other hand, ng-click and such event handlers
evaluate Angular expressions that are a lot safer (e.g. No direct
access to global objects - only scope), cleaner and harder to
exploit.
To migrate the code follow the example below:
Before:
JS: scope.foo = 'alert(1)';
HTML: <div onclick="{{foo}}">
After:
JS: scope.foo = function() { alert(1); }
HTML: <div ng-click="foo()">
| -rw-r--r-- | src/ng/compile.js | 10 | ||||
| -rwxr-xr-x | test/ng/compileSpec.js | 30 |
2 files changed, 40 insertions, 0 deletions
diff --git a/src/ng/compile.js b/src/ng/compile.js index d85af28c..91844f45 100644 --- a/src/ng/compile.js +++ b/src/ng/compile.js @@ -155,6 +155,10 @@ function $CompileProvider($provide) { CLASS_DIRECTIVE_REGEXP = /(([\d\w\-_]+)(?:\:([^;]+))?;?)/, urlSanitizationWhitelist = /^\s*(https?|ftp|mailto|file):/; + // Ref: http://developers.whatwg.org/webappapis.html#event-handler-idl-attributes + // The assumption is that future DOM event attribute names will begin with + // 'on' and be composed of only English letters. + var EVENT_HANDLER_ATTR_REGEXP = /^(on[a-z]*|formaction)$/; /** * @ngdoc function @@ -1165,6 +1169,12 @@ function $CompileProvider($provide) { compile: valueFn(function attrInterpolateLinkFn(scope, element, attr) { var $$observers = (attr.$$observers || (attr.$$observers = {})); + if (EVENT_HANDLER_ATTR_REGEXP.test(name)) { + throw $compileMinErr('nodomevents', + "Interpolations for HTML DOM event attributes are disallowed. Please use the ng- " + + "versions (such as ng-click instead of onclick) instead."); + } + // we need to interpolate again, in case the attribute value has been updated // (e.g. by another directive's compile function) interpolateFn = $interpolate(attr[name], true); diff --git a/test/ng/compileSpec.js b/test/ng/compileSpec.js index b713476b..672a704d 100755 --- a/test/ng/compileSpec.js +++ b/test/ng/compileSpec.js @@ -2831,6 +2831,36 @@ describe('$compile', function() { }); }); + describe('interpolation on HTML DOM event handler attributes onclick, onXYZ, formaction', function() { + it('should disallow interpolation on onclick', inject(function($compile, $rootScope) { + // All interpolations are disallowed. + $rootScope.onClickJs = ""; + expect(function() { + $compile('<button onclick="{{onClickJs}}"></script>')($rootScope); + }).toThrow( + "[$compile:nodomevents] Interpolations for HTML DOM event attributes are disallowed. " + + "Please use the ng- versions (such as ng-click instead of onclick) instead."); + expect(function() { + $compile('<button ONCLICK="{{onClickJs}}"></script>')($rootScope); + }).toThrow( + "[$compile:nodomevents] Interpolations for HTML DOM event attributes are disallowed. " + + "Please use the ng- versions (such as ng-click instead of onclick) instead."); + expect(function() { + $compile('<button ng-attr-onclick="{{onClickJs}}"></script>')($rootScope); + }).toThrow( + "[$compile:nodomevents] Interpolations for HTML DOM event attributes are disallowed. " + + "Please use the ng- versions (such as ng-click instead of onclick) instead."); + })); + + it('should pass through arbitrary values on onXYZ event attributes that contain a hyphen', inject(function($compile, $rootScope) { + element = $compile('<button on-click="{{onClickJs}}"></script>')($rootScope); + $rootScope.onClickJs = 'javascript:doSomething()'; + $rootScope.$apply(); + expect(element.attr('on-click')).toEqual('javascript:doSomething()'); + })); + }); + + describe('ngAttr* attribute binding', function() { it('should bind after digest but not before', inject(function($compile, $rootScope) { |
