notifications(P4g): strip URLs from user-controlled render inputs
Third-pass audit finding. renderShell auto-linkifies any https?:// in body lines so the manageUrl line becomes a clickable link in the HTML email. The same regex would happily turn a malicious URL injected into a user-controlled field (member display_name, equipment / class / form title, submitter label, space name) into a clickable link. In Phase 2 dues emails this was self-only: the recipient is the same member whose display_name appears, so self-phishing is silly. Phase 4 introduces the first email type where one user's display_name lands in a different user's inbox: form_submission_admin embeds the submitter's display_name (or, when the form was submitted anonymously and an email match attributed it to a member, that member's display_name) in an email read by every forms.manage holder. An authenticated member can set display_name to "Click https://evil.com for free stuff", submit a form, and the admin's inbox now has a clickable phishing URL. body.email is Zod-validated against email format so the typed-email path is closed, but display_name (z.string().min(1).max(100)) is a free string. No format restriction will catch a determined attacker. Mitigation: stripUrls(s) replaces https?://\S+ with [link]. Applied to every user-controlled input across all four renderers (dues, booking, class, form) including spaceName, memberName / recipientName, equipment / class / form title, location, and submitterLabel. manageUrl, which buildManageUrl constructs from NEXT_PUBLIC_APP_URL or a Stripe- authenticated webhook host, stays untouched and remains the only linkified URL in the rendered email. 5 new tests cover the new attack vector (form_submission_admin) and defense-in-depth across booking, class, and dues renderers. Suite 572, build clean.
This commit is contained in:
@@ -383,3 +383,87 @@ describe('renderFormEmail', () => {
|
|||||||
expect(r.html).toContain('<img>')
|
expect(r.html).toContain('<img>')
|
||||||
})
|
})
|
||||||
})
|
})
|
||||||
|
|
||||||
|
describe('URL stripping in user-controlled fields (phishing defense)', () => {
|
||||||
|
// renderShell auto-linkifies https?:// in body lines so the manageUrl line
|
||||||
|
// becomes clickable. That same regex would happily turn a malicious URL in
|
||||||
|
// an injected user-controlled field into a clickable link, which is a
|
||||||
|
// phishing vector wherever those fields land in an email read by someone
|
||||||
|
// OTHER than the field's owner (notably form_submission_admin, which
|
||||||
|
// embeds the submitter's display_name in an admin's inbox). The fix is to
|
||||||
|
// strip URLs from user-controlled inputs before render; manageUrl stays
|
||||||
|
// untouched and remains the only linkified URL.
|
||||||
|
|
||||||
|
it('form_submission_admin scrubs a URL embedded in the submitter label', () => {
|
||||||
|
const r = renderFormEmail({
|
||||||
|
type: 'form_submission_admin',
|
||||||
|
spaceName: 'Acme',
|
||||||
|
recipientName: 'Boss',
|
||||||
|
formTitle: 'Liability Waiver',
|
||||||
|
manageUrl: 'https://x.test/forms/abc/results',
|
||||||
|
submitterLabel: 'Click https://evil.com for free stuff',
|
||||||
|
})
|
||||||
|
// The malicious URL is stripped, replaced with [link].
|
||||||
|
expect(r.text).not.toContain('https://evil.com')
|
||||||
|
expect(r.html).not.toContain('href="https://evil.com"')
|
||||||
|
expect(r.text).toContain('[link]')
|
||||||
|
// The legitimate results URL is still linkified.
|
||||||
|
expect(r.html).toContain('href="https://x.test/forms/abc/results"')
|
||||||
|
})
|
||||||
|
|
||||||
|
it('form_submission_admin scrubs a URL embedded in the form title', () => {
|
||||||
|
const r = renderFormEmail({
|
||||||
|
type: 'form_submission_admin',
|
||||||
|
spaceName: 'Acme',
|
||||||
|
formTitle: 'Visit https://evil.com',
|
||||||
|
manageUrl: 'https://x.test/forms/abc/results',
|
||||||
|
submitterLabel: 'Ada',
|
||||||
|
})
|
||||||
|
expect(r.html).not.toContain('href="https://evil.com"')
|
||||||
|
expect(r.text).toContain('[link]')
|
||||||
|
})
|
||||||
|
|
||||||
|
it('renderBookingEmail scrubs URLs in member name, equipment name, and location (defense in depth)', () => {
|
||||||
|
const r = renderBookingEmail({
|
||||||
|
type: 'booking_confirmed',
|
||||||
|
spaceName: 'Visit https://x.evil',
|
||||||
|
memberName: 'Look https://x.evil',
|
||||||
|
equipmentName: 'Buy https://x.evil',
|
||||||
|
location: 'Bay https://x.evil',
|
||||||
|
startsAt: '2026-05-22T15:00:00Z',
|
||||||
|
endsAt: '2026-05-22T17:00:00Z',
|
||||||
|
manageUrl: 'https://x.test/me',
|
||||||
|
})
|
||||||
|
expect(r.html).not.toContain('href="https://x.evil"')
|
||||||
|
expect(r.html).toContain('href="https://x.test/me"')
|
||||||
|
})
|
||||||
|
|
||||||
|
it('renderClassEmail scrubs URLs in class title and location', () => {
|
||||||
|
const r = renderClassEmail({
|
||||||
|
type: 'class_signup_registered',
|
||||||
|
spaceName: 'Acme',
|
||||||
|
memberName: 'Ada',
|
||||||
|
className: 'Welding https://evil.test',
|
||||||
|
location: 'Bay 1',
|
||||||
|
startsAt: '2026-05-22T15:00:00Z',
|
||||||
|
endsAt: '2026-05-22T17:00:00Z',
|
||||||
|
manageUrl: 'https://x.test/me',
|
||||||
|
})
|
||||||
|
expect(r.html).not.toContain('href="https://evil.test"')
|
||||||
|
expect(r.html).toContain('href="https://x.test/me"')
|
||||||
|
})
|
||||||
|
|
||||||
|
it('renderDuesEmail scrubs URLs in member name and space name (defense in depth)', () => {
|
||||||
|
const r = renderDuesEmail({
|
||||||
|
type: 'dues_renewed',
|
||||||
|
spaceName: 'Acme https://evil.test',
|
||||||
|
memberName: 'Bad https://evil.test',
|
||||||
|
amount: 25,
|
||||||
|
currency: 'USD',
|
||||||
|
periodEnd: '2026-06-15',
|
||||||
|
manageUrl: 'https://x.test/me',
|
||||||
|
})
|
||||||
|
expect(r.html).not.toContain('href="https://evil.test"')
|
||||||
|
expect(r.html).toContain('href="https://x.test/me"')
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|||||||
+28
-14
@@ -70,6 +70,20 @@ function escapeHtml(s: string): string {
|
|||||||
.replace(/'/g, ''')
|
.replace(/'/g, ''')
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// renderShell auto-linkifies any https?:// in the body so the manageUrl line
|
||||||
|
// becomes a clickable link. That same regex turns ANY URL embedded in a
|
||||||
|
// user-controlled input (member display_name, equipment/class title, form
|
||||||
|
// title, submitter label, space name) into a clickable link too, opening a
|
||||||
|
// phishing vector wherever those fields land in an email to a different user
|
||||||
|
// (notably form_submission_admin, which surfaces a submitter's display_name
|
||||||
|
// to an admin). Scrub URL patterns from every user-controlled input before
|
||||||
|
// rendering. Only manageUrl, which is server-built from NEXT_PUBLIC_APP_URL,
|
||||||
|
// is allowed to reach the linkify regex.
|
||||||
|
function stripUrls(s: string | null | undefined): string {
|
||||||
|
if (!s) return ''
|
||||||
|
return s.replace(/https?:\/\/\S+/gi, '[link]')
|
||||||
|
}
|
||||||
|
|
||||||
const CURRENCY_SYMBOL: Record<string, string> = { USD: '$', CAD: '$', EUR: '€', GBP: '£' }
|
const CURRENCY_SYMBOL: Record<string, string> = { USD: '$', CAD: '$', EUR: '€', GBP: '£' }
|
||||||
|
|
||||||
// Stripe zero-decimal currencies: the smallest unit IS the major unit, so
|
// Stripe zero-decimal currencies: the smallest unit IS the major unit, so
|
||||||
@@ -164,8 +178,8 @@ export type RenderedEmail = { subject: string; html: string; text: string }
|
|||||||
// is injected, never hard-coded. Returns both an HTML and a plain-text body
|
// is injected, never hard-coded. Returns both an HTML and a plain-text body
|
||||||
// (Resend will also auto-generate text, but we send an explicit one).
|
// (Resend will also auto-generate text, but we send an explicit one).
|
||||||
export function renderDuesEmail(input: DuesEmailInput): RenderedEmail {
|
export function renderDuesEmail(input: DuesEmailInput): RenderedEmail {
|
||||||
const space = input.spaceName || 'your hackerspace'
|
const space = stripUrls(input.spaceName) || 'your hackerspace'
|
||||||
const name = input.memberName?.trim() || null
|
const name = stripUrls(input.memberName).trim() || null
|
||||||
const greeting = name ? `Hi ${name},` : 'Hi,'
|
const greeting = name ? `Hi ${name},` : 'Hi,'
|
||||||
const money = formatMoney(input.amount, input.currency)
|
const money = formatMoney(input.amount, input.currency)
|
||||||
const renews = formatDate(input.periodEnd)
|
const renews = formatDate(input.periodEnd)
|
||||||
@@ -235,12 +249,12 @@ export type ClassEmailInput = {
|
|||||||
}
|
}
|
||||||
|
|
||||||
export function renderClassEmail(input: ClassEmailInput): RenderedEmail {
|
export function renderClassEmail(input: ClassEmailInput): RenderedEmail {
|
||||||
const space = input.spaceName || 'your hackerspace'
|
const space = stripUrls(input.spaceName) || 'your hackerspace'
|
||||||
const name = input.memberName?.trim() || null
|
const name = stripUrls(input.memberName).trim() || null
|
||||||
const greeting = name ? `Hi ${name},` : 'Hi,'
|
const greeting = name ? `Hi ${name},` : 'Hi,'
|
||||||
const cls = input.className || 'the class'
|
const cls = stripUrls(input.className) || 'the class'
|
||||||
const range = formatRange(input.startsAt, input.endsAt)
|
const range = formatRange(input.startsAt, input.endsAt)
|
||||||
const location = (input.location || '').trim()
|
const location = stripUrls(input.location).trim()
|
||||||
|
|
||||||
let subject: string
|
let subject: string
|
||||||
const lines: string[] = [greeting, '']
|
const lines: string[] = [greeting, '']
|
||||||
@@ -308,10 +322,10 @@ export type FormEmailInput = {
|
|||||||
}
|
}
|
||||||
|
|
||||||
export function renderFormEmail(input: FormEmailInput): RenderedEmail {
|
export function renderFormEmail(input: FormEmailInput): RenderedEmail {
|
||||||
const space = input.spaceName || 'your hackerspace'
|
const space = stripUrls(input.spaceName) || 'your hackerspace'
|
||||||
const name = input.recipientName?.trim() || null
|
const name = stripUrls(input.recipientName).trim() || null
|
||||||
const greeting = name ? `Hi ${name},` : 'Hi,'
|
const greeting = name ? `Hi ${name},` : 'Hi,'
|
||||||
const form = input.formTitle || 'the form'
|
const form = stripUrls(input.formTitle) || 'the form'
|
||||||
|
|
||||||
let subject: string
|
let subject: string
|
||||||
const lines: string[] = [greeting, '']
|
const lines: string[] = [greeting, '']
|
||||||
@@ -325,7 +339,7 @@ export function renderFormEmail(input: FormEmailInput): RenderedEmail {
|
|||||||
)
|
)
|
||||||
} else {
|
} else {
|
||||||
subject = `New submission: ${form} at ${space}`
|
subject = `New submission: ${form} at ${space}`
|
||||||
const who = (input.submitterLabel || '').trim() || 'someone'
|
const who = stripUrls(input.submitterLabel).trim() || 'someone'
|
||||||
lines.push(
|
lines.push(
|
||||||
`${who} submitted ${form} at ${space}.`,
|
`${who} submitted ${form} at ${space}.`,
|
||||||
'',
|
'',
|
||||||
@@ -363,12 +377,12 @@ export type BookingEmailInput = {
|
|||||||
}
|
}
|
||||||
|
|
||||||
export function renderBookingEmail(input: BookingEmailInput): RenderedEmail {
|
export function renderBookingEmail(input: BookingEmailInput): RenderedEmail {
|
||||||
const space = input.spaceName || 'your hackerspace'
|
const space = stripUrls(input.spaceName) || 'your hackerspace'
|
||||||
const name = input.memberName?.trim() || null
|
const name = stripUrls(input.memberName).trim() || null
|
||||||
const greeting = name ? `Hi ${name},` : 'Hi,'
|
const greeting = name ? `Hi ${name},` : 'Hi,'
|
||||||
const equipment = input.equipmentName || 'the equipment'
|
const equipment = stripUrls(input.equipmentName) || 'the equipment'
|
||||||
const range = formatRange(input.startsAt, input.endsAt)
|
const range = formatRange(input.startsAt, input.endsAt)
|
||||||
const location = (input.location || '').trim()
|
const location = stripUrls(input.location).trim()
|
||||||
|
|
||||||
let subject: string
|
let subject: string
|
||||||
const lines: string[] = [greeting, '']
|
const lines: string[] = [greeting, '']
|
||||||
|
|||||||
Reference in New Issue
Block a user