Skip to content

Commit c8a99ef

Browse files
bjarklerAndrewKushnir
authored andcommittedNov 23, 2020
fix(compiler): disallow i18n of security-sensitive attributes (#39554)
To minimize security risk (XSS in particular) in the i18n pipeline, disallow i18n translation of attributes that are Trusted Types sinks. Add integration tests to ensure that such sinks cannot be translated. PR Close #39554
1 parent bb70a9b commit c8a99ef

File tree

2 files changed

+34
-3
lines changed

2 files changed

+34
-3
lines changed
 

‎packages/compiler/src/render3/view/i18n/meta.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import * as html from '../../../ml_parser/ast';
1414
import {DEFAULT_INTERPOLATION_CONFIG, InterpolationConfig} from '../../../ml_parser/interpolation_config';
1515
import {ParseTreeResult} from '../../../ml_parser/parser';
1616
import * as o from '../../../output/output_ast';
17+
import {isTrustedTypesSink} from '../../../schema/trusted_types_sinks';
1718

1819
import {hasI18nAttrs, I18N_ATTR, I18N_ATTR_PREFIX, icuFromI18nMessage} from './util';
1920

@@ -90,9 +91,13 @@ export class I18nMetaVisitor implements html.Visitor {
9091

9192
} else if (attr.name.startsWith(I18N_ATTR_PREFIX)) {
9293
// 'i18n-*' attributes
93-
const key = attr.name.slice(I18N_ATTR_PREFIX.length);
94-
attrsMeta[key] = attr.value;
95-
94+
const name = attr.name.slice(I18N_ATTR_PREFIX.length);
95+
if (isTrustedTypesSink(element.name, name)) {
96+
this._reportError(
97+
attr, `Translating attribute '${name}' is disallowed for security reasons.`);
98+
} else {
99+
attrsMeta[name] = attr.value;
100+
}
96101
} else {
97102
// non-i18n attributes
98103
attrs.push(attr);

‎packages/core/test/linker/security_integration_spec.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -283,5 +283,31 @@ function declareTests(config?: {useJit: boolean}) {
283283
expect(e.innerHTML).toEqual('also evil');
284284
});
285285
});
286+
287+
onlyInIvy('Trusted Types are only supported in Ivy').describe('translation', () => {
288+
it('should throw error on security-sensitive attributes with constant values', () => {
289+
const template = `<iframe srcdoc="foo" i18n-srcdoc></iframe>`;
290+
TestBed.overrideComponent(SecuredComponent, {set: {template}});
291+
292+
expect(() => TestBed.createComponent(SecuredComponent))
293+
.toThrowError(/Translating attribute 'srcdoc' is disallowed for security reasons./);
294+
});
295+
296+
it('should throw error on security-sensitive attributes with interpolated values', () => {
297+
const template = `<object i18n-data data="foo{{bar}}baz"></object>`;
298+
TestBed.overrideComponent(SecuredComponent, {set: {template}});
299+
300+
expect(() => TestBed.createComponent(SecuredComponent))
301+
.toThrowError(/Translating attribute 'data' is disallowed for security reasons./);
302+
});
303+
304+
it('should throw error on security-sensitive attributes with bound values', () => {
305+
const template = `<div [innerHTML]="foo" i18n-innerHTML></div>`;
306+
TestBed.overrideComponent(SecuredComponent, {set: {template}});
307+
308+
expect(() => TestBed.createComponent(SecuredComponent))
309+
.toThrowError(/Translating attribute 'innerHTML' is disallowed for security reasons./);
310+
});
311+
});
286312
});
287313
}

0 commit comments

Comments
 (0)
Please sign in to comment.