SEC-1: fix shell injection in POST /api/leads

b6ef2b2c22a18a04e100dc64a8e6b769fa8da840 · 2026-05-23 11:43:07 -0700 · Steve Abrams

server.js:12683 previously built SQL with single-quote escaping then piped
it as a shell argument (`psql -c "${sql}"`). The SQL escape blocked SQL
injection but a literal `"` in any user-supplied field (name, email,
phone, message, source) terminated the shell argument and allowed
arbitrary commands to run under the postgres role on Kamatera. Public
unauthenticated route — pre-auth RCE.

Fix: switch to psqlQuery() which stdin-feeds the SQL (no shell arg
involved) and use pgEsc() for value escaping. Defense in depth:
- email must match /[^\\s@]+@[^\\s@]+\\.[^\\s@]+/
- name capped 200, phone 40, message 4000, source 100, email 320 chars
- design_id forced through parseInt + Number.isFinite

Behavior preserved: still returns {ok:true} on DB error so user isn't
blocked, still creates the table lazily.

Files touched

M server.js

Diff

commit b6ef2b2c22a18a04e100dc64a8e6b769fa8da840
Author: Steve Abrams <steve@designerwallcoverings.com>
Date:   Sat May 23 11:43:07 2026 -0700

    SEC-1: fix shell injection in POST /api/leads
    
    server.js:12683 previously built SQL with single-quote escaping then piped
    it as a shell argument (`psql -c "${sql}"`). The SQL escape blocked SQL
    injection but a literal `"` in any user-supplied field (name, email,
    phone, message, source) terminated the shell argument and allowed
    arbitrary commands to run under the postgres role on Kamatera. Public
    unauthenticated route — pre-auth RCE.
    
    Fix: switch to psqlQuery() which stdin-feeds the SQL (no shell arg
    involved) and use pgEsc() for value escaping. Defense in depth:
    - email must match /[^\\s@]+@[^\\s@]+\\.[^\\s@]+/
    - name capped 200, phone 40, message 4000, source 100, email 320 chars
    - design_id forced through parseInt + Number.isFinite
    
    Behavior preserved: still returns {ok:true} on DB error so user isn't
    blocked, still creates the table lazily.
---
 server.js | 34 +++++++++++++++++++---------------
 1 file changed, 19 insertions(+), 15 deletions(-)

diff --git a/server.js b/server.js
index b36b8dc..28ca665 100644
--- a/server.js
+++ b/server.js
@@ -12680,14 +12680,26 @@ ${HAMBURGER_JS}
 });
 
 // ── API: Lead capture
+// Previous impl shell-piped the SQL via `psql -c "${sql}"`; the single-quote
+// escaping (`replace(/'/g,"''")`) handled SQL injection but a literal `"` in
+// any user field terminated the shell argument → RCE under postgres. Switched
+// to psqlQuery() which stdin-feeds the SQL (no shell arg involved). Inputs
+// are also length-capped + email-validated as defense in depth.
+const _capStr = (v, max) => (v == null ? null : String(v).slice(0, max));
 app.post('/api/leads', async (req, res) => {
-  const { name, email, phone, message, design_id, source } = req.body;
-  if (!email) return res.status(400).json({ ok: false, error: 'email required' });
+  const raw = req.body || {};
+  const email = _capStr(raw.email, 320);  // RFC 5321 max
+  if (!email || !/^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(email)) {
+    return res.status(400).json({ ok: false, error: 'valid email required' });
+  }
+  const name      = _capStr(raw.name,    200);
+  const phone     = _capStr(raw.phone,   40);
+  const message   = _capStr(raw.message, 4000);
+  const source    = _capStr(raw.source,  100);
+  const design_id = Number.isFinite(parseInt(raw.design_id, 10)) ? parseInt(raw.design_id, 10) : null;
 
-  // Write to dw_unified.wallco_leads (create table if not exists)
   try {
-    const { execSync } = require('child_process');
-    const sql = `
+    psqlQuery(`
       CREATE TABLE IF NOT EXISTS wallco_leads (
         id BIGSERIAL PRIMARY KEY,
         name TEXT,
@@ -12699,16 +12711,8 @@ app.post('/api/leads', async (req, res) => {
         created_at TIMESTAMPTZ DEFAULT now()
       );
       INSERT INTO wallco_leads (name, email, phone, message, design_id, source)
-      VALUES (
-        ${name    ? `'${name.replace(/'/g,"''")}'`       : 'NULL'},
-        '${email.replace(/'/g,"''")}',
-        ${phone   ? `'${phone.replace(/'/g,"''")}'`      : 'NULL'},
-        ${message ? `'${message.replace(/'/g,"''")}'`    : 'NULL'},
-        ${design_id ? parseInt(design_id,10) : 'NULL'},
-        ${source  ? `'${source.replace(/'/g,"''")}'`     : 'NULL'}
-      );
-    `;
-    execSync(`${PSQL_CMD} -c "${sql.replace(/\n/g,' ')}"`, { encoding: 'utf8' });
+      VALUES (${pgEsc(name)}, ${pgEsc(email)}, ${pgEsc(phone)}, ${pgEsc(message)}, ${pgEsc(design_id)}, ${pgEsc(source)});
+    `);
   } catch (e) {
     console.error('Lead insert error:', e.message);
     // Still return ok — don't block the user on a DB error

← 53c740e cleanup: delete .bak files, orphan entrypoints, unused @dw/i · back to Wallco Ai · SEC-2: harden /_devlogin and prod secret fallbacks (fail-clo ad389f7 →