fix SQL injection in create-item staging insert (dollar-quote escape defeated by $$$)

060357dfd503817d2f9a5f40dbe6e73806891d50 · 2026-09-22 14:14:11 -0700 · Steve Abrams

The create-item staging insert wrapped user values in $$...$$ and "escaped" via
replace(/$$/g,'$'), which is non-idempotent and fails on runs of >=3 '$'
(esc("$$$")->"$$" still contains $$), letting a crafted dw_sku/mfr/name/color
close the literal early and inject SQL into DW_DB (dw_unified mirror) via an
authenticated POST to /api/create-item.

Fix: embed every user-controlled value through pgLit — standard single-quote
literal escaping ('' doubling), the same proven pattern already used by the _pgq
successor resolver (now pointed at the shared helper). With
standard_conforming_strings on (PG default) no value can terminate its own
literal. Also gate the numeric price column through Number.isFinite so the raw
user string is never interpolated; pid stays a bare bigint literal by design
(exact digits, no Number() precision loss).

Audited the sibling psql -c interpolation sites: unifiedSimilar (3010, %term%)
and identifyUnified (3052, 'norm') are already injection-safe by input
whitelisting ([^a-z0-9 &-] and [^A-Z0-9] respectively). No change needed there.

Proof: pgLit yields a balanced single-quote literal for every attack string
tested ("$$$", "$$$$", "'; DROP TABLE ...;--"); the old $$-wrap left $$ in the
escaped value. node --check passes.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0142i1ci5jdMrmc8DNr6RjUS

Files touched

M server.js

Diff

commit 060357dfd503817d2f9a5f40dbe6e73806891d50
Author: Steve Abrams <steve@designerwallcoverings.com>
Date:   Tue Sep 22 14:14:11 2026 -0700

    fix SQL injection in create-item staging insert (dollar-quote escape defeated by $$$)
    
    The create-item staging insert wrapped user values in $$...$$ and "escaped" via
    replace(/$$/g,'$'), which is non-idempotent and fails on runs of >=3 '$'
    (esc("$$$")->"$$" still contains $$), letting a crafted dw_sku/mfr/name/color
    close the literal early and inject SQL into DW_DB (dw_unified mirror) via an
    authenticated POST to /api/create-item.
    
    Fix: embed every user-controlled value through pgLit — standard single-quote
    literal escaping ('' doubling), the same proven pattern already used by the _pgq
    successor resolver (now pointed at the shared helper). With
    standard_conforming_strings on (PG default) no value can terminate its own
    literal. Also gate the numeric price column through Number.isFinite so the raw
    user string is never interpolated; pid stays a bare bigint literal by design
    (exact digits, no Number() precision loss).
    
    Audited the sibling psql -c interpolation sites: unifiedSimilar (3010, %term%)
    and identifyUnified (3052, 'norm') are already injection-safe by input
    whitelisting ([^a-z0-9 &-] and [^A-Z0-9] respectively). No change needed there.
    
    Proof: pgLit yields a balanced single-quote literal for every attack string
    tested ("$$$", "$$$$", "'; DROP TABLE ...;--"); the old $$-wrap left $$ in the
    escaped value. node --check passes.
    
    Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
    Claude-Session: https://claude.ai/code/session_0142i1ci5jdMrmc8DNr6RjUS
---
 server.js | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/server.js b/server.js
index 0ebc5d3..1b07073 100644
--- a/server.js
+++ b/server.js
@@ -2443,6 +2443,11 @@ function getVendors() {
 }
 
 // ── tiny psql helpers (this app shells to psql; zero pg driver deps) ──────────
+// Safe SQL string literal: standard single-quote escaping ('' doubling). With
+// standard_conforming_strings on (PG default), no value can terminate its own
+// literal, so this is the correct way to embed user input in a psql -c statement.
+// Shared by createNewItem's staging insert and the _pgq successor resolver below.
+const pgLit = s => "'" + String(s == null ? '' : s).replace(/'/g, "''") + "'";
 function pgRows(sql) {
   return new Promise(resolve => {
     execFile(PSQL, ['-d', DW_DB, '-tAF', '\t', '-c', sql], { timeout: 9000, maxBuffer: 8 * 1024 * 1024 }, (err, out) => {
@@ -2690,12 +2695,16 @@ async function createNewItem(p, b64, dryRun) {
     if (cr.status < 200 || cr.status >= 300) return { ok: false, err: `Shopify create HTTP ${cr.status}: ${(cr.raw || '').slice(0, 160)}` };
     const pid = cr.body && cr.body.product && cr.body.product.id;
     // stage into dw_unified (additive table — does NOT touch canonical catalog rows).
-    // $$-dollar-quoting: a stray "$$" in ANY value would break the quote → strip it from EVERY
-    // interpolated string (not just specs) so a vendor/mfr like "Foo $$ Bar" can't malform the SQL.
-    const esc = s => String(s == null ? '' : s).replace(/\$\$/g, '$');
-    const specsJson = esc(JSON.stringify(specsObj));
+    // Embed every user-controlled value via pgLit (standard '' -escaped SQL literals). The old
+    // $$-dollar-quote escape was broken: replace(/\$\$/g,'$') is non-idempotent and fails on runs
+    // of ≥3 '$' (esc("$$$")→"$$" still contains $$), so a crafted value like dw_sku="$$$" could
+    // close the surrounding $$…$$ literal early and inject SQL into DW_DB. pgLit has no such hatch.
+    const specsJson = JSON.stringify(specsObj);
+    // price is user input (String(p.price)); emit it only as a validated finite number, never a raw
+    // interpolated string — a numeric column can't take a quoted literal, so it must be gated as a number.
+    const priceLit = (price != null && Number.isFinite(+price)) ? +price : 'NULL';
     const stageSQL = `insert into new_items_staging (dw_sku, mfr_sku, vendor, vid, pattern_name, color, price, specs, shopify_product_id, created_via)
-      values ($$${esc(dwsku)}$$,$$${esc(mfr)}$$,$$${esc(vreg.vendor)}$$,$$${esc(vreg.vid || '')}$$,$$${esc(name)}$$,$$${esc(color)}$$,${price || 'NULL'},$$${specsJson}$$::jsonb,${pid || 'NULL'},$$scan$$) on conflict do nothing`;
+      values (${pgLit(dwsku)},${pgLit(mfr)},${pgLit(vreg.vendor)},${pgLit(vreg.vid || '')},${pgLit(name)},${pgLit(color)},${priceLit},${pgLit(specsJson)}::jsonb,${pid || 'NULL'},'scan') on conflict do nothing`;
     execFile(PSQL, ['-d', DW_DB, '-c', 'create table if not exists new_items_staging (id bigserial primary key, dw_sku text, mfr_sku text, vendor text, vid text, pattern_name text, color text, price numeric, shopify_product_id bigint, created_via text, created_at timestamptz default now()); alter table new_items_staging add column if not exists specs jsonb; ' + stageSQL], { timeout: 8000 }, () => {});
     // FileMaker WALLPAPER master — real create (dedupe on Series + Mfr Pattern so a re-run never dupes).
     let fmResult = { committed: false, skipped: 'FileMaker disabled or not configured' };
@@ -3081,7 +3090,7 @@ function identifyUnified(code) {
 // discontinued and linked to its live SUCCESSOR: the nearest ACTIVE product whose name matches
 // the dead item's real pattern — resolved via a COPY-OF-<pattern> mfr hint (often only on a
 // SIBLING sharing the dead title) then the dead base title itself. $0 local psql.
-const _pgLit = s => "'" + String(s == null ? '' : s).replace(/'/g, "''") + "'";
+const _pgLit = pgLit;   // shared single-quote SQL-literal escaper (defined near the psql helpers above)
 function _pgq(sql) {
   return new Promise(resolve => {
     execFile(PSQL, ['-d', DW_DB, '--csv', '-v', 'ON_ERROR_STOP=1', '-c', sql],

← 0d34d1b gitignore 5x/out/ screen-capture test artifacts · back to Dw Photo Capture · harden pid interpolation in staging insert (independent-revi 4d1ee36 →