Sitelet https://github.com/angular/angular/commit/5ff83c11fe38587b6bf484b4b7435bb57874e41a
Skip to content

Commit 5ff83c1

Browse files
SkyZeroZxNothingEverHappens
SkyZeroZx
authored and
NothingEverHappens
committed
fix(compiler): strip script elements regardless of namespace
Namespace inheritance qualifies script element names, for example :xhtml:script or a custom prefix. The existing prefix allowlist only handles known namespaces, allowing arbitrary and future namespaces to bypass template script stripping and fall back to executable HTML elements. Classify script elements by local name in the template preparser so script stripping applies independently of namespace. (cherry picked from commit 97e6aa4)
1 parent 6fa2b08 commit 5ff83c1

3 files changed

Lines changed: 68 additions & 4 deletions

File tree

‎packages/compiler/src/template_parser/template_preparser.ts‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,15 +7,15 @@
77
*/
88

99
import * as html from '../ml_parser/ast';
10-
import {isNgContent} from '../ml_parser/tags';
10+
import {isNgContent, splitNsName} from '../ml_parser/tags';
1111

1212
const NG_CONTENT_SELECT_ATTR = 'select';
1313
const LINK_ELEMENT = 'link';
1414
const LINK_STYLE_REL_ATTR = 'rel';
1515
const LINK_STYLE_HREF_ATTR = 'href';
1616
const LINK_STYLE_REL_VALUE = 'stylesheet';
1717
const STYLE_ELEMENT = 'style';
18-
const SCRIPT_ELEMENTS: ReadonlySet<string> = new Set([':math:script', ':svg:script', 'script']);
18+
const SCRIPT_ELEMENT = 'script';
1919
const NG_NON_BINDABLE_ATTR = 'ngNonBindable';
2020
const NG_PROJECT_AS = 'ngProjectAs';
2121

@@ -47,12 +47,14 @@ export function preparseElement(ast: html.Element): PreparsedElement {
4747
selectAttr ||= '*';
4848

4949
const nodeName = ast.name.toLowerCase();
50+
// Custom namespaces fall back to HTML, where script elements are executable.
51+
const nodeNameWithoutNamespace = splitNsName(nodeName)[1];
5052
let type = PreparsedElementType.OTHER;
5153
if (isNgContent(nodeName)) {
5254
type = PreparsedElementType.NG_CONTENT;
5355
} else if (STYLE_ELEMENT === nodeName) {
5456
type = PreparsedElementType.STYLE;
55-
} else if (SCRIPT_ELEMENTS.has(nodeName)) {
57+
} else if (SCRIPT_ELEMENT === nodeNameWithoutNamespace) {
5658
type = PreparsedElementType.SCRIPT;
5759
} else if (nodeName == LINK_ELEMENT && relAttr == LINK_STYLE_REL_VALUE) {
5860
type = PreparsedElementType.STYLESHEET;

‎packages/compiler/test/render3/r3_template_transform_spec.ts‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -966,6 +966,17 @@ describe('R3 template transform', () => {
966966
expectFromHtml('<script></script>a').toEqual([['Text', 'a']]);
967967
});
968968

969+
it('should ignore <script> elements inherited into custom namespaces', () => {
970+
expectFromHtml(
971+
'<xhtml:div xmlns:xhtml="http://www.w3.org/1999/xhtml"><script [textContent]="evil"></script><span>safe</span></xhtml:div>',
972+
).toEqual([
973+
['Element', ':xhtml:div'],
974+
['TextAttribute', ':xmlns:xhtml', 'http://www.w3.org/1999/xhtml'],
975+
['Element', ':xhtml:span'],
976+
['Text', 'safe'],
977+
]);
978+
});
979+
969980
it('should ignore <style> elements', () => {
970981
expectFromHtml('<style></style>a').toEqual([['Text', 'a']]);
971982
});
@@ -1036,6 +1047,17 @@ describe('R3 template transform', () => {
10361047
]);
10371048
});
10381049

1050+
it('should ignore custom-namespaced <script> elements inside of elements with ngNonBindable', () => {
1051+
expectFromHtml(
1052+
'<foo:div ngNonBindable><script>evil</script><span>safe</span></foo:div>',
1053+
).toEqual([
1054+
['Element', ':foo:div'],
1055+
['TextAttribute', 'ngNonBindable', ''],
1056+
['Element', ':foo:span'],
1057+
['Text', 'safe'],
1058+
]);
1059+
});
1060+
10391061
it('should ignore <style> elements inside of elements with ngNonBindable', () => {
10401062
expectFromHtml('<div ngNonBindable><style></style>a</div>').toEqual([
10411063
['Element', 'div'],

‎packages/core/test/acceptance/security_spec.ts‎

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1344,7 +1344,7 @@ describe('Component host element validation', () => {
13441344
});
13451345
});
13461346

1347-
describe('SVG <script> bindings', () => {
1347+
describe('<script> elements in templates', () => {
13481348
it(`should remove svg <script> element`, () => {
13491349
@Component({
13501350
template: `<svg><script src="https://bad.com/script.js"></script></svg>`,
@@ -1356,6 +1356,46 @@ describe('SVG <script> bindings', () => {
13561356
fixture.detectChanges();
13571357
expect(fixture.nativeElement.querySelector('script')).toBeFalsy();
13581358
});
1359+
1360+
it('should remove <script> elements inherited into the XHTML namespace', async () => {
1361+
@Component({
1362+
template: `
1363+
<xhtml:div xmlns:xhtml="http://www.w3.org/1999/xhtml">
1364+
<script [textContent]="evil"></script>
1365+
<span>safe</span>
1366+
</xhtml:div>
1367+
`,
1368+
})
1369+
class TestCmp {
1370+
evil = 'evil';
1371+
}
1372+
1373+
const fixture = TestBed.createComponent(TestCmp);
1374+
await fixture.whenStable();
1375+
1376+
expect(fixture.nativeElement.querySelector('script')).toBeFalsy();
1377+
expect(fixture.nativeElement.querySelector('span').textContent).toBe('safe');
1378+
});
1379+
1380+
it('should remove <script> elements inherited into arbitrary custom namespaces', async () => {
1381+
@Component({
1382+
template: `
1383+
<foo:div xmlns:foo="urn:example">
1384+
<script [textContent]="evil"></script>
1385+
<span>custom safe</span>
1386+
</foo:div>
1387+
`,
1388+
})
1389+
class TestCmp {
1390+
evil = 'evil';
1391+
}
1392+
1393+
const fixture = TestBed.createComponent(TestCmp);
1394+
await fixture.whenStable();
1395+
1396+
expect(fixture.nativeElement.querySelector('script')).toBeFalsy();
1397+
expect(fixture.nativeElement.querySelector('span').textContent).toBe('custom safe');
1398+
});
13591399
});
13601400

13611401
describe('SVG <a> link sanitization', () => {

0 commit comments

Comments
 (0)