Skip to content

Commit

Permalink
refactor(core): express trusted constants with tagged template litera…
Browse files Browse the repository at this point in the history
…ls (#40082)

The trustConstantHtml and trustConstantResourceUrl functions are only
meant to be passed constant strings extracted from Angular application
templates, as passing other strings or variables could introduce XSS
vulnerabilities.

To better protect these APIs, turn them into template tags. This makes
it possible to assert that the associated template literals do not
contain any interpolation, and thus must be constant.

Also add tests for the change to prevent regression.

PR Close #40082
  • Loading branch information
bjarkler authored and josephperrott committed Jan 5, 2021
1 parent 8cdfd77 commit 6a9d7e5
Show file tree
Hide file tree
Showing 8 changed files with 160 additions and 12 deletions.
Expand Up @@ -555,3 +555,64 @@ export declare class MyModule {
static ɵinj: i0.ɵɵInjectorDef<MyModule>;
}

/****************************************************************************************************
* PARTIAL FILE: security_sensitive_constant_attributes.js
****************************************************************************************************/
import { Component, NgModule } from '@angular/core';
import * as i0 from "@angular/core";
export class MyComponent {
}
MyComponent.ɵfac = function MyComponent_Factory(t) { return new (t || MyComponent)(); };
MyComponent.ɵcmp = i0.ɵɵngDeclareComponent({ version: "0.0.0-PLACEHOLDER", type: MyComponent, selector: "my-component", ngImport: i0, template: { source: `
<!-- A couple of security-sensitive attributes with constant values -->
<embed src="https://angular.io/" />
<iframe srcdoc="<h1>Angular</h1>"></iframe>
<object data="https://angular.io/" codebase="/"></object>
<!-- Repeated element to make sure attribute deduplication works properly -->
<embed src="https://angular.io/" />
<!-- Another element with a src attribute that is not security sensitive -->
<img src="https://angular.io/" />
`, isInline: true } });
(function () { (typeof ngDevMode === "undefined" || ngDevMode) && i0.ɵsetClassMetadata(MyComponent, [{
type: Component,
args: [{
selector: 'my-component',
template: `
<!-- A couple of security-sensitive attributes with constant values -->
<embed src="https://angular.io/" />
<iframe srcdoc="<h1>Angular</h1>"></iframe>
<object data="https://angular.io/" codebase="/"></object>
<!-- Repeated element to make sure attribute deduplication works properly -->
<embed src="https://angular.io/" />
<!-- Another element with a src attribute that is not security sensitive -->
<img src="https://angular.io/" />
`
}]
}], null, null); })();
export class MyModule {
}
MyModule.ɵmod = i0.ɵɵdefineNgModule({ type: MyModule });
MyModule.ɵinj = i0.ɵɵdefineInjector({ factory: function MyModule_Factory(t) { return new (t || MyModule)(); } });
(function () { (typeof ngJitMode === "undefined" || ngJitMode) && i0.ɵɵsetNgModuleScope(MyModule, { declarations: [MyComponent] }); })();
(function () { (typeof ngDevMode === "undefined" || ngDevMode) && i0.ɵsetClassMetadata(MyModule, [{
type: NgModule,
args: [{ declarations: [MyComponent] }]
}], null, null); })();

/****************************************************************************************************
* PARTIAL FILE: security_sensitive_constant_attributes.d.ts
****************************************************************************************************/
import * as i0 from "@angular/core";
export declare class MyComponent {
static ɵfac: i0.ɵɵFactoryDef<MyComponent, never>;
static ɵcmp: i0.ɵɵComponentDefWithMeta<MyComponent, "my-component", never, {}, {}, never, never>;
}
export declare class MyModule {
static ɵmod: i0.ɵɵNgModuleDefWithMeta<MyModule, [typeof MyComponent], never, never>;
static ɵinj: i0.ɵɵInjectorDef<MyModule>;
}

Expand Up @@ -226,6 +226,17 @@
"failureMessage": "Incorrect generated template."
}
]
},
{
"description": "should specify security-sensitive constant attributes as template literals",
"inputFiles": [
"security_sensitive_constant_attributes.ts"
],
"expectations": [
{
"failureMessage": "Incorrect generated template."
}
]
}
]
}
Expand Up @@ -12,4 +12,4 @@ export class MyComponent {

@NgModule({declarations: [MyComponent]})
export class MyModule {
}
}
@@ -0,0 +1,16 @@
consts: [
["src", i0.ɵɵtrustConstantResourceUrl `https://angular.io/`],
["srcdoc", i0.ɵɵtrustConstantHtml `<h1>Angular</h1>`],
["data", i0.ɵɵtrustConstantResourceUrl `https://angular.io/`, "codebase", i0.ɵɵtrustConstantResourceUrl `/`],
["src", "https://angular.io/"]
],
template: function MyComponent_Template(rf, ctx) {
if (rf & 1) {
$r3$.ɵɵelement(0, "embed", 0);
$r3$.ɵɵelement(1, "iframe", 1);
$r3$.ɵɵelement(2, "object", 2);
$r3$.ɵɵelement(3, "embed", 0);
$r3$.ɵɵelement(4, "img", 3);
}
}
@@ -0,0 +1,23 @@
import {Component, NgModule} from '@angular/core';

@Component({
selector: 'my-component',
template: `
<!-- A couple of security-sensitive attributes with constant values -->
<embed src="https://angular.io/" />
<iframe srcdoc="<h1>Angular</h1>"></iframe>
<object data="https://angular.io/" codebase="/"></object>
<!-- Repeated element to make sure attribute deduplication works properly -->
<embed src="https://angular.io/" />
<!-- Another element with a src attribute that is not security sensitive -->
<img src="https://angular.io/" />
`
})
export class MyComponent {
}

@NgModule({declarations: [MyComponent]})
export class MyModule {
}
10 changes: 8 additions & 2 deletions packages/compiler/src/render3/view/template.ts
Expand Up @@ -2193,10 +2193,16 @@ function trustedConstAttribute(tagName: string, attr: t.TextAttribute): o.Expres
if (isTrustedTypesSink(tagName, attr.name)) {
switch (elementRegistry.securityContext(tagName, attr.name, /* isAttribute */ true)) {
case core.SecurityContext.HTML:
return o.importExpr(R3.trustConstantHtml).callFn([value], attr.valueSpan);
return o.taggedTemplate(
o.importExpr(R3.trustConstantHtml),
new o.TemplateLiteral([new o.TemplateLiteralElement(attr.value)], []), undefined,
attr.valueSpan);
// NB: no SecurityContext.SCRIPT here, as the corresponding tags are stripped by the compiler.
case core.SecurityContext.RESOURCE_URL:
return o.importExpr(R3.trustConstantResourceUrl).callFn([value], attr.valueSpan);
return o.taggedTemplate(
o.importExpr(R3.trustConstantResourceUrl),
new o.TemplateLiteral([new o.TemplateLiteralElement(attr.value)], []), undefined,
attr.valueSpan);
default:
return value;
}
Expand Down
38 changes: 30 additions & 8 deletions packages/core/src/sanitization/sanitization.ts
Expand Up @@ -145,8 +145,10 @@ export function ɵɵsanitizeScript(unsafeScript: any): TrustedScript|string {
}

/**
* Promotes the given constant string to a TrustedHTML.
* @param html constant string containing trusted HTML.
* A template tag function for promoting the associated constant literal to a
* TrustedHTML. Interpolation is explicitly not allowed.
*
* @param html constant template literal containing trusted HTML.
* @returns TrustedHTML wrapping `html`.
*
* @security This is a security-sensitive function and should only be used to
Expand All @@ -155,13 +157,24 @@ export function ɵɵsanitizeScript(unsafeScript: any): TrustedScript|string {
*
* @codeGenApi
*/
export function ɵɵtrustConstantHtml(html: string): TrustedHTML|string {
return trustedHTMLFromString(html);
export function ɵɵtrustConstantHtml(html: TemplateStringsArray): TrustedHTML|string {
// The following runtime check ensures that the function was called as a
// template tag (e.g. ɵɵtrustConstantHtml`content`), without any interpolation
// (e.g. not ɵɵtrustConstantHtml`content ${variable}`). A TemplateStringsArray
// is an array with a `raw` property that is also an array. The associated
// template literal has no interpolation if and only if the length of the
// TemplateStringsArray is 1.
if (ngDevMode && (!Array.isArray(html) || !Array.isArray(html.raw) || html.length !== 1)) {
throw new Error(`Unexpected interpolation in trusted HTML constant: ${html.join('?')}`);
}
return trustedHTMLFromString(html[0]);
}

/**
* Promotes the given constant string to a TrustedScriptURL.
* @param url constant string containing a trusted script URL.
* A template tag function for promoting the associated constant literal to a
* TrustedScriptURL. Interpolation is explicitly not allowed.
*
* @param url constant template literal containing a trusted script URL.
* @returns TrustedScriptURL wrapping `url`.
*
* @security This is a security-sensitive function and should only be used to
Expand All @@ -170,8 +183,17 @@ export function ɵɵtrustConstantHtml(html: string): TrustedHTML|string {
*
* @codeGenApi
*/
export function ɵɵtrustConstantResourceUrl(url: string): TrustedScriptURL|string {
return trustedScriptURLFromString(url);
export function ɵɵtrustConstantResourceUrl(url: TemplateStringsArray): TrustedScriptURL|string {
// The following runtime check ensures that the function was called as a
// template tag (e.g. ɵɵtrustConstantResourceUrl`content`), without any
// interpolation (e.g. not ɵɵtrustConstantResourceUrl`content ${variable}`). A
// TemplateStringsArray is an array with a `raw` property that is also an
// array. The associated template literal has no interpolation if and only if
// the length of the TemplateStringsArray is 1.
if (ngDevMode && (!Array.isArray(url) || !Array.isArray(url.raw) || url.length !== 1)) {
throw new Error(`Unexpected interpolation in trusted URL constant: ${url.join('?')}`);
}
return trustedScriptURLFromString(url[0]);
}

/**
Expand Down
11 changes: 10 additions & 1 deletion packages/core/test/sanitization/sanitization_spec.ts
Expand Up @@ -12,7 +12,7 @@ import {LView} from '@angular/core/src/render3/interfaces/view';
import {enterView, leaveView} from '@angular/core/src/render3/state';

import {bypassSanitizationTrustHtml, bypassSanitizationTrustResourceUrl, bypassSanitizationTrustScript, bypassSanitizationTrustStyle, bypassSanitizationTrustUrl} from '../../src/sanitization/bypass';
import {getUrlSanitizer, ɵɵsanitizeHtml, ɵɵsanitizeResourceUrl, ɵɵsanitizeScript, ɵɵsanitizeStyle, ɵɵsanitizeUrl, ɵɵsanitizeUrlOrResourceUrl} from '../../src/sanitization/sanitization';
import {getUrlSanitizer, ɵɵsanitizeHtml, ɵɵsanitizeResourceUrl, ɵɵsanitizeScript, ɵɵsanitizeStyle, ɵɵsanitizeUrl, ɵɵsanitizeUrlOrResourceUrl, ɵɵtrustConstantHtml, ɵɵtrustConstantResourceUrl} from '../../src/sanitization/sanitization';
import {SecurityContext} from '../../src/sanitization/security';

function fakeLView(): LView {
Expand Down Expand Up @@ -134,4 +134,13 @@ describe('sanitization', () => {
expect(ɵɵsanitizeUrlOrResourceUrl(bypassSanitizationTrustUrl('javascript:true'), 'a', 'href'))
.toEqual('javascript:true');
});

it('should only trust constant strings from template literal tags without interpolation', () => {
expect(ɵɵtrustConstantHtml`<h1>good</h1>`.toString()).toEqual('<h1>good</h1>');
expect(ɵɵtrustConstantResourceUrl`http://good.com`.toString()).toEqual('http://good.com');
expect(() => (ɵɵtrustConstantHtml as any) `<h1>${'evil'}</h1>`)
.toThrowError(/Unexpected interpolation in trusted HTML constant/);
expect(() => (ɵɵtrustConstantResourceUrl as any) `http://${'evil'}.com`)
.toThrowError(/Unexpected interpolation in trusted URL constant/);
});
});

0 comments on commit 6a9d7e5

Please sign in to comment.