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
angular · Nov 23, 2020 · c8a99ef · c8a99ef
1 parent bb70a9b
commit c8a99ef
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 3 deletions.
diff --git a/packages/compiler/src/render3/view/i18n/meta.ts b/packages/compiler/src/render3/view/i18n/meta.ts
@@ -14,6 +14,7 @@ import * as html from '../../../ml_parser/ast';
 import {DEFAULT_INTERPOLATION_CONFIG, InterpolationConfig} from '../../../ml_parser/interpolation_config';
 import {ParseTreeResult} from '../../../ml_parser/parser';
 import * as o from '../../../output/output_ast';
+import {isTrustedTypesSink} from '../../../schema/trusted_types_sinks';
 
 import {hasI18nAttrs, I18N_ATTR, I18N_ATTR_PREFIX, icuFromI18nMessage} from './util';
 
@@ -90,9 +91,13 @@ export class I18nMetaVisitor implements html.Visitor {
 
         } else if (attr.name.startsWith(I18N_ATTR_PREFIX)) {
           // 'i18n-*' attributes
-          const key = attr.name.slice(I18N_ATTR_PREFIX.length);
-          attrsMeta[key] = attr.value;
-
+          const name = attr.name.slice(I18N_ATTR_PREFIX.length);
+          if (isTrustedTypesSink(element.name, name)) {
+            this._reportError(
+                attr, `Translating attribute '${name}' is disallowed for security reasons.`);
+          } else {
+            attrsMeta[name] = attr.value;
+          }
         } else {
           // non-i18n attributes
           attrs.push(attr);

diff --git a/packages/core/test/linker/security_integration_spec.ts b/packages/core/test/linker/security_integration_spec.ts
@@ -283,5 +283,31 @@ function declareTests(config?: {useJit: boolean}) {
         expect(e.innerHTML).toEqual('also  evil');
       });
     });
+
+    onlyInIvy('Trusted Types are only supported in Ivy').describe('translation', () => {
+      it('should throw error on security-sensitive attributes with constant values', () => {
+        const template = `<iframe srcdoc="foo" i18n-srcdoc></iframe>`;
+        TestBed.overrideComponent(SecuredComponent, {set: {template}});
+
+        expect(() => TestBed.createComponent(SecuredComponent))
+            .toThrowError(/Translating attribute 'srcdoc' is disallowed for security reasons./);
+      });
+
+      it('should throw error on security-sensitive attributes with interpolated values', () => {
+        const template = `<object i18n-data data="foo{{bar}}baz"></object>`;
+        TestBed.overrideComponent(SecuredComponent, {set: {template}});
+
+        expect(() => TestBed.createComponent(SecuredComponent))
+            .toThrowError(/Translating attribute 'data' is disallowed for security reasons./);
+      });
+
+      it('should throw error on security-sensitive attributes with bound values', () => {
+        const template = `<div [innerHTML]="foo" i18n-innerHTML></div>`;
+        TestBed.overrideComponent(SecuredComponent, {set: {template}});
+
+        expect(() => TestBed.createComponent(SecuredComponent))
+            .toThrowError(/Translating attribute 'innerHTML' is disallowed for security reasons./);
+      });
+    });
   });
 }