cargdev/angular
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(ivy): sanitization for Host Bindings (#27939)

Browse Source 
This commit adds sanitization for `elementProperty` and `elementAttribute` instructions used in `hostBindings` function, similar to what we already have in the `template` function. Main difference is the fact that for some attributes (like "href" and "src") we can't define which SecurityContext they belong to (URL vs RESOURCE_URL) in Compiler, since information in Directive selector may not be enough to calculate it. In order to resolve the problem, Compiler injects slightly different sanitization function which detects proper Security Context at runtime.

PR Close #27939
This commit is contained in:
Andrew Kushnir
2019-01-03 10:04:06 -08:00
committed by Kara Erickson
parent 1de4031d9c
commit c3aa24c3f9
14 changed files with 430 additions and 48 deletions
Download Patch File Download Diff File Expand all files Collapse all files

54
packages/core/test/linker/security_integration_spec.ts
View File

@ -171,39 +171,37 @@ function declareTests(config?: {useJit: boolean}) {
checkEscapeOfHrefProperty(fixture, true);
});
fixmeIvy('FW-785: Host bindings are not sanitised')
.it('should escape unsafe properties if they are used in host bindings', () => {
@Directive({selector: '[dirHref]'})
class HrefDirective {
// TODO(issue/24571): remove '!'.
@HostBinding('href') @Input()
dirHref !: string;
}
it('should escape unsafe properties if they are used in host bindings', () => {
@Directive({selector: '[dirHref]'})
class HrefDirective {
// TODO(issue/24571): remove '!'.
@HostBinding('href') @Input()
dirHref !: string;
}
const template = `<a [dirHref]="ctxProp">Link Title</a>`;
TestBed.configureTestingModule({declarations: [HrefDirective]});
TestBed.overrideComponent(SecuredComponent, {set: {template}});
const fixture = TestBed.createComponent(SecuredComponent);
const template = `<a [dirHref]="ctxProp">Link Title</a>`;
TestBed.configureTestingModule({declarations: [HrefDirective]});
TestBed.overrideComponent(SecuredComponent, {set: {template}});
const fixture = TestBed.createComponent(SecuredComponent);
checkEscapeOfHrefProperty(fixture, false);
});
checkEscapeOfHrefProperty(fixture, false);
});
fixmeIvy('FW-785: Host bindings are not sanitised')
.it('should escape unsafe attributes if they are used in host bindings', () => {
@Directive({selector: '[dirHref]'})
class HrefDirective {
// TODO(issue/24571): remove '!'.
@HostBinding('attr.href') @Input()
dirHref !: string;
}
it('should escape unsafe attributes if they are used in host bindings', () => {
@Directive({selector: '[dirHref]'})
class HrefDirective {
// TODO(issue/24571): remove '!'.
@HostBinding('attr.href') @Input()
dirHref !: string;
}
const template = `<a [dirHref]="ctxProp">Link Title</a>`;
TestBed.configureTestingModule({declarations: [HrefDirective]});
TestBed.overrideComponent(SecuredComponent, {set: {template}});
const fixture = TestBed.createComponent(SecuredComponent);
const template = `<a [dirHref]="ctxProp">Link Title</a>`;
TestBed.configureTestingModule({declarations: [HrefDirective]});
TestBed.overrideComponent(SecuredComponent, {set: {template}});
const fixture = TestBed.createComponent(SecuredComponent);
checkEscapeOfHrefProperty(fixture, true);
});
checkEscapeOfHrefProperty(fixture, true);
});
it('should escape unsafe style values', () => {
const template = `<div [style.background]="ctxProp">Text</div>`;

60
packages/core/test/render3/host_binding_spec.ts
View File

@ -9,10 +9,12 @@
import {ElementRef, QueryList} from '@angular/core';
import {AttributeMarker, defineComponent, template, defineDirective, InheritDefinitionFeature, ProvidersFeature, NgOnChangesFeature} from '../../src/render3/index';
import {allocHostVars, bind, directiveInject, element, elementEnd, elementProperty, elementStyleProp, elementStyling, elementStylingApply, elementStart, listener, load, text, textBinding, loadQueryList, registerContentQuery, elementHostAttrs} from '../../src/render3/instructions';
import {allocHostVars, bind, directiveInject, element, elementAttribute, elementEnd, elementProperty, elementStyleProp, elementStyling, elementStylingApply, elementStart, listener, load, text, textBinding, loadQueryList, registerContentQuery, elementHostAttrs} from '../../src/render3/instructions';
import {query, queryRefresh} from '../../src/render3/query';
import {RenderFlags} from '../../src/render3/interfaces/definition';
import {pureFunction1, pureFunction2} from '../../src/render3/pure_function';
import {sanitizeUrl, sanitizeUrlOrResourceUrl, sanitizeHtml} from '../../src/sanitization/sanitization';
import {bypassSanitizationTrustHtml, bypassSanitizationTrustResourceUrl, bypassSanitizationTrustUrl} from '../../src/sanitization/bypass';
import {ComponentFixture, TemplateFixture, createComponent, createDirective} from './render_util';
import {NgForOf} from './common_with_def';
@ -1164,4 +1166,60 @@ describe('host bindings', () => {
expect(hostBindingEl.className).toEqual('mat-toolbar');
});
});
describe('sanitization', () => {
function verify(
tag: string, prop: string, value: any, expectedSanitizedValue: any, sanitizeFn: any,
bypassFn: any, isAttribute: boolean = true) {
it('should sanitize potentially unsafe properties and attributes', () => {
let hostBindingDir: UnsafeUrlHostBindingDir;
class UnsafeUrlHostBindingDir {
// val: any = value;
static ngDirectiveDef = defineDirective({
type: UnsafeUrlHostBindingDir,
selectors: [['', 'unsafeUrlHostBindingDir', '']],
factory: () => hostBindingDir = new UnsafeUrlHostBindingDir(),
hostBindings: (rf: RenderFlags, ctx: any, elementIndex: number) => {
if (rf & RenderFlags.Create) {
allocHostVars(1);
}
if (rf & RenderFlags.Update) {
const fn = isAttribute ? elementAttribute : elementProperty;
(fn as any)(elementIndex, prop, bind(ctx[prop]), sanitizeFn, true);
}
}
});
}
const fixture = new TemplateFixture(() => {
element(0, tag, ['unsafeUrlHostBindingDir', '']);
}, () => {}, 1, 0, [UnsafeUrlHostBindingDir]);
const el = fixture.hostElement.querySelector(tag) !;
const current = () => isAttribute ? el.getAttribute(prop) : (el as any)[prop];
(hostBindingDir !as any)[prop] = value;
fixture.update();
expect(current()).toEqual(expectedSanitizedValue);
(hostBindingDir !as any)[prop] = bypassFn(value);
fixture.update();
expect(current()).toEqual(value);
});
}
verify(
'a', 'href', 'javascript:alert(1)', 'unsafe:javascript:alert(1)', sanitizeUrlOrResourceUrl,
bypassSanitizationTrustUrl);
verify(
'script', 'src', bypassSanitizationTrustResourceUrl('javascript:alert(2)'),
'javascript:alert(2)', sanitizeUrlOrResourceUrl, bypassSanitizationTrustResourceUrl);
verify(
'blockquote', 'cite', 'javascript:alert(3)', 'unsafe:javascript:alert(3)', sanitizeUrl,
bypassSanitizationTrustUrl);
verify(
'b', 'innerHTML', '<img src="javascript:alert(4)">',
'<img src="unsafe:javascript:alert(4)">', sanitizeHtml, bypassSanitizationTrustHtml,
/* isAttribute */ false);
});
});

52
packages/core/test/render3/integration_spec.ts
View File

@ -2878,6 +2878,58 @@ describe('render3 integration test', () => {
expect(anchor.getAttribute('href')).toEqual('http://foo');
});
it('should sanitize HostBindings data using provided sanitization interface', () => {
let hostBindingDir: UnsafeUrlHostBindingDir;
class UnsafeUrlHostBindingDir {
// @HostBinding()
cite: any = 'http://cite-dir-value';
static ngDirectiveDef = defineDirective({
type: UnsafeUrlHostBindingDir,
selectors: [['', 'unsafeUrlHostBindingDir', '']],
factory: () => hostBindingDir = new UnsafeUrlHostBindingDir(),
hostBindings: (rf: RenderFlags, ctx: any, elementIndex: number) => {
if (rf & RenderFlags.Create) {
allocHostVars(1);
}
if (rf & RenderFlags.Update) {
elementProperty(elementIndex, 'cite', bind(ctx.cite), sanitizeUrl, true);
}
}
});
}
class SimpleComp {
static ngComponentDef = defineComponent({
type: SimpleComp,
selectors: [['sanitize-this']],
factory: () => new SimpleComp(),
consts: 1,
vars: 0,
template: (rf: RenderFlags, ctx: SimpleComp) => {
if (rf & RenderFlags.Create) {
element(0, 'blockquote', ['unsafeUrlHostBindingDir', '']);
}
},
directives: [UnsafeUrlHostBindingDir]
});
}
const sanitizer = new LocalSanitizer((value) => 'http://bar');
const fixture = new ComponentFixture(SimpleComp, {sanitizer});
hostBindingDir !.cite = 'http://foo';
fixture.update();
const anchor = fixture.hostElement.querySelector('blockquote') !;
expect(anchor.getAttribute('cite')).toEqual('http://bar');
hostBindingDir !.cite = sanitizer.bypassSecurityTrustUrl('http://foo');
fixture.update();
expect(anchor.getAttribute('cite')).toEqual('http://foo');
});
});
});

54
packages/core/test/sanitization/sanatization_spec.ts
View File

@ -7,10 +7,12 @@
* found in the LICENSE file at https://angular.io/license
*/
import {SECURITY_SCHEMA} from '@angular/compiler/src/schema/dom_security_schema';
import {setTNodeAndViewData} from '@angular/core/src/render3/state';
import {bypassSanitizationTrustHtml, bypassSanitizationTrustResourceUrl, bypassSanitizationTrustScript, bypassSanitizationTrustStyle, bypassSanitizationTrustUrl} from '../../src/sanitization/bypass';
import {sanitizeHtml, sanitizeResourceUrl, sanitizeScript, sanitizeStyle, sanitizeUrl} from '../../src/sanitization/sanitization';
import {getUrlSanitizer, sanitizeHtml, sanitizeResourceUrl, sanitizeScript, sanitizeStyle, sanitizeUrl, sanitizeUrlOrResourceUrl} from '../../src/sanitization/sanitization';
import {SecurityContext} from '../../src/sanitization/security';
describe('sanitization', () => {
beforeEach(() => setTNodeAndViewData(null !, [] as any));
@ -69,4 +71,54 @@ describe('sanitization', () => {
expect(() => sanitizeScript(bypassSanitizationTrustHtml('true'))).toThrowError(ERROR);
expect(sanitizeScript(bypassSanitizationTrustScript('true'))).toEqual('true');
});
it('should select correct sanitizer for URL props', () => {
// making sure security schema we have on compiler side is in sync with the `getUrlSanitizer`
// runtime function definition
const schema = SECURITY_SCHEMA();
const contextsByProp: Map<string, Set<number>> = new Map();
const sanitizerNameByContext: Map<number, string> = new Map([
[SecurityContext.URL, 'sanitizeUrl'], [SecurityContext.RESOURCE_URL, 'sanitizeResourceUrl']
]);
Object.keys(schema).forEach(key => {
const context = schema[key];
if (context === SecurityContext.URL || SecurityContext.RESOURCE_URL) {
const [tag, prop] = key.split('|');
const contexts = contextsByProp.get(prop) || new Set<number>();
contexts.add(context);
contextsByProp.set(prop, contexts);
// check only in case a prop can be a part of both URL contexts
if (contexts.size === 2) {
expect(getUrlSanitizer(tag, prop).name).toEqual(sanitizerNameByContext.get(context) !);
}
}
});
});
it('should sanitize resourceUrls via sanitizeUrlOrResourceUrl', () => {
const ERROR = 'unsafe value used in a resource URL context (see http://g.co/ng/security#xss)';
expect(() => sanitizeUrlOrResourceUrl('http://server', 'iframe', 'src')).toThrowError(ERROR);
expect(() => sanitizeUrlOrResourceUrl('javascript:true', 'iframe', 'src')).toThrowError(ERROR);
expect(
() => sanitizeUrlOrResourceUrl(
bypassSanitizationTrustHtml('javascript:true'), 'iframe', 'src'))
.toThrowError(ERROR);
expect(sanitizeUrlOrResourceUrl(
bypassSanitizationTrustResourceUrl('javascript:true'), 'iframe', 'src'))
.toEqual('javascript:true');
});
it('should sanitize urls via sanitizeUrlOrResourceUrl', () => {
expect(sanitizeUrlOrResourceUrl('http://server', 'a', 'href')).toEqual('http://server');
expect(sanitizeUrlOrResourceUrl(new Wrap('http://server'), 'a', 'href'))
.toEqual('http://server');
expect(sanitizeUrlOrResourceUrl('javascript:true', 'a', 'href'))
.toEqual('unsafe:javascript:true');
expect(sanitizeUrlOrResourceUrl(new Wrap('javascript:true'), 'a', 'href'))
.toEqual('unsafe:javascript:true');
expect(sanitizeUrlOrResourceUrl(bypassSanitizationTrustHtml('javascript:true'), 'a', 'href'))
.toEqual('unsafe:javascript:true');
expect(sanitizeUrlOrResourceUrl(bypassSanitizationTrustUrl('javascript:true'), 'a', 'href'))
.toEqual('javascript:true');
});
});