feat(security): complete DOM security schema.
This addresses several oversights in assigning security contexts to DOM schema elements found by our security reviewers (thanks!). This also adds some more precise unit tests for the interaction between (Dom)ElementSchemaRegistry and the TemplateParser, and extracts the security specific parts into dom_security_schema.ts. Comparison of (potentially) dangerous property names is done case insensitive, to avoid issues like formAction vs formaction. Part of issue #8511.
This commit is contained in:
parent
a78a43c816
commit
040b101842
|
@ -3,6 +3,7 @@ import {SecurityContext} from '../../core_private';
|
||||||
import {isPresent} from '../facade/lang';
|
import {isPresent} from '../facade/lang';
|
||||||
import {StringMapWrapper} from '../facade/collection';
|
import {StringMapWrapper} from '../facade/collection';
|
||||||
import {ElementSchemaRegistry} from './element_schema_registry';
|
import {ElementSchemaRegistry} from './element_schema_registry';
|
||||||
|
import {SECURITY_SCHEMA} from './dom_security_schema';
|
||||||
|
|
||||||
const EVENT = 'event';
|
const EVENT = 'event';
|
||||||
const BOOLEAN = 'boolean';
|
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,
|
* NOTE: This schema is auto extracted from `schema_extractor.ts` located in the test folder,
|
||||||
* see dom_element_schema_registry_spec.ts
|
* 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[] =
|
const SCHEMA: string[] =
|
||||||
/*@ts2dart_const*/ ([
|
/*@ts2dart_const*/ ([
|
||||||
'*|%classList,className,id,innerHTML,*beforecopy,*beforecut,*beforepaste,*copy,*cut,*paste,*search,*selectstart,*webkitfullscreenchange,*webkitfullscreenerror,*wheel,outerHTML,#scrollLeft,#scrollTop',
|
'*|%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} = <any>{
|
var attrToPropMap: {[name: string]: string} = <any>{
|
||||||
'class': 'className',
|
'class': 'className',
|
||||||
|
'formaction': 'formAction',
|
||||||
'innerHtml': 'innerHTML',
|
'innerHtml': 'innerHTML',
|
||||||
'readonly': 'readOnly',
|
'readonly': 'readOnly',
|
||||||
'tabindex': 'tabIndex'
|
'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()
|
@Injectable()
|
||||||
export class DomElementSchemaRegistry extends ElementSchemaRegistry {
|
export class DomElementSchemaRegistry extends ElementSchemaRegistry {
|
||||||
schema = <{[element: string]: {[property: string]: string}}>{};
|
schema = <{[element: string]: {[property: string]: string}}>{};
|
||||||
|
@ -276,6 +244,8 @@ export class DomElementSchemaRegistry extends ElementSchemaRegistry {
|
||||||
if (property == '') {
|
if (property == '') {
|
||||||
} else if (property.startsWith('*')) {
|
} else if (property.startsWith('*')) {
|
||||||
// We don't yet support events.
|
// 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;
|
// type[property.substring(1)] = EVENT;
|
||||||
} else if (property.startsWith('!')) {
|
} else if (property.startsWith('!')) {
|
||||||
type[property.substring(1)] = BOOLEAN;
|
type[property.substring(1)] = BOOLEAN;
|
||||||
|
@ -315,6 +285,10 @@ export class DomElementSchemaRegistry extends ElementSchemaRegistry {
|
||||||
* attack vectors are assigned their appropriate context.
|
* attack vectors are assigned their appropriate context.
|
||||||
*/
|
*/
|
||||||
securityContext(tagName: string, propName: string): SecurityContext {
|
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];
|
let ctx = SECURITY_SCHEMA[tagName + '|' + propName];
|
||||||
if (ctx !== undefined) return ctx;
|
if (ctx !== undefined) return ctx;
|
||||||
ctx = SECURITY_SCHEMA['*|' + propName];
|
ctx = SECURITY_SCHEMA['*|' + propName];
|
||||||
|
|
|
@ -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',
|
||||||
|
]);
|
|
@ -77,6 +77,12 @@ export function main() {
|
||||||
expect(registry.hasProperty(nodeName, 'type')).toBeTruthy();
|
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) {
|
if (browserDetection.isChromeDesktop) {
|
||||||
it('generate a new schema', () => {
|
it('generate a new schema', () => {
|
||||||
// console.log(JSON.stringify(registry.properties));
|
// console.log(JSON.stringify(registry.properties));
|
||||||
|
|
|
@ -11,6 +11,7 @@ import {
|
||||||
beforeEachProviders
|
beforeEachProviders
|
||||||
} from '@angular/core/testing/testing_internal';
|
} from '@angular/core/testing/testing_internal';
|
||||||
import {provide} from '@angular/core';
|
import {provide} from '@angular/core';
|
||||||
|
import {SecurityContext} from '../core_private';
|
||||||
|
|
||||||
import {Console} from '@angular/core/src/console';
|
import {Console} from '@angular/core/src/console';
|
||||||
|
|
||||||
|
@ -51,6 +52,7 @@ import {
|
||||||
import {identifierToken, Identifiers} from '../src/identifiers';
|
import {identifierToken, Identifiers} from '../src/identifiers';
|
||||||
|
|
||||||
import {ElementSchemaRegistry} from '@angular/compiler/src/schema/element_schema_registry';
|
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 {MockSchemaRegistry} from '@angular/compiler/testing';
|
||||||
|
|
||||||
import {Unparser} from './expression_parser/unparser';
|
import {Unparser} from './expression_parser/unparser';
|
||||||
|
@ -66,9 +68,13 @@ var MOCK_SCHEMA_REGISTRY = [
|
||||||
{useValue: new MockSchemaRegistry({'invalidProp': false}, {'mappedAttr': 'mappedProp'})})
|
{useValue: new MockSchemaRegistry({'invalidProp': false}, {'mappedAttr': 'mappedProp'})})
|
||||||
];
|
];
|
||||||
|
|
||||||
|
let zeConsole = console;
|
||||||
|
|
||||||
export function main() {
|
export function main() {
|
||||||
var ngIf;
|
var ngIf;
|
||||||
var parse;
|
var parse:
|
||||||
|
(template: string, directives: CompileDirectiveMetadata[], pipes?: CompilePipeMetadata[]) =>
|
||||||
|
TemplateAst[];
|
||||||
var console: ArrayConsole;
|
var console: ArrayConsole;
|
||||||
|
|
||||||
function commonBeforeEach() {
|
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 = (<ElementAst>ast[0]).inputs[0];
|
||||||
|
return propBinding.securityContext;
|
||||||
|
}
|
||||||
|
|
||||||
|
it('should set for properties', () => {
|
||||||
|
expect(secContext('<div [title]="v">')).toBe(SecurityContext.NONE);
|
||||||
|
expect(secContext('<div [innerHTML]="v">')).toBe(SecurityContext.HTML);
|
||||||
|
});
|
||||||
|
it('should set for property value bindings', () => {
|
||||||
|
expect(secContext('<div innerHTML="{{v}}">')).toBe(SecurityContext.HTML);
|
||||||
|
});
|
||||||
|
it('should set for attributes', () => {
|
||||||
|
expect(secContext('<a [attr.href]="v">')).toBe(SecurityContext.URL);
|
||||||
|
// NB: attributes below need to change case.
|
||||||
|
expect(secContext('<a [attr.innerHtml]="v">')).toBe(SecurityContext.HTML);
|
||||||
|
expect(secContext('<a [attr.formaction]="v">')).toBe(SecurityContext.URL);
|
||||||
|
});
|
||||||
|
it('should set for style', () => {
|
||||||
|
expect(secContext('<a [style.backgroundColor]="v">')).toBe(SecurityContext.STYLE);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
describe('TemplateParser', () => {
|
describe('TemplateParser', () => {
|
||||||
beforeEachProviders(() => [TEST_PROVIDERS, MOCK_SCHEMA_REGISTRY]);
|
beforeEachProviders(() => [TEST_PROVIDERS, MOCK_SCHEMA_REGISTRY]);
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue