Skip to content

Commit

Permalink
[lit-html] change CompiledTemplate h field to a TemplateStringsArray (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
AndrewJakubowicz committed Jun 30, 2023
1 parent 2190dfe commit bb2560f
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 42 deletions.
6 changes: 6 additions & 0 deletions .changeset/weak-roses-laugh.md
@@ -0,0 +1,6 @@
---
'lit-html': patch
'lit': patch
---

Change the `h` field of `CompiledTemplate`s to a `TemplateStringsArray` preventing the spoofing of `CompiledTemplate`s by JSON injection attacks. This should not be a breaking change for most users unless you're using CompiledTemplates. This is a necessary security fix, similar to [#2307](https://github.com/lit/lit/pull/2307).
74 changes: 41 additions & 33 deletions packages/lit-html/src/lit-html.ts
Expand Up @@ -489,7 +489,9 @@ export interface CompiledTemplate extends Omit<Template, 'el'> {
el?: HTMLTemplateElement;

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

/**
Expand Down Expand Up @@ -652,6 +654,39 @@ export interface DirectiveParent {
__directives?: Array<Directive | undefined>;
}

function trustFromTemplateString(
tsa: TemplateStringsArray,
stringFromTSA: string
): TrustedHTML {
// A security check to prevent spoofing of Lit template results.
// In the future, we may be able to replace this with Array.isTemplateObject,
// though we might need to make that check inside of the html and svg
// functions, because precompiled templates don't come in as
// TemplateStringArray objects.
if (!Array.isArray(tsa) || !tsa.hasOwnProperty('raw')) {
let message = 'invalid template strings array';
if (DEV_MODE) {
message = `
Internal Error: expected template strings to be an array
with a 'raw' field. Faking a template strings array by
calling html or svg like an ordinary function is effectively
the same as calling unsafeHtml and can lead to major security
issues, e.g. opening your code up to XSS attacks.
If you're using the html or svg tagged template functions normally
and still seeing this error, please file a bug at
https://github.com/lit/lit/issues/new?template=bug_report.md
and include information about your build tooling, if any.
`
.trim()
.replace(/\n */g, '\n');
}
throw new Error(message);
}
return policy !== undefined
? policy.createHTML(stringFromTSA)
: (stringFromTSA as unknown as TrustedHTML);
}

/**
* Returns an HTML string for the given TemplateStringsArray and result type
* (HTML or SVG), along with the case-sensitive bound attribute names in
Expand Down Expand Up @@ -816,38 +851,8 @@ const getTemplateHtml = (
const htmlResult: string | TrustedHTML =
html + (strings[l] || '<?>') + (type === SVG_RESULT ? '</svg>' : '');

// A security check to prevent spoofing of Lit template results.
// In the future, we may be able to replace this with Array.isTemplateObject,
// though we might need to make that check inside of the html and svg
// functions, because precompiled templates don't come in as
// TemplateStringArray objects.
if (!Array.isArray(strings) || !strings.hasOwnProperty('raw')) {
let message = 'invalid template strings array';
if (DEV_MODE) {
message = `
Internal Error: expected template strings to be an array
with a 'raw' field. Faking a template strings array by
calling html or svg like an ordinary function is effectively
the same as calling unsafeHtml and can lead to major security
issues, e.g. opening your code up to XSS attacks.
If you're using the html or svg tagged template functions normally
and still seeing this error, please file a bug at
https://github.com/lit/lit/issues/new?template=bug_report.md
and include information about your build tooling, if any.
`
.trim()
.replace(/\n */g, '\n');
}
throw new Error(message);
}
// Returned as an array for terseness
return [
policy !== undefined
? policy.createHTML(htmlResult)
: (htmlResult as unknown as TrustedHTML),
attrNames,
];
return [trustFromTemplateString(strings, htmlResult), attrNames];
};

/** @internal */
Expand Down Expand Up @@ -1517,7 +1522,10 @@ class ChildPart implements Disconnectable {
typeof type === 'number'
? this._$getTemplate(result as TemplateResult)
: (type.el === undefined &&
(type.el = Template.createElement(type.h, this.options)),
(type.el = Template.createElement(
trustFromTemplateString(type.h, type.h[0]),
this.options
)),
type);

if ((this._$committedValue as TemplateInstance)?._$template === template) {
Expand Down
27 changes: 18 additions & 9 deletions packages/lit-html/src/test/lit-html_test.ts
Expand Up @@ -1590,16 +1590,12 @@ suite('lit-html', () => {
});

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

test('only text', () => {
// A compiled template for html`${'A'}`
const _$lit_template_1: CompiledTemplate = {
h: policy.createHTML('<!---->'),
h: branding_tag`<!---->`,
parts: [{type: 2, index: 0}],
};
assertRender(
Expand All @@ -1615,7 +1611,7 @@ suite('lit-html', () => {
test('text expression', () => {
// A compiled template for html`<div>${'A'}</div>`
const _$lit_template_1: CompiledTemplate = {
h: policy.createHTML(`<div><!----></div>`),
h: branding_tag`<div><!----></div>`,
parts: [{type: 2, index: 1}],
};
const result = {
Expand All @@ -1629,7 +1625,7 @@ suite('lit-html', () => {
test('attribute expression', () => {
// A compiled template for html`<div foo=${'A'}></div>`
const _$lit_template_1: CompiledTemplate = {
h: policy.createHTML('<div></div>'),
h: branding_tag`<div></div>`,
parts: [
{
type: 1,
Expand All @@ -1652,7 +1648,7 @@ suite('lit-html', () => {
const r = createRef();
// A compiled template for html`<div ${ref(r)}></div>`
const _$lit_template_1: CompiledTemplate = {
h: policy.createHTML('<div></div>'),
h: branding_tag`<div></div>`,
parts: [{type: 6, index: 0}],
};
const result = {
Expand All @@ -1665,6 +1661,19 @@ suite('lit-html', () => {
assert.isDefined(div);
assert.strictEqual(r.value, div);
});

test(`throw if unbranded`, () => {
const _$lit_template_1: CompiledTemplate = {
h: ['<div><!----></div>'] as unknown as TemplateStringsArray,
parts: [{type: 2, index: 1}],
};
const result = {
// This property needs to remain unminified.
['_$litType$']: _$lit_template_1,
values: ['A'],
};
assert.throws(() => render(result, container));
});
});

suite('directives', () => {
Expand Down

0 comments on commit bb2560f

Please sign in to comment.