From 1818056912016a9932577f06d6cf4ac627b16027 Mon Sep 17 00:00:00 2001
From: Victor Berchet <victor@suumit.com>
Date: Thu, 25 Aug 2016 11:13:44 -0700
Subject: [PATCH] fix(TemplateParser): disallow event-property binding even
 with the NO_ERRORS_SCHEMA

closes #11026
---
 .../src/template_parser/template_parser.ts    | 16 ++++---
 .../test/linker/security_integration_spec.ts  | 45 +++++++++++++------
 2 files changed, 42 insertions(+), 19 deletions(-)

diff --git a/modules/@angular/compiler/src/template_parser/template_parser.ts b/modules/@angular/compiler/src/template_parser/template_parser.ts
index c7974a7e18..a21a1026ca 100644
--- a/modules/@angular/compiler/src/template_parser/template_parser.ts
+++ b/modules/@angular/compiler/src/template_parser/template_parser.ts
@@ -854,6 +854,7 @@ class TemplateParseVisitor implements html.Visitor {
         boundPropertyName = this._schemaRegistry.getMappedPropName(partValue);
         securityContext = this._schemaRegistry.securityContext(elementName, boundPropertyName);
         bindingType = PropertyBindingType.Property;
+        this._assertNoEventBinding(boundPropertyName, sourceSpan);
         if (!this._schemaRegistry.hasProperty(elementName, boundPropertyName, this._schemas)) {
           let errorMsg =
               `Can't bind to '${boundPropertyName}' since it isn't a known property of '${elementName}'.`;
@@ -868,12 +869,7 @@ class TemplateParseVisitor implements html.Visitor {
     } else {
       if (parts[0] == ATTRIBUTE_PREFIX) {
         boundPropertyName = parts[1];
-        if (boundPropertyName.toLowerCase().startsWith('on')) {
-          this._reportError(
-              `Binding to event attribute '${boundPropertyName}' is disallowed ` +
-                  `for security reasons, please use (${boundPropertyName.slice(2)})=...`,
-              sourceSpan);
-        }
+        this._assertNoEventBinding(boundPropertyName, sourceSpan);
         // NB: For security purposes, use the mapped property name, not the attribute name.
         const mapPropName = this._schemaRegistry.getMappedPropName(boundPropertyName);
         securityContext = this._schemaRegistry.securityContext(elementName, mapPropName);
@@ -906,6 +902,14 @@ class TemplateParseVisitor implements html.Visitor {
         boundPropertyName, bindingType, securityContext, ast, unit, sourceSpan);
   }
 
+  private _assertNoEventBinding(propName: string, sourceSpan: ParseSourceSpan): void {
+    if (propName.toLowerCase().startsWith('on')) {
+      this._reportError(
+          `Binding to event attribute '${propName}' is disallowed ` +
+              `for security reasons, please use (${propName.slice(2)})=...`,
+          sourceSpan, ParseErrorLevel.FATAL);
+    }
+  }
 
   private _findComponentDirectiveNames(directives: DirectiveAst[]): string[] {
     const componentTypeNames: string[] = [];
diff --git a/modules/@angular/core/test/linker/security_integration_spec.ts b/modules/@angular/core/test/linker/security_integration_spec.ts
index ea07c60953..3fb0891e90 100644
--- a/modules/@angular/core/test/linker/security_integration_spec.ts
+++ b/modules/@angular/core/test/linker/security_integration_spec.ts
@@ -6,9 +6,10 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
+import {CUSTOM_ELEMENTS_SCHEMA, NO_ERRORS_SCHEMA} from '@angular/core';
 import {Component} from '@angular/core/src/metadata';
 import {TestBed, getTestBed} from '@angular/core/testing';
-import {afterEach, beforeEach, beforeEachProviders, ddescribe, describe, expect, inject, it} from '@angular/core/testing/testing_internal';
+import {afterEach, beforeEach, beforeEachProviders, ddescribe, describe, expect, iit, inject, it} from '@angular/core/testing/testing_internal';
 import {getDOM} from '@angular/platform-browser/src/dom/dom_adapter';
 import {DomSanitizer} from '@angular/platform-browser/src/security/dom_sanitization_service';
 
@@ -39,19 +40,37 @@ function declareTests({useJit}: {useJit: boolean}) {
     });
     afterEach(() => { getDOM().log = originalLog; });
 
+    describe('events', () => {
+      it('should disallow binding to attr.on*', () => {
+        const template = `<div [attr.onclick]="ctxProp"></div>`;
+        TestBed.overrideComponent(SecuredComponent, {set: {template}});
+        try {
+          TestBed.createComponent(SecuredComponent);
+          throw 'Should throw';
+        } catch (e) {
+          expect(e.message).toContain(
+              `Template parse errors:\n` +
+              `Binding to event attribute 'onclick' is disallowed ` +
+              `for security reasons, please use (click)=... `);
+        }
+      });
 
-    it('should disallow binding on*', () => {
-      const template = `<div [attr.onclick]="ctxProp"></div>`;
-      TestBed.overrideComponent(SecuredComponent, {set: {template}});
-      try {
-        TestBed.createComponent(SecuredComponent);
-        throw 'Should throw';
-      } catch (e) {
-        expect(e.message).toContain(
-            `Template parse errors:\n` +
-            `Binding to event attribute 'onclick' is disallowed ` +
-            `for security reasons, please use (click)=... `);
-      }
+      it('should disallow binding to on* with NO_ERRORS_SCHEMA', () => {
+        const template = `<div [onclick]="ctxProp"></div>`;
+        TestBed.overrideComponent(SecuredComponent, {set: {template}}).configureTestingModule({
+          schemas: [NO_ERRORS_SCHEMA]
+        });
+        ;
+        try {
+          TestBed.createComponent(SecuredComponent);
+          throw 'Should throw';
+        } catch (e) {
+          expect(e.message).toContain(
+              `Template parse errors:\n` +
+              `Binding to event attribute 'onclick' is disallowed ` +
+              `for security reasons, please use (click)=... `);
+        }
+      });
     });
 
     describe('safe HTML values', function() {