diff --git a/modules/@angular/compiler/src/schema/dom_element_schema_registry.ts b/modules/@angular/compiler/src/schema/dom_element_schema_registry.ts index 9dec97c367..c62ef42d4c 100644 --- a/modules/@angular/compiler/src/schema/dom_element_schema_registry.ts +++ b/modules/@angular/compiler/src/schema/dom_element_schema_registry.ts @@ -3,6 +3,7 @@ import {SecurityContext} from '../../core_private'; import {isPresent} from '../facade/lang'; import {StringMapWrapper} from '../facade/collection'; import {ElementSchemaRegistry} from './element_schema_registry'; +import {SECURITY_SCHEMA} from './dom_security_schema'; const EVENT = 'event'; const BOOLEAN = 'boolean'; @@ -51,6 +52,20 @@ const OBJECT = 'object'; * NOTE: This schema is auto extracted from `schema_extractor.ts` located in the test folder, * see dom_element_schema_registry_spec.ts */ + +// ================================================================================================= +// ================================================================================================= +// =========== S T O P - S T O P - S T O P - S T O P - S T O P - S T O P =========== +// ================================================================================================= +// ================================================================================================= +// +// DO NOT EDIT THIS DOM SCHEMA WITHOUT A SECURITY REVIEW! +// +// Newly added properties must be security reviewed and assigned an appropriate SecurityContext in +// dom_security_schema.ts. Reach out to mprobst for details. +// +// ================================================================================================= + const SCHEMA: string[] = /*@ts2dart_const*/ ([ '*|%classList,className,id,innerHTML,*beforecopy,*beforecut,*beforepaste,*copy,*cut,*paste,*search,*selectstart,*webkitfullscreenchange,*webkitfullscreenerror,*wheel,outerHTML,#scrollLeft,#scrollTop', @@ -202,59 +217,12 @@ const SCHEMA: string[] = var attrToPropMap: {[name: string]: string} = { 'class': 'className', + 'formaction': 'formAction', 'innerHtml': 'innerHTML', 'readonly': 'readOnly', 'tabindex': 'tabIndex' }; -function registerContext(map: {[k: string]: SecurityContext}, ctx: SecurityContext, specs: string[]) { - for (let spec of specs) map[spec] = ctx; -} - -/** Map from tagName|propertyName SecurityContext. Properties applying to all tags use '*'. */ -const SECURITY_SCHEMA: {[k: string]: SecurityContext} = {}; - -registerContext(SECURITY_SCHEMA, SecurityContext.HTML, [ - 'iframe|srcdoc', - '*|innerHTML', - '*|outerHTML', -]); -registerContext(SECURITY_SCHEMA, SecurityContext.STYLE, ['*|style']); -// NB: no SCRIPT contexts here, they are never allowed. -registerContext(SECURITY_SCHEMA, SecurityContext.URL, [ - 'area|href', - 'area|ping', - 'audio|src', - 'a|href', - 'a|ping', - 'blockquote|cite', - 'body|background', - 'button|formaction', - 'del|cite', - 'form|action', - 'img|src', - 'input|formaction', - 'input|src', - 'ins|cite', - 'q|cite', - 'source|src', - 'video|poster', - 'video|src', -]); -registerContext(SECURITY_SCHEMA, SecurityContext.RESOURCE_URL, [ - 'applet|code', - 'applet|codebase', - 'base|href', - 'frame|src', - 'head|profile', - 'html|manifest', - 'iframe|src', - 'object|codebase', - 'object|data', - 'script|src', - 'track|src', -]); - @Injectable() export class DomElementSchemaRegistry extends ElementSchemaRegistry { schema = <{[element: string]: {[property: string]: string}}>{}; @@ -276,6 +244,8 @@ export class DomElementSchemaRegistry extends ElementSchemaRegistry { if (property == '') { } else if (property.startsWith('*')) { // We don't yet support events. + // If ever allowing to bind to events, GO THROUGH A SECURITY REVIEW, allowing events will + // almost certainly introduce bad XSS vulnerabilities. // type[property.substring(1)] = EVENT; } else if (property.startsWith('!')) { type[property.substring(1)] = BOOLEAN; @@ -315,6 +285,10 @@ export class DomElementSchemaRegistry extends ElementSchemaRegistry { * attack vectors are assigned their appropriate context. */ securityContext(tagName: string, propName: string): SecurityContext { + // Make sure comparisons are case insensitive, so that case differences between attribute and + // property names do not have a security impact. + tagName = tagName.toLowerCase(); + propName = propName.toLowerCase(); let ctx = SECURITY_SCHEMA[tagName + '|' + propName]; if (ctx !== undefined) return ctx; ctx = SECURITY_SCHEMA['*|' + propName]; diff --git a/modules/@angular/compiler/src/schema/dom_security_schema.ts b/modules/@angular/compiler/src/schema/dom_security_schema.ts new file mode 100644 index 0000000000..69199ce8ed --- /dev/null +++ b/modules/@angular/compiler/src/schema/dom_security_schema.ts @@ -0,0 +1,66 @@ +import {SecurityContext} from '../../core_private'; + +// ================================================================================================= +// ================================================================================================= +// =========== S T O P - S T O P - S T O P - S T O P - S T O P - S T O P =========== +// ================================================================================================= +// ================================================================================================= +// +// DO NOT EDIT THIS LIST OF SECURITY SENSITIVE PROPERTIES WITHOUT A SECURITY REVIEW! +// Reach out to mprobst for details. +// +// ================================================================================================= + +/** Map from tagName|propertyName SecurityContext. Properties applying to all tags use '*'. */ +export const SECURITY_SCHEMA: {[k: string]: SecurityContext} = {}; + +function registerContext(ctx: SecurityContext, specs: string[]) { + for (let spec of specs) SECURITY_SCHEMA[spec.toLowerCase()] = ctx; +} + +// Case is insignificant below, all element and attribute names are lower-cased for lookup. + +registerContext(SecurityContext.HTML, [ + 'iframe|srcdoc', + '*|innerHTML', + '*|outerHTML', +]); +registerContext(SecurityContext.STYLE, ['*|style']); +// NB: no SCRIPT contexts here, they are never allowed due to the parser stripping them. +registerContext(SecurityContext.URL, [ + '*|formAction', + 'area|href', + 'area|ping', + 'audio|src', + 'a|href', + 'a|ping', + 'blockquote|cite', + 'body|background', + 'del|cite', + 'form|action', + 'img|src', + 'img|srcset', + 'input|src', + 'ins|cite', + 'q|cite', + 'source|src', + 'source|srcset', + 'video|poster', + 'video|src', +]); +registerContext(SecurityContext.RESOURCE_URL, [ + 'applet|code', + 'applet|codebase', + 'base|href', + 'embed|src', + 'frame|src', + 'head|profile', + 'html|manifest', + 'iframe|src', + 'link|href', + 'media|src', + 'object|codebase', + 'object|data', + 'script|src', + 'track|src', +]); diff --git a/modules/@angular/compiler/test/schema/dom_element_schema_registry_spec.ts b/modules/@angular/compiler/test/schema/dom_element_schema_registry_spec.ts index e48e4e8c4a..f8be96ca55 100644 --- a/modules/@angular/compiler/test/schema/dom_element_schema_registry_spec.ts +++ b/modules/@angular/compiler/test/schema/dom_element_schema_registry_spec.ts @@ -77,6 +77,12 @@ export function main() { expect(registry.hasProperty(nodeName, 'type')).toBeTruthy(); }); + it('should check security contexts case insensitive', () => { + expect(registry.securityContext('p', 'iNnErHtMl')).toBe(SecurityContext.HTML); + expect(registry.securityContext('p', 'formaction')).toBe(SecurityContext.URL); + expect(registry.securityContext('p', 'formAction')).toBe(SecurityContext.URL); + }); + if (browserDetection.isChromeDesktop) { it('generate a new schema', () => { // console.log(JSON.stringify(registry.properties)); diff --git a/modules/@angular/compiler/test/template_parser_spec.ts b/modules/@angular/compiler/test/template_parser_spec.ts index 8f48a1a3ed..aea243fe50 100644 --- a/modules/@angular/compiler/test/template_parser_spec.ts +++ b/modules/@angular/compiler/test/template_parser_spec.ts @@ -11,6 +11,7 @@ import { beforeEachProviders } from '@angular/core/testing/testing_internal'; import {provide} from '@angular/core'; +import {SecurityContext} from '../core_private'; import {Console} from '@angular/core/src/console'; @@ -51,6 +52,7 @@ import { import {identifierToken, Identifiers} from '../src/identifiers'; import {ElementSchemaRegistry} from '@angular/compiler/src/schema/element_schema_registry'; +import {DomElementSchemaRegistry} from '@angular/compiler/src/schema/dom_element_schema_registry'; import {MockSchemaRegistry} from '@angular/compiler/testing'; import {Unparser} from './expression_parser/unparser'; @@ -66,9 +68,13 @@ var MOCK_SCHEMA_REGISTRY = [ {useValue: new MockSchemaRegistry({'invalidProp': false}, {'mappedAttr': 'mappedProp'})}) ]; +let zeConsole = console; + export function main() { var ngIf; - var parse; + var parse: + (template: string, directives: CompileDirectiveMetadata[], pipes?: CompilePipeMetadata[]) => + TemplateAst[]; var console: ArrayConsole; function commonBeforeEach() { @@ -120,6 +126,41 @@ export function main() { }); }); + describe('TemplateParser Security', () => { + // Semi-integration test to make sure TemplateParser properly sets the security context. + // Uses the actual DomElementSchemaRegistry. + beforeEachProviders( + () => + [TEST_PROVIDERS, provide(ElementSchemaRegistry, {useClass: DomElementSchemaRegistry})]); + + commonBeforeEach(); + + describe('security context', () => { + function secContext(tpl: string): SecurityContext { + let ast = parse(tpl, []); + let propBinding = (ast[0]).inputs[0]; + return propBinding.securityContext; + } + + it('should set for properties', () => { + expect(secContext('
')).toBe(SecurityContext.NONE); + expect(secContext('
')).toBe(SecurityContext.HTML); + }); + it('should set for property value bindings', () => { + expect(secContext('
')).toBe(SecurityContext.HTML); + }); + it('should set for attributes', () => { + expect(secContext('')).toBe(SecurityContext.URL); + // NB: attributes below need to change case. + expect(secContext('')).toBe(SecurityContext.HTML); + expect(secContext('')).toBe(SecurityContext.URL); + }); + it('should set for style', () => { + expect(secContext('')).toBe(SecurityContext.STYLE); + }); + }); + }); + describe('TemplateParser', () => { beforeEachProviders(() => [TEST_PROVIDERS, MOCK_SCHEMA_REGISTRY]);