From 156adeab599241f0d1affa7f701d6bdb517f1862 Mon Sep 17 00:00:00 2001 From: olsch01 Date: Thu, 23 Jul 2026 08:25:22 -0400 Subject: [PATCH] Validate email addresses on both public form endpoints MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The ROI calculator accepted the email field with no validation at all and stored whatever arrived, which is where the spam submissions were putting shell-command payloads. Nothing was executable — there is no child_process or eval in the codebase and all writes are parameterized — but the junk was being persisted, and the field is the obvious place to stop it. Adds security.validateEmail(), deliberately stricter than RFC 5322: the local part is limited to the characters real addresses use, which excludes every shell metacharacter (; | & ` $ ( ) < > \ " ' space) and CSV-injection lead-ins. Also rejects control characters (including the CR/LF used for mail-header injection), caps lengths at 254/64/253, rejects non-strings, and normalizes to trimmed lowercase before storage. Applied to /api/calculate (optional field — empty is fine, present must be valid) and to /api/leads, replacing its much weaker regex. The client mirrors the check for immediate feedback; the server remains authoritative. Also hardens the /api/leads required-field checks, which called .trim() on unvalidated input and returned a 500 rather than a 400 when a bot posted a non-string. Trade-off: RFC-legal but vanishingly rare addresses (foo!bar$baz@x.com, a leading + in the local part) are rejected. Those characters are the injection surface. Co-Authored-By: Claude Opus 4.8 --- app.js | 21 +++++++++++++++++- security.js | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++++ server.js | 42 ++++++++++++++++++++++++------------ 3 files changed, 110 insertions(+), 15 deletions(-) diff --git a/app.js b/app.js index 85cd4a3..63efea9 100644 --- a/app.js +++ b/app.js @@ -121,6 +121,16 @@ const calcBtnText = submitBtn?.querySelector('.calc-btn-text'); const calcBtnLoading = submitBtn?.querySelector('.calc-btn-loading'); + // Mirrors security.js validateEmail — the server is the authority, this just + // gives immediate feedback instead of a round-trip. + const EMAIL_RX = /^[A-Za-z0-9](?:[A-Za-z0-9._%+-]{0,62}[A-Za-z0-9])?@[A-Za-z0-9](?:[A-Za-z0-9-]{0,61}[A-Za-z0-9])?(?:\.[A-Za-z0-9](?:[A-Za-z0-9-]{0,61}[A-Za-z0-9])?)*\.[A-Za-z]{2,24}$/; + + function isValidEmail(v) { + if (v.length > 254 || v.indexOf('@') > 64) return false; + if (v.includes('..')) return false; + return EMAIL_RX.test(v); + } + function showCalcError(msg) { if (!calcErr) return; calcErr.textContent = msg; @@ -148,6 +158,14 @@ showCalcError('Please fill in homesites and annual dues income to continue.'); return; } + + // Email is optional, but anything entered must be a real address. + if (calcEmail && !isValidEmail(calcEmail)) { + showCalcError('Please enter a valid email address.'); + document.getElementById('calcEmail')?.focus(); + return; + } + calcErr.classList.add('hidden'); setCalcLoading(true); @@ -225,9 +243,10 @@ // is stored. AI/service errors (502/503) fall through to the local result. if (!res.ok) { const data = await res.json().catch(() => ({})); - if (data.blocked) { + if (data.blocked || data.field === 'email') { setCalcLoading(false); showCalcError(data.error || 'We could not verify this submission. Please try again.'); + if (data.field === 'email') document.getElementById('calcEmail')?.focus(); resetCaptcha(); await fetchFormToken(); // tokens are single-use; issue a fresh one return; diff --git a/security.js b/security.js index a101f02..298c04f 100644 --- a/security.js +++ b/security.js @@ -144,10 +144,70 @@ const MESSAGES = { captcha_missing: 'Please complete the verification challenge.', captcha_failed: 'Verification failed. Please try the challenge again.', captcha_unavailable: 'Verification is temporarily unavailable. Please try again in a moment.', + email_required: 'Please enter your email address.', + email_invalid: 'Please enter a valid email address.', + email_too_long: 'That email address is too long.', }; const STATUS = { rate_limited: 429, honeypot: 400 }; +/** User-facing message for a validation/abuse reason code. */ +function messageFor(reason) { + return MESSAGES[reason] || 'Submission rejected.'; +} + +// ── Email validation ───────────────────────────────────── +// Deliberately stricter than RFC 5322. The local part is limited to the +// characters real-world addresses actually use, which excludes every shell +// metacharacter (; | & ` $ ( ) < > \ " ' space) and every CSV-injection lead-in +// (= + @ at position 0). RFC-legal oddities like `foo!bar$baz@x.com` are +// rejected — an acceptable trade for a marketing form. +const EMAIL_RX = /^[A-Za-z0-9](?:[A-Za-z0-9._%+-]{0,62}[A-Za-z0-9])?@[A-Za-z0-9](?:[A-Za-z0-9-]{0,61}[A-Za-z0-9])?(?:\.[A-Za-z0-9](?:[A-Za-z0-9-]{0,61}[A-Za-z0-9])?)*\.[A-Za-z]{2,24}$/; + +// C0/C1 control characters and DEL — includes the CR/LF used for header injection. +const CONTROL_CHARS_RX = /[\x00-\x1F\x7F-\x9F]/; + +/** + * Validate an email address. + * Returns { ok: true, email } with the normalised (trimmed, lower-cased) value, + * or { ok: false, reason }. + * + * Pass { required: false } to accept an empty value (the calculator's email + * field is optional) — an empty result comes back as { ok: true, email: null }. + */ +function validateEmail(raw, { required = true } = {}) { + if (raw === undefined || raw === null || raw === '') { + return required ? { ok: false, reason: 'email_required' } : { ok: true, email: null }; + } + + // Anything that isn't a plain string is a structured-injection attempt + // (arrays and objects can survive into places a string wouldn't). + if (typeof raw !== 'string') return { ok: false, reason: 'email_invalid' }; + + const email = raw.trim(); + if (email === '') { + return required ? { ok: false, reason: 'email_required' } : { ok: true, email: null }; + } + + // Length caps first — bounds every check that follows. + if (email.length > 254) return { ok: false, reason: 'email_too_long' }; + + // Control characters, including the newlines used for header injection. + if (CONTROL_CHARS_RX.test(email)) return { ok: false, reason: 'email_invalid' }; + + const at = email.indexOf('@'); + if (at < 1 || at !== email.lastIndexOf('@')) return { ok: false, reason: 'email_invalid' }; + + const local = email.slice(0, at); + const domain = email.slice(at + 1); + + if (local.length > 64 || domain.length > 253) return { ok: false, reason: 'email_too_long' }; + if (email.includes('..')) return { ok: false, reason: 'email_invalid' }; + if (!EMAIL_RX.test(email)) return { ok: false, reason: 'email_invalid' }; + + return { ok: true, email: email.toLowerCase() }; +} + /** * Run every protection layer for a public form POST. * Returns { ok: true, ip } or { ok: false, status, error, reason }. @@ -215,6 +275,8 @@ module.exports = { turnstileEnabled, issueFormToken, guardSubmission, + validateEmail, + messageFor, publicConfig, clientIp, }; diff --git a/server.js b/server.js index 0df0ed2..65e53aa 100644 --- a/server.js +++ b/server.js @@ -129,36 +129,41 @@ app.get('/api/form-token', (_req, res) => { app.post('/api/leads', (req, res) => { const { firstName, lastName, email, orgName, state, role, unitCount, betaInterest, source } = req.body ?? {}; + // Coerce defensively: a non-string (array, object, number) would otherwise + // blow up on .trim() and surface as a 500 instead of a 400. + const str = v => (typeof v === 'string' ? v.trim() : ''); + // Validate required fields - if (!firstName?.trim() || !lastName?.trim() || !email?.trim()) { + if (!str(firstName) || !str(lastName) || !str(email)) { return res.status(400).json({ error: 'firstName, lastName, and email are required.' }); } - if (!orgName?.trim()) { + if (!str(orgName)) { return res.status(400).json({ error: 'Organization name is required.' }); } - if (!state?.trim()) { + if (!str(state)) { return res.status(400).json({ error: 'State is required.' }); } - // Simple email format check - const emailRx = /^[^\s@]+@[^\s@]+\.[^\s@]+$/; - if (!emailRx.test(email.trim())) { - return res.status(400).json({ error: 'Invalid email address.' }); + // Strict email format check — see security.validateEmail + const emailCheck = security.validateEmail(email); + if (!emailCheck.ok) { + return res.status(400).json({ error: security.messageFor(emailCheck.reason), field: 'email' }); } + const cleanEmail = emailCheck.email; // Check for duplicate - const existing = findByEmail.get(email.trim().toLowerCase()); + const existing = findByEmail.get(cleanEmail); if (existing) { return res.status(409).json({ error: 'This email is already on the list.', id: existing.id }); } try { const info = insertLead.run({ - firstName: firstName.trim(), - lastName: lastName.trim(), - email: email.trim().toLowerCase(), - orgName: orgName?.trim() ?? null, - state: state?.trim() ?? null, + firstName: str(firstName), + lastName: str(lastName), + email: cleanEmail, + orgName: str(orgName) || null, + state: str(state) || null, role: role ?? null, unitCount: unitCount ?? null, betaInterest: betaInterest ? 1 : 0, @@ -190,7 +195,7 @@ app.post('/api/calculate', async (req, res) => { function saveCalcSubmission(aiRecommendation) { try { insertCalcSubmission.run({ - email: email?.trim() || null, + email: cleanEmail, optIn: optIn ? 1 : 0, homesites: homesites || null, propertyType: propertyType || null, @@ -220,6 +225,15 @@ app.post('/api/calculate', async (req, res) => { email, optIn, totalPotential, opInterest, resInterest, } = req.body ?? {}; + // Email is optional here, but if one is supplied it must be a real address. + // Nothing is stored or sent onward until it passes. + const emailCheck = security.validateEmail(email, { required: false }); + if (!emailCheck.ok) { + console.warn(`[abuse] rejected email from ${guard.ip}: ${JSON.stringify(String(email).slice(0, 120))}`); + return res.status(400).json({ error: security.messageFor(emailCheck.reason), field: 'email' }); + } + const cleanEmail = emailCheck.email; + if (!homesites || !annualIncome) { return res.status(400).json({ error: 'homesites and annualIncome are required.' }); }