From d863b1fd4ba39109cb70fec36d4f7cbb1cb413cd Mon Sep 17 00:00:00 2001 From: Stefan Hildebrandt <695494+hildebrandttk@users.noreply.github.com> Date: Wed, 24 Jun 2026 20:04:53 +0200 Subject: [PATCH] fix: HTML-escape sender/subject in reply/forward quote header (#482) The forward quote header renders "From: Name ", but the HTML variant interpolated the sender string unescaped. In the rich-text composer the "" portion is parsed by the browser as a bogus HTML tag and dropped, so the address silently disappears - the user sees only "From: Display Name". The plain-text variant and the details panel escape correctly, which is why the address shows there. This is the regression from #367, which added the "" into the HTML string without escaping it. Fix: HTML-escape the user-controlled values (sender, subject, date) in every HTML quote-header path - the production builder in lib/quote-header.ts and the composer's inline fallback (both htmlBody and plain-body branches), for forward and reply. The reply line keeps the bare display name by design (#367), but its HTML form is now escaped too so a display name containing markup can't break out. As a side benefit this closes an HTML-injection vector: a crafted subject or display name was previously injected raw into the composer document. Adds lib/__tests__/quote-header.test.ts covering: forward text keeps "Name "; forward HTML escapes the angle brackets (address survives) and a markup subject/display name; reply stays bare-name and HTML-safe. --- components/email/email-composer.tsx | 21 +++---- lib/__tests__/quote-header.test.ts | 92 +++++++++++++++++++++++++++++ lib/quote-header.ts | 21 ++++--- 3 files changed, 116 insertions(+), 18 deletions(-) create mode 100644 lib/__tests__/quote-header.test.ts diff --git a/components/email/email-composer.tsx b/components/email/email-composer.tsx index be2d23e2..9456d5c2 100644 --- a/components/email/email-composer.tsx +++ b/components/email/email-composer.tsx @@ -11,7 +11,7 @@ import { debug } from "@/lib/debug"; import { toast } from "@/stores/toast-store"; import { useContextMenu } from "@/hooks/use-context-menu"; import { ContextMenu, ContextMenuItem, ContextMenuSeparator } from "@/components/ui/context-menu"; -import { sanitizeSignatureHtml, sanitizeEmailHtml } from "@/lib/email-sanitization"; +import { sanitizeSignatureHtml, sanitizeEmailHtml, escapeHtml } from "@/lib/email-sanitization"; import { buildReplySubject, buildForwardSubject } from "@/lib/subject-prefix"; import { isFilePreviewable } from "@/lib/file-preview"; import { buildQuotedHtmlBlock, serializeEditorContent } from "@/components/email/quoted-html"; @@ -347,9 +347,8 @@ export function EmailComposer({ const date = replyTo.receivedAt ? formatDateTime(replyTo.receivedAt, timeFormat, { weekday: 'short', year: 'numeric', month: 'short', day: 'numeric' }) : ""; const from = replyTo.from?.[0]; - const fromStr = from ? `${from.name || from.email}` : tCommon('unknown'); - // Forward "From:" shows the full sender incl. address; reply line keeps - // the bare name (reads naturally in the localized "On … wrote:" line). + // Forward "From:" and the reply "On … wrote:" line both show the full + // sender incl. address ("Name "), like Gmail/Outlook (#482). const fromStrFull = from ? (from.name && from.email && from.name !== from.email ? `${from.name} <${from.email}>` @@ -377,7 +376,7 @@ export function EmailComposer({ if (mode === 'forward') { return `${prefix}${signatureBlock}\n\n${tQuote('forwarded_separator')}\n${tQuote('from_label')}: ${fromStrFull}\n${tQuote('date_label')}: ${date}\n${tQuote('subject_label')}: ${replyTo.subject || ''}\n\n${originalText}`; } else if (mode === 'reply' || mode === 'replyAll') { - return `${prefix}${signatureBlock}\n\n${tQuote('reply_line', { date, from: fromStr })}\n${quotedText}`; + return `${prefix}${signatureBlock}\n\n${tQuote('reply_line', { date, from: fromStrFull })}\n${quotedText}`; } return prefix; } @@ -399,7 +398,7 @@ export function EmailComposer({ const date = replyTo.receivedAt ? formatDateTime(replyTo.receivedAt, timeFormat, { weekday: 'short', year: 'numeric', month: 'short', day: 'numeric' }) : ""; const from = replyTo.from?.[0]; - const fromStr = from ? `${from.name || from.email}` : tCommon('unknown'); + // Forward and reply quote lines both show the full "Name " sender (#482). const fromStrFull = from ? (from.name && from.email && from.name !== from.email ? `${from.name} <${from.email}>` @@ -434,9 +433,11 @@ export function EmailComposer({ // Build quoted content as HTML if (replyTo.htmlBody && (mode === 'reply' || mode === 'replyAll' || mode === 'forward')) { + // HTML-escape user-controlled values: an unescaped sender "Name " + // has its "" eaten as a bogus HTML tag by the rich-text editor (#482). const quoteHeader = mode === 'forward' - ? `${tQuote('forwarded_separator')}
${tQuote('from_label')}: ${fromStrFull}
${tQuote('date_label')}: ${date}
${tQuote('subject_label')}: ${replyTo.subject || ''}

` - : `${tQuote('reply_line', { date, from: fromStr })}
`; + ? `${tQuote('forwarded_separator')}
${tQuote('from_label')}: ${escapeHtml(fromStrFull)}
${tQuote('date_label')}: ${escapeHtml(date)}
${tQuote('subject_label')}: ${escapeHtml(replyTo.subject || '')}

` + : `${tQuote('reply_line', { date: escapeHtml(date), from: escapeHtml(fromStrFull) })}
`; // Embed the original as a QuotedHtml island (verbatim, schema-free) so // its layout survives the editor round-trip. Sanitize first to strip // scripts/styles/head; cid rewrite afterwards so data-cid markers @@ -450,9 +451,9 @@ export function EmailComposer({ if (replyTo.body) { const escapedOriginal = replyTo.body.replace(/&/g, '&').replace(//g, '>').replace(/\n/g, '
'); if (mode === 'forward') { - return `${prefix}${signatureBlock}

${tQuote('forwarded_separator')}
${tQuote('from_label')}: ${fromStrFull}
${tQuote('date_label')}: ${date}
${tQuote('subject_label')}: ${replyTo.subject || ''}

${escapedOriginal}`; + return `${prefix}${signatureBlock}

${tQuote('forwarded_separator')}
${tQuote('from_label')}: ${escapeHtml(fromStrFull)}
${tQuote('date_label')}: ${escapeHtml(date)}
${tQuote('subject_label')}: ${escapeHtml(replyTo.subject || '')}

${escapedOriginal}`; } else if (mode === 'reply' || mode === 'replyAll') { - return `${prefix}${signatureBlock}

${tQuote('reply_line', { date, from: fromStr })}
${escapedOriginal}
`; + return `${prefix}${signatureBlock}

${tQuote('reply_line', { date: escapeHtml(date), from: escapeHtml(fromStrFull) })}
${escapedOriginal}
`; } } return prefix; diff --git a/lib/__tests__/quote-header.test.ts b/lib/__tests__/quote-header.test.ts new file mode 100644 index 00000000..5493f646 --- /dev/null +++ b/lib/__tests__/quote-header.test.ts @@ -0,0 +1,92 @@ +import { describe, it, expect } from 'vitest'; +import { buildQuoteHeader } from '@/lib/quote-header'; + +const base = { + newTo: [] as string[], + newCc: [] as string[], + locale: 'en', + timeFormat: '24h' as const, + unknownLabel: 'Unknown', +}; + +const sender = { name: 'Display Name', email: 'user@domain.tld' }; + +describe('buildQuoteHeader (#482 — sender address survives HTML rendering)', () => { + it('forward TEXT keeps the full "Name " sender', async () => { + const h = await buildQuoteHeader({ + mode: 'forward', + email: { from: [sender], subject: 'Hello', receivedAt: '2026-01-01T10:00:00Z' }, + ...base, + }); + expect(h.text).toContain('From: Display Name '); + }); + + it('forward HTML escapes the angle brackets so the address is not eaten as a tag', async () => { + const h = await buildQuoteHeader({ + mode: 'forward', + email: { from: [sender], subject: 'Hello', receivedAt: '2026-01-01T10:00:00Z' }, + ...base, + }); + // The regression: a raw "" is parsed as an HTML tag by the + // rich-text composer and dropped, leaving only "From: Display Name". + expect(h.html).toContain('Display Name <user@domain.tld>'); + expect(h.html).not.toContain(''); + }); + + it('forward HTML escapes a subject containing markup (injection hardening)', async () => { + const h = await buildQuoteHeader({ + mode: 'forward', + email: { from: [sender], subject: 'Hi x', receivedAt: '2026-01-01T10:00:00Z' }, + ...base, + }); + expect(h.html).toContain('Hi <b>x</b>'); + expect(h.html).not.toContain('x'); + }); + + it('forward HTML escapes a malicious display name', async () => { + const h = await buildQuoteHeader({ + mode: 'forward', + email: { + from: [{ name: '', email: 'evil@x.tld' }], + subject: 'Hello', + receivedAt: '2026-01-01T10:00:00Z', + }, + ...base, + }); + expect(h.html).not.toContain('" sender, escaped in HTML', async () => { + const h = await buildQuoteHeader({ + mode: 'reply', + email: { from: [sender], subject: 'Hello', receivedAt: '2026-01-01T10:00:00Z' }, + ...base, + }); + // TEXT keeps the real angle brackets ("On , Display Name wrote:"). + expect(h.text).toContain('Display Name wrote:'); + // HTML escapes them so the address survives the rich-text editor (#482). + expect(h.html).toContain('Display Name <user@domain.tld>'); + expect(h.html).not.toContain(''); + }); + + it('reply line stays HTML-safe for a display name containing markup', async () => { + const evil = await buildQuoteHeader({ + mode: 'reply', + email: { from: [{ name: 'x', email: 'e@x.tld' }], subject: 'Hello', receivedAt: '2026-01-01T10:00:00Z' }, + ...base, + }); + expect(evil.html).not.toContain('x'); + expect(evil.html).toContain('<b>x</b>'); + }); + + it('reply line falls back to bare email when there is no display name', async () => { + const h = await buildQuoteHeader({ + mode: 'reply', + email: { from: [{ email: 'noname@x.tld' }], subject: 'Hello', receivedAt: '2026-01-01T10:00:00Z' }, + ...base, + }); + expect(h.text).toContain('noname@x.tld wrote:'); + expect(h.text).not.toContain(''); + }); +}); diff --git a/lib/quote-header.ts b/lib/quote-header.ts index c55eb046..686df5e1 100644 --- a/lib/quote-header.ts +++ b/lib/quote-header.ts @@ -8,6 +8,7 @@ import { formatDateTime } from "@/lib/utils"; import { emailHooks } from "@/lib/plugin-hooks"; +import { escapeHtml } from "@/lib/email-sanitization"; import type { QuoteHeader, QuoteHeaderContext } from "@/lib/plugin-types"; // Localized label set the caller passes in. Labels live on the client where @@ -62,10 +63,8 @@ function defaultHeader(args: BuildArgs): QuoteHeader { }) : ""; const from = email.from?.[0]; - const fromStr = from ? `${from.name || from.email}` : unknownLabel; - // Forward header "From:" shows the full sender incl. address ("Name - // "), like every mail client. The reply line keeps the bare name - // (reads more naturally in "On … wrote:"). + // Both the forward "From:" line and the reply "On … wrote:" line show the + // full sender incl. address ("Name "), like Gmail/Outlook (#482). const fromStrFull = from ? (from.name && from.email && from.name !== from.email ? `${from.name} <${from.email}>` @@ -75,13 +74,19 @@ function defaultHeader(args: BuildArgs): QuoteHeader { if (mode === "forward") { const text = `${labels.forwardedSeparator}\n${labels.fromLabel}: ${fromStrFull}\n${labels.dateLabel}: ${date}\n${labels.subjectLabel}: ${subject}\n`; - const html = `
${labels.forwardedSeparator}
${labels.fromLabel}: ${fromStrFull}
${labels.dateLabel}: ${date}
${labels.subjectLabel}: ${subject}

`; + // Escape the interpolated values for the HTML variant: the sender string is + // "Name ", and the unescaped "" would be parsed as an HTML tag + // by the rich-text composer and silently dropped (#482). Subject/name are + // likewise user-controlled. Label/separator strings are trusted i18n text. + const html = `
${labels.forwardedSeparator}
${labels.fromLabel}: ${escapeHtml(fromStrFull)}
${labels.dateLabel}: ${escapeHtml(date)}
${labels.subjectLabel}: ${escapeHtml(subject)}

`; return { html, text, wrapInBlockquote: false }; } - const replyLine = labels.formatReplyLine({ date, from: fromStr }); - const text = `${replyLine}\n`; - const html = `
${replyLine}
`; + const text = `${labels.formatReplyLine({ date, from: fromStrFull })}\n`; + // Escape the interpolated sender/date for the HTML reply line: the sender is + // now "Name ", and the unescaped "" would be parsed as an HTML + // tag by the rich-text composer and dropped (#482). Label template is trusted. + const html = `
${labels.formatReplyLine({ date: escapeHtml(date), from: escapeHtml(fromStrFull) })}
`; return { html, text, wrapInBlockquote: true }; }