diff --git a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts index fb6f2bea5c2..b1bac73b818 100644 --- a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts +++ b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts @@ -8278,34 +8278,6 @@ runInEachFileSystem((os: string) => { expect(trim(jsContents)).toContain(trim(hostBindingsFn)); }); - it('should generate sanitizers for URL properties in SVG script fn in Component', () => { - env.write( - 'test.ts', - ` - import {Component} from '@angular/core'; - - @Component({ - selector: 'test-cmp', - template: \` - - - - \`, - }) - export class TestCmp { - attr = './script.js'; - } - `, - ); - - env.driveMain(); - - const jsContents = env.getContents('test.js'); - expect(jsContents).toContain( - 'i0.ɵɵattribute("href", ctx.attr, i0.ɵɵsanitizeResourceUrl, "xlink")("href", ctx.attr, i0.ɵɵsanitizeResourceUrl);', - ); - }); - it('should not generate sanitizers for URL properties in hostBindings fn in Component', () => { env.write( `test.ts`, diff --git a/packages/compiler/src/schema/dom_security_schema.ts b/packages/compiler/src/schema/dom_security_schema.ts index 11c7a305fc0..21f389854b3 100644 --- a/packages/compiler/src/schema/dom_security_schema.ts +++ b/packages/compiler/src/schema/dom_security_schema.ts @@ -115,12 +115,6 @@ export function SECURITY_SCHEMA(): {[k: string]: SecurityContext} { ['object', ['codebase', 'data']], ]); - // The below are for Script SVG - // See: https://developer.mozilla.org/en-US/docs/Web/API/SVGScriptElement/href - registerContext(SecurityContext.RESOURCE_URL, SVG_NAMESPACE, [ - ['script', ['src', 'href', 'xlink:href']], - ]); - // Keep this in sync with SECURITY_SENSITIVE_ELEMENTS in packages/core/src/sanitization/sanitization.ts // Unknown is the internal tag name for unknown elements example used for host-bindings. // These are unsafe as `attributeName` can be `href` or `xlink:href` diff --git a/packages/compiler/src/template_parser/template_preparser.ts b/packages/compiler/src/template_parser/template_preparser.ts index 5097352081c..2c5889cae6c 100644 --- a/packages/compiler/src/template_parser/template_preparser.ts +++ b/packages/compiler/src/template_parser/template_preparser.ts @@ -14,8 +14,8 @@ const LINK_ELEMENT = 'link'; const LINK_STYLE_REL_ATTR = 'rel'; const LINK_STYLE_HREF_ATTR = 'href'; const LINK_STYLE_REL_VALUE = 'stylesheet'; -const STYLE_ELEMENT = 'style'; -const SCRIPT_ELEMENT = 'script'; +const STYLE_ELEMENTS: ReadonlySet = new Set([':svg:style', 'style']); +const SCRIPT_ELEMENTS: ReadonlySet = new Set([':svg:script', 'script']); const NG_NON_BINDABLE_ATTR = 'ngNonBindable'; const NG_PROJECT_AS = 'ngProjectAs'; @@ -25,7 +25,8 @@ export function preparseElement(ast: html.Element): PreparsedElement { let relAttr: string | null = null; let nonBindable = false; let projectAs = ''; - ast.attrs.forEach((attr) => { + + for (const attr of ast.attrs) { const lcAttrName = attr.name.toLowerCase(); if (lcAttrName == NG_CONTENT_SELECT_ATTR) { selectAttr = attr.value; @@ -40,15 +41,18 @@ export function preparseElement(ast: html.Element): PreparsedElement { projectAs = attr.value; } } - }); - selectAttr = normalizeNgContentSelect(selectAttr); + } + + // Normalize selector to '*' if empty + selectAttr ||= '*'; + const nodeName = ast.name.toLowerCase(); let type = PreparsedElementType.OTHER; if (isNgContent(nodeName)) { type = PreparsedElementType.NG_CONTENT; - } else if (nodeName == STYLE_ELEMENT) { + } else if (STYLE_ELEMENTS.has(nodeName)) { type = PreparsedElementType.STYLE; - } else if (nodeName == SCRIPT_ELEMENT) { + } else if (SCRIPT_ELEMENTS.has(nodeName)) { type = PreparsedElementType.SCRIPT; } else if (nodeName == LINK_ELEMENT && relAttr == LINK_STYLE_REL_VALUE) { type = PreparsedElementType.STYLESHEET; @@ -73,10 +77,3 @@ export class PreparsedElement { public projectAs: string, ) {} } - -function normalizeNgContentSelect(selectAttr: string | null): string { - if (selectAttr === null || selectAttr.length === 0) { - return '*'; - } - return selectAttr; -} diff --git a/packages/core/src/sanitization/sanitization.ts b/packages/core/src/sanitization/sanitization.ts index 0f628f3def4..c817a789412 100644 --- a/packages/core/src/sanitization/sanitization.ts +++ b/packages/core/src/sanitization/sanitization.ts @@ -219,8 +219,7 @@ const RESOURCE_MAP: Record | undefined> 'frame': {'src': true}, 'iframe': {'src': true}, 'media': {'src': true}, - 'script': {'src': true, 'href': true, 'xlink:href': true}, - ':svg:script': {'src': true, 'href': true, 'xlink:href': true}, + 'base': {'href': true}, 'link': {'href': true}, 'object': {'data': true, 'codebase': true}, diff --git a/packages/core/test/acceptance/security_spec.ts b/packages/core/test/acceptance/security_spec.ts index 58e2070a61d..fa02893c741 100644 --- a/packages/core/test/acceptance/security_spec.ts +++ b/packages/core/test/acceptance/security_spec.ts @@ -8,8 +8,11 @@ import {NgIf} from '@angular/common'; import { + ChangeDetectionStrategy, Component, + createComponent, Directive, + EnvironmentInjector, inject, TemplateRef, Type, @@ -838,3 +841,64 @@ describe('innerHTML processing', () => { expect(fixture.nativeElement.innerHTML).not.toContain('action'); }); }); +describe('Component host element validation', () => { + it('should throw an error when dynamically mounting a component onto a script tag', () => { + @Component({ + selector: 'my-sink', + template: '', + }) + class MySink {} + + const scriptHost = document.createElement('script'); + document.head.appendChild(scriptHost); + + try { + const environmentInjector = TestBed.inject(EnvironmentInjector); + expect(() => { + createComponent(MySink, { + environmentInjector, + hostElement: scriptHost, + }); + }).toThrowError(/"`, + changeDetection: ChangeDetectionStrategy.Default, + }) + class TestCmp {} + + const fixture = TestBed.createComponent(TestCmp); + fixture.detectChanges(); + expect(fixture.nativeElement.querySelector('script')).toBeFalsy(); + }); +}); diff --git a/packages/core/test/sanitization/sanitization_spec.ts b/packages/core/test/sanitization/sanitization_spec.ts index e0fb907c4a2..4ba8a49e4df 100644 --- a/packages/core/test/sanitization/sanitization_spec.ts +++ b/packages/core/test/sanitization/sanitization_spec.ts @@ -117,7 +117,7 @@ describe('sanitization', () => { [SecurityContext.RESOURCE_URL, ɵɵsanitizeResourceUrl], ]); Object.entries(schema).forEach(([key, context]) => { - if (context === SecurityContext.URL || SecurityContext.RESOURCE_URL) { + if (context === SecurityContext.URL || context === SecurityContext.RESOURCE_URL) { const [tag, prop] = key.split('|'); const contexts = contextsByProp.get(prop) || new Set(); contexts.add(context); @@ -132,6 +132,31 @@ describe('sanitization', () => { }); }); + it('should select URL sanitizer case-insensitively', () => { + expect(getUrlSanitizer('IFRAME', 'SRC')).toEqual(ɵɵsanitizeResourceUrl); + expect(getUrlSanitizer('IFRAME', 'src')).toEqual(ɵɵsanitizeResourceUrl); + expect(getUrlSanitizer('iframe', 'SRC')).toEqual(ɵɵsanitizeResourceUrl); + expect(getUrlSanitizer('ScRiPt', 'xLiNk:HrEf')).toEqual(ɵɵsanitizeUrl); + expect(getUrlSanitizer('A', 'HREF')).toEqual(ɵɵsanitizeUrl); + }); + + it('should sanitize URL or ResourceURL case-insensitively', () => { + const ERROR = /NG0904: unsafe value used in a resource URL context.*/; + + expect(() => ɵɵsanitizeUrlOrResourceUrl('http://server', 'IFRAME', 'SRC')).toThrowError(ERROR); + + expect(() => ɵɵsanitizeUrlOrResourceUrl('http://server', 'IFRAME', 'src')).toThrowError(ERROR); + + expect(() => ɵɵsanitizeUrlOrResourceUrl('http://server', 'iframe', 'SRC')).toThrowError(ERROR); + + expect(ɵɵsanitizeUrlOrResourceUrl('javascript:true', 'ScRiPt', 'xLiNk:HrEf')).toEqual( + 'unsafe:javascript:true', + ); + + expect(ɵɵsanitizeUrlOrResourceUrl('javascript:true', 'A', 'HREF')).toEqual( + 'unsafe:javascript:true', + ); + }); it('should sanitize resourceUrls via sanitizeUrlOrResourceUrl', () => { const ERROR = /NG0904: unsafe value used in a resource URL context.*/; expect(() => ɵɵsanitizeUrlOrResourceUrl('http://server', 'iframe', 'src')).toThrowError(ERROR);