Skip to content

Commit 725555b

Browse files
AndrewJakubowiczAndrewJakubowicz
authored
Merge 617aae2 into 2190dfe 
2 parents 2190dfe + 617aae2 commit 725555b

3 files changed

Lines changed: 65 additions & 42 deletions

File tree

‎.changeset/weak-roses-laugh.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
'lit-html': patch
3+
'lit': patch
4+
---
5+
6+
Change the `h` field of `CompiledTemplate`s to a `TemplateStringsArray` preventing the spoofing of `CompiledTemplate`s by JSON injection attacks. This is a breaking change, but necessary as a security fix. Similar to [#2307](https://github.com/lit/lit/pull/2307)

‎packages/lit-html/src/lit-html.ts‎

Lines changed: 41 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -489,7 +489,9 @@ export interface CompiledTemplate extends Omit<Template, 'el'> {
489489
el?: HTMLTemplateElement;
490490

491491
// The prepared HTML string to create a template element from.
492-
h: TrustedHTML;
492+
// The type is a TemplateStringsArray to guarantee that the value came from
493+
// source code, preventing a JSON injection attack.
494+
h: TemplateStringsArray;
493495
}
494496

495497
/**
@@ -652,6 +654,39 @@ export interface DirectiveParent {
652654
__directives?: Array<Directive | undefined>;
653655
}
654656

657+
function trustFromTemplateString(
658+
tsa: TemplateStringsArray,
659+
stringFromTSA: string
660+
): TrustedHTML {
661+
// A security check to prevent spoofing of Lit template results.
662+
// In the future, we may be able to replace this with Array.isTemplateObject,
663+
// though we might need to make that check inside of the html and svg
664+
// functions, because precompiled templates don't come in as
665+
// TemplateStringArray objects.
666+
if (!Array.isArray(tsa) || !tsa.hasOwnProperty('raw')) {
667+
let message = 'invalid template strings array';
668+
if (DEV_MODE) {
669+
message = `
670+
Internal Error: expected template strings to be an array
671+
with a 'raw' field. Faking a template strings array by
672+
calling html or svg like an ordinary function is effectively
673+
the same as calling unsafeHtml and can lead to major security
674+
issues, e.g. opening your code up to XSS attacks.
675+
If you're using the html or svg tagged template functions normally
676+
and still seeing this error, please file a bug at
677+
https://github.com/lit/lit/issues/new?template=bug_report.md
678+
and include information about your build tooling, if any.
679+
`
680+
.trim()
681+
.replace(/\n */g, '\n');
682+
}
683+
throw new Error(message);
684+
}
685+
return policy !== undefined
686+
? policy.createHTML(stringFromTSA)
687+
: (stringFromTSA as unknown as TrustedHTML);
688+
}
689+
655690
/**
656691
* Returns an HTML string for the given TemplateStringsArray and result type
657692
* (HTML or SVG), along with the case-sensitive bound attribute names in
@@ -816,38 +851,8 @@ const getTemplateHtml = (
816851
const htmlResult: string | TrustedHTML =
817852
html + (strings[l] || '<?>') + (type === SVG_RESULT ? '</svg>' : '');
818853

819-
// A security check to prevent spoofing of Lit template results.
820-
// In the future, we may be able to replace this with Array.isTemplateObject,
821-
// though we might need to make that check inside of the html and svg
822-
// functions, because precompiled templates don't come in as
823-
// TemplateStringArray objects.
824-
if (!Array.isArray(strings) || !strings.hasOwnProperty('raw')) {
825-
let message = 'invalid template strings array';
826-
if (DEV_MODE) {
827-
message = `
828-
Internal Error: expected template strings to be an array
829-
with a 'raw' field. Faking a template strings array by
830-
calling html or svg like an ordinary function is effectively
831-
the same as calling unsafeHtml and can lead to major security
832-
issues, e.g. opening your code up to XSS attacks.
833-
834-
If you're using the html or svg tagged template functions normally
835-
and still seeing this error, please file a bug at
836-
https://github.com/lit/lit/issues/new?template=bug_report.md
837-
and include information about your build tooling, if any.
838-
`
839-
.trim()
840-
.replace(/\n */g, '\n');
841-
}
842-
throw new Error(message);
843-
}
844854
// Returned as an array for terseness
845-
return [
846-
policy !== undefined
847-
? policy.createHTML(htmlResult)
848-
: (htmlResult as unknown as TrustedHTML),
849-
attrNames,
850-
];
855+
return [trustFromTemplateString(strings, htmlResult), attrNames];
851856
};
852857

853858
/** @internal */
@@ -1517,7 +1522,10 @@ class ChildPart implements Disconnectable {
15171522
typeof type === 'number'
15181523
? this._$getTemplate(result as TemplateResult)
15191524
: (type.el === undefined &&
1520-
(type.el = Template.createElement(type.h, this.options)),
1525+
(type.el = Template.createElement(
1526+
trustFromTemplateString(type.h, type.h[0]),
1527+
this.options
1528+
)),
15211529
type);
15221530

15231531
if ((this._$committedValue as TemplateInstance)?._$template === template) {

‎packages/lit-html/src/test/lit-html_test.ts‎

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1590,16 +1590,12 @@ suite('lit-html', () => {
15901590
});
15911591

15921592
suite('compiled', () => {
1593-
const trustedTypes = (globalThis as unknown as Partial<Window>)
1594-
.trustedTypes;
1595-
const policy = trustedTypes?.createPolicy('lit-html', {
1596-
createHTML: (s) => s,
1597-
}) ?? {createHTML: (s) => s as unknown as TrustedHTML};
1593+
const branding_tag = (s: TemplateStringsArray) => s;
15981594

15991595
test('only text', () => {
16001596
// A compiled template for html`${'A'}`
16011597
const _$lit_template_1: CompiledTemplate = {
1602-
h: policy.createHTML('<!---->'),
1598+
h: branding_tag`<!---->`,
16031599
parts: [{type: 2, index: 0}],
16041600
};
16051601
assertRender(
@@ -1615,7 +1611,7 @@ suite('lit-html', () => {
16151611
test('text expression', () => {
16161612
// A compiled template for html`<div>${'A'}</div>`
16171613
const _$lit_template_1: CompiledTemplate = {
1618-
h: policy.createHTML(`<div><!----></div>`),
1614+
h: branding_tag`<div><!----></div>`,
16191615
parts: [{type: 2, index: 1}],
16201616
};
16211617
const result = {
@@ -1629,7 +1625,7 @@ suite('lit-html', () => {
16291625
test('attribute expression', () => {
16301626
// A compiled template for html`<div foo=${'A'}></div>`
16311627
const _$lit_template_1: CompiledTemplate = {
1632-
h: policy.createHTML('<div></div>'),
1628+
h: branding_tag`<div></div>`,
16331629
parts: [
16341630
{
16351631
type: 1,
@@ -1652,7 +1648,7 @@ suite('lit-html', () => {
16521648
const r = createRef();
16531649
// A compiled template for html`<div ${ref(r)}></div>`
16541650
const _$lit_template_1: CompiledTemplate = {
1655-
h: policy.createHTML('<div></div>'),
1651+
h: branding_tag`<div></div>`,
16561652
parts: [{type: 6, index: 0}],
16571653
};
16581654
const result = {
@@ -1665,6 +1661,19 @@ suite('lit-html', () => {
16651661
assert.isDefined(div);
16661662
assert.strictEqual(r.value, div);
16671663
});
1664+
1665+
test(`throw if unbranded`, () => {
1666+
const _$lit_template_1: CompiledTemplate = {
1667+
h: ['<div><!----></div>'] as unknown as TemplateStringsArray,
1668+
parts: [{type: 2, index: 1}],
1669+
};
1670+
const result = {
1671+
// This property needs to remain unminified.
1672+
['_$litType$']: _$lit_template_1,
1673+
values: ['A'],
1674+
};
1675+
assert.throws(() => render(result, container));
1676+
});
16681677
});
16691678

16701679
suite('directives', () => {

0 commit comments

Comments
 (0)