From c8a99ef4588ee6f3ca14c8c8909d1c4f2062b36b Mon Sep 17 00:00:00 2001
From: Bjarki <bjarki@google.com>
Date: Tue, 3 Nov 2020 16:43:35 +0000
Subject: [PATCH] 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
---
 .../compiler/src/render3/view/i18n/meta.ts    | 11 +++++---
 .../test/linker/security_integration_spec.ts  | 26 +++++++++++++++++++
 2 files changed, 34 insertions(+), 3 deletions(-)

diff --git a/packages/compiler/src/render3/view/i18n/meta.ts b/packages/compiler/src/render3/view/i18n/meta.ts
index 6c93cbebf3..ebc8681aa9 100644
--- 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
index 73ee5b5765..65fb81296a 100644
--- 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./);
+      });
+    });
   });
 }