From 1f013ee2de1238a15ad76f03bc4c76ed834f355d Mon Sep 17 00:00:00 2001 From: Andrew Kushnir Date: Tue, 25 Oct 2022 14:45:32 -0700 Subject: [PATCH] fix(core): hardening attribute and property binding rules for + \` + }) + export class SomeComponent {} + `); + + env.driveMain(); + const jsContents = env.getContents('test.js'); + + // Only `sandbox` has an extra validation fn (since it's security-sensitive), + // the `title` property doesn't have an extra validation fn. + expect(jsContents) + .toContain( + 'ɵɵproperty("sandbox", "", i0.ɵɵvalidateIframeAttribute)("title", "Hi!")'); + + // The `allow` property is also security-sensitive, thus an extra validation fn. + expect(jsContents).toContain('ɵɵattribute("allow", "", i0.ɵɵvalidateIframeAttribute)'); + }); + + it('should generate an attribute binding instruction with a validator function ' + + '(making sure it\'s case-insensitive, since this is allowed in Angular templates)', + () => { + env.write('test.ts', ` + import {Component} from '@angular/core'; + + @Component({ + template: \` + + \` + }) + export class SomeComponent {} + `); + + env.driveMain(); + const jsContents = env.getContents('test.js'); + + // Make sure that the `sandbox` has an extra validation fn, + // and the check is case-insensitive (since the `setAttribute` DOM API + // is case-insensitive as well). + expect(jsContents).toContain('ɵɵattribute("SANDBOX", "", i0.ɵɵvalidateIframeAttribute)'); + }); + + it('should *not* generate a validator fn for attribute and property bindings when *not* on `, + }) + class IframeComp { + } + + expectIframeToBeCreated(IframeComp, {[srcAttr]: TEST_IFRAME_URL}); + }); + + it(`should work when a security-sensitive attribute is set ` + + `as a static attribute (checking \`${securityAttr}\` and ` + + `making sure it's case-insensitive)`, + () => { + @Component({ + standalone: true, + selector: 'my-comp', + template: ` + `, + }) + class IframeComp { + } + + expectIframeToBeCreated(IframeComp, {[srcAttr]: TEST_IFRAME_URL}); + }); + + it(`should error when a security-sensitive attribute is applied ` + + `using a property binding (checking \`${securityAttr}\`)`, + () => { + @Component({ + standalone: true, + selector: 'my-comp', + template: + ``, + }) + class IframeComp { + } + + expectIframeCreationToFail(IframeComp); + }); + + it(`should error when a security-sensitive attribute is applied ` + + `using a property interpolation (checking \`${securityAttr}\`)`, + () => { + @Component({ + standalone: true, + selector: 'my-comp', + template: + ``, + }) + class IframeComp { + } + + expectIframeCreationToFail(IframeComp); + }); + + it(`should error when a security-sensitive attribute is applied ` + + `using a property binding (checking \`${securityAttr}\`, making ` + + `sure it's case-insensitive)`, + () => { + @Component({ + standalone: true, + selector: 'my-comp', + template: ` + + `, + }) + class IframeComp { + } + + expectIframeCreationToFail(IframeComp); + }); + + it(`should error when a security-sensitive attribute is applied ` + + `using a property binding (checking \`${securityAttr}\`)`, + () => { + @Component({ + standalone: true, + selector: 'my-comp', + template: ` + + `, + }) + class IframeComp { + } + + expectIframeCreationToFail(IframeComp); + }); + + it(`should error when a security-sensitive attribute is applied ` + + `using a property binding (checking \`${securityAttr}\`, making ` + + `sure it's case-insensitive)`, + () => { + @Component({ + standalone: true, + selector: 'my-comp', + template: ` + + `, + }) + class IframeComp { + } + + expectIframeCreationToFail(IframeComp); + }); + + it(`should allow changing \`${srcAttr}\` after initial render`, () => { + @Component({ + standalone: true, + selector: 'my-comp', + template: ` + + `, + }) + class IframeComp { + private sanitizer = inject(DomSanitizer); + src = this.sanitizeFn(TEST_IFRAME_URL); + + get sanitizeFn() { + return srcAttr === 'src' ? this.sanitizer.bypassSecurityTrustResourceUrl : + this.sanitizer.bypassSecurityTrustHtml; + } + } + + const fixture = expectIframeToBeCreated(IframeComp, {[srcAttr]: TEST_IFRAME_URL}); + const component = fixture.componentInstance; + + // Changing `src` or `srcdoc` is allowed. + const newUrl = 'https://angular.io/about?group=Angular'; + component.src = component.sanitizeFn(newUrl); + expect(() => fixture.detectChanges()).not.toThrow(); + expect(fixture.nativeElement.querySelector('iframe')[srcAttr]).toEqual(newUrl); + }); + }); + }); + + it('should work when a directive sets a security-sensitive attribute as a static attribute', + () => { + @Directive({ + standalone: true, + selector: '[dir]', + host: { + 'src': TEST_IFRAME_URL, + 'sandbox': '', + }, + }) + class IframeDir { + } + @Component({ + standalone: true, + imports: [IframeDir], + selector: 'my-comp', + template: '', + }) + class IframeComp { + } + + expectIframeToBeCreated(IframeComp, {src: TEST_IFRAME_URL}); + }); + + it('should work when a directive sets a security-sensitive host attribute on a non-iframe element', + () => { + @Directive({ + standalone: true, + selector: '[dir]', + host: { + 'src': TEST_IFRAME_URL, + 'sandbox': '', + }, + }) + class Dir { + } + + @Component({ + standalone: true, + imports: [Dir], + selector: 'my-comp', + template: '', + }) + class NonIframeComp { + } + + const fixture = TestBed.createComponent(NonIframeComp); + fixture.detectChanges(); + + expect(fixture.nativeElement.firstChild.src).toEqual(TEST_IFRAME_URL); + }); + + + it('should work when a security-sensitive attribute on an `, + }) + class IframeComp { + visible = true; + } + + expectIframeToBeCreated(IframeComp, {src: TEST_IFRAME_URL}); + }); + + it('should work when a security-sensitive attribute is set between `src` and `srcdoc`', + () => { + @Component({ + standalone: true, + selector: 'my-comp', + template: ``, + }) + class IframeComp { + } + + expectIframeToBeCreated(IframeComp, {src: TEST_IFRAME_URL}); + }); + + it('should work when a directive sets a security-sensitive attribute before setting `src`', + () => { + @Directive({ + standalone: true, + selector: '[dir]', + host: { + 'sandbox': '', + 'src': TEST_IFRAME_URL, + }, + }) + class IframeDir { + } + + @Component({ + standalone: true, + imports: [IframeDir], + selector: 'my-comp', + template: '', + }) + class IframeComp { + } + + expectIframeToBeCreated(IframeComp, {src: TEST_IFRAME_URL}); + }); + + it('should work when a directive sets an `src` and ' + + 'there was a security-sensitive attribute set in a template' + + '(directive attribute after `sandbox`)', + () => { + @Directive({ + standalone: true, + selector: '[dir]', + host: { + 'src': TEST_IFRAME_URL, + }, + }) + class IframeDir { + } + + @Component({ + standalone: true, + imports: [IframeDir], + selector: 'my-comp', + template: '', + }) + class IframeComp { + } + + expectIframeToBeCreated(IframeComp, {src: TEST_IFRAME_URL}); + }); + + it('should error when a directive sets a security-sensitive attribute ' + + 'as an attribute binding (checking that it\'s case-insensitive)', + () => { + @Directive({ + standalone: true, + selector: '[dir]', + host: { + '[attr.SANDBOX]': '\'\'', + }, + }) + class IframeDir { + } + + @Component({ + standalone: true, + imports: [IframeDir], + selector: 'my-comp', + template: ``, + }) + class IframeComp { + } + + expectIframeCreationToFail(IframeComp); + }); + + it('should work when a directive sets an `src` and ' + + 'there was a security-sensitive attribute set in a template' + + '(directive attribute before `sandbox`)', + () => { + @Directive({ + standalone: true, + selector: '[dir]', + host: { + 'src': TEST_IFRAME_URL, + }, + }) + class IframeDir { + } + + @Component({ + standalone: true, + imports: [IframeDir], + selector: 'my-comp', + template: '', + }) + class IframeComp { + } + + expectIframeToBeCreated(IframeComp, {src: TEST_IFRAME_URL}); + }); + + it('should work when a directive sets a security-sensitive attribute and ' + + 'there was an `src` attribute set in a template' + + '(directive attribute after `src`)', + () => { + @Directive({ + standalone: true, + selector: '[dir]', + host: { + 'sandbox': '', + }, + }) + class IframeDir { + } + + @Component({ + standalone: true, + imports: [IframeDir], + selector: 'my-comp', + template: ``, + }) + class IframeComp { + } + + expectIframeToBeCreated(IframeComp, {src: TEST_IFRAME_URL}); + }); + + it('should work when a security-sensitive attribute is set as a static attribute', () => { + @Component({ + standalone: true, + selector: 'my-comp', + template: ` + + `, + }) + class IframeComp { + } + + expectIframeToBeCreated(IframeComp, { + src: TEST_IFRAME_URL, + referrerPolicy: 'no-referrer', + }); + }); + + it('should error when a security-sensitive attribute is set ' + + 'as a property binding and an + `, + }) + class IframeComp { + } + + expectIframeCreationToFail(IframeComp); + }); + + it('should work when a directive sets a security-sensitive attribute and ' + + 'there was an `src` attribute set in a template' + + '(directive attribute before `src`)', + () => { + @Directive({ + standalone: true, + selector: '[dir]', + host: { + 'sandbox': '', + }, + }) + class IframeDir { + } + + @Component({ + standalone: true, + imports: [IframeDir], + selector: 'my-comp', + template: ``, + }) + class IframeComp { + } + + expectIframeToBeCreated(IframeComp, {src: TEST_IFRAME_URL}); + }); + + it('should work when a directive that sets a security-sensitive attribute goes ' + + 'before the directive that sets an `src` attribute value', + () => { + @Directive({ + standalone: true, + selector: '[set-src]', + host: { + 'src': TEST_IFRAME_URL, + }, + }) + class DirThatSetsSrc { + } + + @Directive({ + standalone: true, + selector: '[set-sandbox]', + host: { + 'sandbox': '', + }, + }) + class DirThatSetsSandbox { + } + + @Component({ + standalone: true, + imports: [DirThatSetsSandbox, DirThatSetsSrc], + selector: 'my-comp', + // Important note: even though the `set-sandbox` goes after the `set-src`, + // the directive matching order (thus the order of host attributes) is + // based on the imports order, so the `sandbox` gets set first and the `src` second. + template: '', + }) + class IframeComp { + } + + expectIframeToBeCreated(IframeComp, {src: TEST_IFRAME_URL}); + }); + + it('should work when a directive that sets a security-sensitive attribute has ' + + 'a host directive that sets an `src` attribute value', + () => { + @Directive({ + standalone: true, + selector: '[set-src-dir]', + host: { + 'src': TEST_IFRAME_URL, + }, + }) + class DirThatSetsSrc { + } + + @Directive({ + standalone: true, + selector: '[dir]', + hostDirectives: [DirThatSetsSrc], + host: { + 'sandbox': '', + }, + }) + class DirThatSetsSandbox { + } + + @Component({ + standalone: true, + imports: [DirThatSetsSandbox], + selector: 'my-comp', + template: '', + }) + class IframeComp { + } + + expectIframeToBeCreated(IframeComp, {src: TEST_IFRAME_URL}); + }); + + it('should work when a directive that sets an `src` has ' + + 'a host directive that sets a security-sensitive attribute value', + () => { + @Directive({ + standalone: true, + selector: '[set-sandbox-dir]', + host: { + 'sandbox': '', + }, + }) + class DirThatSetsSandbox { + } + + @Directive({ + standalone: true, + selector: '[dir]', + hostDirectives: [DirThatSetsSandbox], + host: { + 'src': TEST_IFRAME_URL, + }, + }) + class DirThatSetsSrc { + } + + @Component({ + standalone: true, + imports: [DirThatSetsSrc], + selector: 'my-comp', + template: '', + }) + class IframeComp { + } + + expectIframeToBeCreated(IframeComp, {src: TEST_IFRAME_URL}); + }); + + + it('should error when creating a view that contains an + + `, + }) + class IframeComp { + @ViewChild('container', {read: ViewContainerRef}) container!: ViewContainerRef; + @ViewChild('template') template!: TemplateRef; + + createEmbeddedView() { + this.container.createEmbeddedView(this.template); + } + } + + const fixture = TestBed.createComponent(IframeComp); + fixture.detectChanges(); + + expect(() => { + fixture.componentInstance.createEmbeddedView(); + fixture.detectChanges(); + }).toThrowError(getErrorMessageRegexp()); + + ensureNoIframePresent(fixture); + }); + + describe('i18n', () => { + it('should error when a security-sensitive attribute is set as ' + + 'a property binding on an + + `, + }) + class IframeComp { + } + + expectIframeCreationToFail(IframeComp); + }); + + it('should error when a security-sensitive attribute is set as ' + + 'a property binding on an + `, + }) + class IframeComp { + } + + expectIframeCreationToFail(IframeComp); + }); + + it('should work when a security-sensitive attributes are marked for translation', () => { + @Component({ + standalone: true, + selector: 'my-comp', + template: ` + + `, + }) + class IframeComp { + } + + expectIframeToBeCreated(IframeComp, {src: TEST_IFRAME_URL}); + }); + }); + }); + }); +}); diff --git a/packages/tsec-exemption.json b/packages/tsec-exemption.json index 8bbaaf0d4ed44..6a75acdbe502c 100644 --- a/packages/tsec-exemption.json +++ b/packages/tsec-exemption.json @@ -31,5 +31,8 @@ ], "ban-window-stringfunctiondef": [ "core/src/render3/util/misc_utils.ts" + ], + "ban-iframe-srcdoc-assignments": [ + "core/src/sanitization/iframe_attrs_validation.ts" ] }