diff --git a/components/email/email-composer.tsx b/components/email/email-composer.tsx index 60077f6e6..e2aaabb1b 100644 --- a/components/email/email-composer.tsx +++ b/components/email/email-composer.tsx @@ -2165,7 +2165,7 @@ export function EmailComposer({ ? substitutePlaceholders(template.subject, filledValues) : template.subject; const filledBody = Object.keys(filledValues).length > 0 - ? substitutePlaceholders(template.body, filledValues) + ? substitutePlaceholders(template.body, filledValues, { html: !!template.isHTML }) : template.body; // Match the template to the draft's format: an HTML template goes into a diff --git a/components/templates/placeholder-fill-modal.tsx b/components/templates/placeholder-fill-modal.tsx index 3dd5e8d2e..8cae3b2b8 100644 --- a/components/templates/placeholder-fill-modal.tsx +++ b/components/templates/placeholder-fill-modal.tsx @@ -42,8 +42,8 @@ export function PlaceholderFillModal({ const dirty = placeholders.some((p) => values[p] !== initialValues[p]); const preview = useMemo(() => { - return substitutePlaceholders(template.body, values); - }, [template.body, values]); + return substitutePlaceholders(template.body, values, { html: !!template.isHTML }); + }, [template.body, template.isHTML, values]); const requestClose = async () => { if (await confirmDiscard(dirty)) onClose(); diff --git a/lib/__tests__/template-utils.test.ts b/lib/__tests__/template-utils.test.ts index 05e677966..94149da89 100644 --- a/lib/__tests__/template-utils.test.ts +++ b/lib/__tests__/template-utils.test.ts @@ -10,6 +10,7 @@ import { filterTemplates, exportTemplates, importTemplates, + parseSyncedTemplates, spliceTemplateAboveSignature, composeBodyHasUserContent, embedTemplateImages, @@ -79,9 +80,18 @@ describe('substitutePlaceholders', () => { expect(substitutePlaceholders('{{known}} {{unknown}}', { known: 'yes' })).toBe('yes {{unknown}}'); }); - it('sanitizes XSS in values', () => { - const result = substitutePlaceholders('{{name}}', { name: '' }); - expect(result).not.toContain('' }, { html: true }); + expect(result).toBe('

<script>alert(1)</script>

'); + }); + + // Values are text: an address in angle brackets or an ampersand is kept. + it('keeps values as typed in a plain template', () => { + expect(substitutePlaceholders('To {{name}}', { name: 'Smith & Co ' })).toBe('To Smith & Co '); + }); + + it('keeps the address in an HTML template visible', () => { + expect(substitutePlaceholders('

{{name}}

', { name: 'Smith ' }, { html: true })).toBe('

Smith <j@x.com>

'); }); it('handles empty values', () => { @@ -284,15 +294,33 @@ describe('importTemplates', () => { expect(result.errors).toContain('missing_template_name'); }); - it('sanitizes imported values against XSS', () => { + // Name, subject, category and a plain body are text, shown as text: markup + // in them is harmless and is kept as typed, not cut out or entity-encoded. + it('keeps angle brackets and ampersands in plain-text fields', () => { const json = JSON.stringify({ type: 'webmail-templates', version: 1, - templates: [{ name: '', subject: '' }], + templates: [{ + name: 'Sales ', + subject: 'Re: {{recipient_name}} <{{recipient_email}}>', + category: 'R&D', + body: 'Please reply to if a'); + const [tpl] = importTemplates(json).templates; + expect(tpl.name).toBe('Sales '); + expect(tpl.subject).toBe('Re: {{recipient_name}} <{{recipient_email}}>'); + expect(tpl.category).toBe('R&D'); + expect(tpl.body).toBe('Please reply to if a { + const [tpl] = parseSyncedTemplates([ + { id: 't1', name: 'Sales', subject: 'a <= b', body: 'Write to & us', isHTML: false }, + ])!; + expect(tpl.subject).toBe('a <= b'); + expect(tpl.body).toBe('Write to & us'); }); it('handles missing templates array', () => { diff --git a/lib/template-utils.ts b/lib/template-utils.ts index edfb03588..877b2fbd9 100644 --- a/lib/template-utils.ts +++ b/lib/template-utils.ts @@ -1,12 +1,10 @@ -import DOMPurify from 'dompurify'; -import { sanitizeEmailHtml } from './email-sanitization'; +import { escapeHtml, sanitizeEmailHtml } from './email-sanitization'; import type { EmailTemplate } from './template-types'; import { BUILT_IN_PLACEHOLDERS } from './template-types'; import { generateUUID } from './utils'; const PLACEHOLDER_REGEX = /\{\{(\w+)\}\}/g; const MAX_TEMPLATE_NAME_LENGTH = 200; -const STRIP_HTML_CONFIG = { ALLOWED_TAGS: [] as string[], ALLOWED_ATTR: [] as string[] }; export function extractPlaceholders(text: string): string[] { const matches = new Set(); @@ -18,13 +16,18 @@ export function extractPlaceholders(text: string): string[] { return Array.from(matches); } +/** + * Fill in {{placeholders}}. Values are text: escaped for an HTML template, + * kept as typed for a plain one, so "Smith " survives either way. + */ export function substitutePlaceholders( text: string, - values: Record + values: Record, + options: { html?: boolean } = {}, ): string { return text.replace(PLACEHOLDER_REGEX, (full, name) => { if (values[name] === undefined) return full; - return DOMPurify.sanitize(values[name], STRIP_HTML_CONFIG); + return options.html ? escapeHtml(values[name]) : values[name]; }); } @@ -189,8 +192,11 @@ export async function embedTemplateImages( return changed ? doc.body.innerHTML : html; } +// Names, subjects, categories and plain bodies are text, shown as text and +// escaped wherever they go into HTML. Running them through an HTML sanitizer +// cut "" out and turned "&" into "&". function sanitizeText(value: unknown): string { - return DOMPurify.sanitize(String(value || ''), STRIP_HTML_CONFIG); + return String(value || ''); } /**