feat(security): fail more detectably when using a safe value in an interpolation.
If a user ends up with a safe value in an interpolation context, that's probably a bug. Returning `"SafeValue must use [property]= binding"` will make it easier to detect and correct the situation. Detecting the situation and throwing an error for it could cause performance issues, so we're not doing this at this point (but might revisit later). Part of #8511 and #9253.
This commit is contained in:
parent
44e0ad4987
commit
8879aa1df4
|
@ -29,12 +29,12 @@ class SecuredComponent {
|
||||||
constructor() { this.ctxProp = 'some value'; }
|
constructor() { this.ctxProp = 'some value'; }
|
||||||
}
|
}
|
||||||
|
|
||||||
function itAsync(msg: string, injections: Function[], f: Function): any; /** TODO #???? */
|
function itAsync(msg: string, injections: Function[], f: Function): void;
|
||||||
function itAsync(msg: string, f: (tcb: TestComponentBuilder, atc: AsyncTestCompleter) => void):
|
function itAsync(
|
||||||
any; /** TODO #???? */
|
msg: string, f: (tcb: TestComponentBuilder, atc: AsyncTestCompleter) => void): void;
|
||||||
function itAsync(
|
function itAsync(
|
||||||
msg: string, f: Function[] | ((tcb: TestComponentBuilder, atc: AsyncTestCompleter) => void),
|
msg: string, f: Function[] | ((tcb: TestComponentBuilder, atc: AsyncTestCompleter) => void),
|
||||||
fn?: Function): any /** TODO #???? */ {
|
fn?: Function): void {
|
||||||
if (f instanceof Function) {
|
if (f instanceof Function) {
|
||||||
it(msg, inject([TestComponentBuilder, AsyncTestCompleter], <Function>f));
|
it(msg, inject([TestComponentBuilder, AsyncTestCompleter], <Function>f));
|
||||||
} else {
|
} else {
|
||||||
|
@ -63,8 +63,7 @@ function declareTests({useJit}: {useJit: boolean}) {
|
||||||
|
|
||||||
|
|
||||||
itAsync(
|
itAsync(
|
||||||
'should disallow binding on*',
|
'should disallow binding on*', (tcb: TestComponentBuilder, async: AsyncTestCompleter) => {
|
||||||
(tcb: TestComponentBuilder, async: any /** TODO #???? */) => {
|
|
||||||
let tpl = `<div [attr.onclick]="ctxProp"></div>`;
|
let tpl = `<div [attr.onclick]="ctxProp"></div>`;
|
||||||
tcb = tcb.overrideView(SecuredComponent, new ViewMetadata({template: tpl}));
|
tcb = tcb.overrideView(SecuredComponent, new ViewMetadata({template: tpl}));
|
||||||
PromiseWrapper.catchError(tcb.createAsync(SecuredComponent), (e) => {
|
PromiseWrapper.catchError(tcb.createAsync(SecuredComponent), (e) => {
|
||||||
|
@ -81,7 +80,7 @@ function declareTests({useJit}: {useJit: boolean}) {
|
||||||
itAsync(
|
itAsync(
|
||||||
'should not escape values marked as trusted',
|
'should not escape values marked as trusted',
|
||||||
[TestComponentBuilder, AsyncTestCompleter, DomSanitizationService],
|
[TestComponentBuilder, AsyncTestCompleter, DomSanitizationService],
|
||||||
(tcb: TestComponentBuilder, async: any /** TODO #???? */,
|
(tcb: TestComponentBuilder, async: AsyncTestCompleter,
|
||||||
sanitizer: DomSanitizationService) => {
|
sanitizer: DomSanitizationService) => {
|
||||||
let tpl = `<a [href]="ctxProp">Link Title</a>`;
|
let tpl = `<a [href]="ctxProp">Link Title</a>`;
|
||||||
tcb.overrideView(SecuredComponent, new ViewMetadata({template: tpl, directives: []}))
|
tcb.overrideView(SecuredComponent, new ViewMetadata({template: tpl, directives: []}))
|
||||||
|
@ -101,7 +100,7 @@ function declareTests({useJit}: {useJit: boolean}) {
|
||||||
itAsync(
|
itAsync(
|
||||||
'should error when using the wrong trusted value',
|
'should error when using the wrong trusted value',
|
||||||
[TestComponentBuilder, AsyncTestCompleter, DomSanitizationService],
|
[TestComponentBuilder, AsyncTestCompleter, DomSanitizationService],
|
||||||
(tcb: TestComponentBuilder, async: any /** TODO #???? */,
|
(tcb: TestComponentBuilder, async: AsyncTestCompleter,
|
||||||
sanitizer: DomSanitizationService) => {
|
sanitizer: DomSanitizationService) => {
|
||||||
let tpl = `<a [href]="ctxProp">Link Title</a>`;
|
let tpl = `<a [href]="ctxProp">Link Title</a>`;
|
||||||
tcb.overrideView(SecuredComponent, new ViewMetadata({template: tpl, directives: []}))
|
tcb.overrideView(SecuredComponent, new ViewMetadata({template: tpl, directives: []}))
|
||||||
|
@ -116,12 +115,32 @@ function declareTests({useJit}: {useJit: boolean}) {
|
||||||
async.done();
|
async.done();
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
itAsync(
|
||||||
|
'should warn when using in string interpolation',
|
||||||
|
[TestComponentBuilder, AsyncTestCompleter, DomSanitizationService],
|
||||||
|
(tcb: TestComponentBuilder, async: AsyncTestCompleter,
|
||||||
|
sanitizer: DomSanitizationService) => {
|
||||||
|
let tpl = `<a href="/foo/{{ctxProp}}">Link Title</a>`;
|
||||||
|
tcb.overrideView(SecuredComponent, new ViewMetadata({template: tpl, directives: []}))
|
||||||
|
.createAsync(SecuredComponent)
|
||||||
|
.then((fixture) => {
|
||||||
|
let e = fixture.debugElement.children[0].nativeElement;
|
||||||
|
let trusted = sanitizer.bypassSecurityTrustUrl('bar/baz');
|
||||||
|
let ci = fixture.debugElement.componentInstance;
|
||||||
|
ci.ctxProp = trusted;
|
||||||
|
fixture.detectChanges();
|
||||||
|
expect(getDOM().getProperty(e, 'href')).toMatch(/SafeValue(%20| )must(%20| )use/);
|
||||||
|
|
||||||
|
async.done();
|
||||||
|
});
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
describe('sanitizing', () => {
|
describe('sanitizing', () => {
|
||||||
itAsync(
|
itAsync(
|
||||||
'should escape unsafe attributes',
|
'should escape unsafe attributes',
|
||||||
(tcb: TestComponentBuilder, async: any /** TODO #???? */) => {
|
(tcb: TestComponentBuilder, async: AsyncTestCompleter) => {
|
||||||
let tpl = `<a [href]="ctxProp">Link Title</a>`;
|
let tpl = `<a [href]="ctxProp">Link Title</a>`;
|
||||||
tcb.overrideView(SecuredComponent, new ViewMetadata({template: tpl, directives: []}))
|
tcb.overrideView(SecuredComponent, new ViewMetadata({template: tpl, directives: []}))
|
||||||
.createAsync(SecuredComponent)
|
.createAsync(SecuredComponent)
|
||||||
|
@ -144,7 +163,7 @@ function declareTests({useJit}: {useJit: boolean}) {
|
||||||
|
|
||||||
itAsync(
|
itAsync(
|
||||||
'should escape unsafe style values',
|
'should escape unsafe style values',
|
||||||
(tcb: TestComponentBuilder, async: any /** TODO #???? */) => {
|
(tcb: TestComponentBuilder, async: AsyncTestCompleter) => {
|
||||||
let tpl = `<div [style.background]="ctxProp">Text</div>`;
|
let tpl = `<div [style.background]="ctxProp">Text</div>`;
|
||||||
tcb.overrideView(SecuredComponent, new ViewMetadata({template: tpl, directives: []}))
|
tcb.overrideView(SecuredComponent, new ViewMetadata({template: tpl, directives: []}))
|
||||||
.createAsync(SecuredComponent)
|
.createAsync(SecuredComponent)
|
||||||
|
@ -169,7 +188,7 @@ function declareTests({useJit}: {useJit: boolean}) {
|
||||||
|
|
||||||
itAsync(
|
itAsync(
|
||||||
'should escape unsafe HTML values',
|
'should escape unsafe HTML values',
|
||||||
(tcb: TestComponentBuilder, async: any /** TODO #???? */) => {
|
(tcb: TestComponentBuilder, async: AsyncTestCompleter) => {
|
||||||
let tpl = `<div [innerHTML]="ctxProp">Text</div>`;
|
let tpl = `<div [innerHTML]="ctxProp">Text</div>`;
|
||||||
tcb.overrideView(SecuredComponent, new ViewMetadata({template: tpl, directives: []}))
|
tcb.overrideView(SecuredComponent, new ViewMetadata({template: tpl, directives: []}))
|
||||||
.createAsync(SecuredComponent)
|
.createAsync(SecuredComponent)
|
||||||
|
|
|
@ -9,7 +9,9 @@ import {sanitizeUrl} from './url_sanitizer';
|
||||||
export {SecurityContext};
|
export {SecurityContext};
|
||||||
|
|
||||||
|
|
||||||
/** Marker interface for a value that's safe to use in a particular context. */
|
/**
|
||||||
|
* Marker interface for a value that's safe to use in a particular context.
|
||||||
|
*/
|
||||||
export interface SafeValue {}
|
export interface SafeValue {}
|
||||||
/** Marker interface for a value that's safe to use as HTML. */
|
/** Marker interface for a value that's safe to use as HTML. */
|
||||||
export interface SafeHtml extends SafeValue {}
|
export interface SafeHtml extends SafeValue {}
|
||||||
|
@ -151,7 +153,12 @@ abstract class SafeValueImpl implements SafeValue {
|
||||||
constructor(public changingThisBreaksApplicationSecurity: string) {
|
constructor(public changingThisBreaksApplicationSecurity: string) {
|
||||||
// empty
|
// empty
|
||||||
}
|
}
|
||||||
|
|
||||||
abstract getTypeName(): string;
|
abstract getTypeName(): string;
|
||||||
|
|
||||||
|
toString() {
|
||||||
|
return `SafeValue must use [property]=binding: ${this.changingThisBreaksApplicationSecurity}`;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
class SafeHtmlImpl extends SafeValueImpl implements SafeHtml {
|
class SafeHtmlImpl extends SafeValueImpl implements SafeHtml {
|
||||||
|
|
Loading…
Reference in New Issue