← back to Wallco Ai
security — close path traversal + shell injection in /api/studio/upload. sid/name user inputs are now strictly validated (sid: ^[a-z0-9-]{1,40}$ allowlist or anon-N fallback; name: strip non-[a-z0-9._-], collapse '..' sequences, trim leading dots). Resolved target verified to start with safeDir+sep. sips and python3 invocations switched from execSync(`...${path}...`) → execFileSync(cmd, [args]) so shell metacharacters can't reach a shell. Python script reads path from argv[1] instead of being source-string-interpolated. +5 integration tests verifying ../../etc/passwd traversal, name='../../escape.png', leading-dot '.env', shell-special-char sid all neutralized while normal uploads still 200.
70b45704da75a370c0cc2c33c59343eb262aa3c2 · 2026-05-12 06:40:57 -0700 · Steve Abrams
Files touched
M server.jsA tests/integration/upload-security.spec.js
Diff
commit 70b45704da75a370c0cc2c33c59343eb262aa3c2
Author: Steve Abrams <steve@designerwallcoverings.com>
Date: Tue May 12 06:40:57 2026 -0700
security — close path traversal + shell injection in /api/studio/upload. sid/name user inputs are now strictly validated (sid: ^[a-z0-9-]{1,40}$ allowlist or anon-N fallback; name: strip non-[a-z0-9._-], collapse '..' sequences, trim leading dots). Resolved target verified to start with safeDir+sep. sips and python3 invocations switched from execSync(`...${path}...`) → execFileSync(cmd, [args]) so shell metacharacters can't reach a shell. Python script reads path from argv[1] instead of being source-string-interpolated. +5 integration tests verifying ../../etc/passwd traversal, name='../../escape.png', leading-dot '.env', shell-special-char sid all neutralized while normal uploads still 200.
---
server.js | 46 +++++++++----
tests/integration/upload-security.spec.js | 106 ++++++++++++++++++++++++++++++
2 files changed, 141 insertions(+), 11 deletions(-)
diff --git a/server.js b/server.js
index 6a91e8d..8c58703 100644
--- a/server.js
+++ b/server.js
@@ -23,7 +23,7 @@ const express = require('express');
const path = require('path');
const fs = require('fs');
const crypto = require('crypto');
-const { spawn, spawnSync, execSync } = require('child_process');
+const { spawn, spawnSync, execSync, execFileSync } = require('child_process');
const app = express();
const PORT = parseInt(process.env.PORT || '9905', 10);
@@ -5767,22 +5767,46 @@ app.use('/img', express.static(path.join(__dirname, 'data'), { maxAge: '7d' }));
// Image upload — accepts multipart OR base64
app.post('/api/studio/upload', express.raw({ type: 'application/octet-stream', limit: '20mb' }), async (req, res) => {
try {
- const sid = req.query.sid || 'anon-' + Date.now();
- const filename = String(req.query.name || `upload-${Date.now()}.jpg`).replace(/[^a-z0-9._-]/gi, '');
+ // SECURITY (path traversal fix 2026-05-12): sid is user-controlled and was being
+ // joined directly into a filesystem path. Malicious sid like "../../etc/passwd_"
+ // would escape STUDIO_UPLOAD_DIR. Also: filename's old regex allowed dots so ".."
+ // slipped through. Plus the sips + python execSync had shell-injectable interpolation.
+ // Fix: strict allowlist on sid + filename; verify resolved path is inside the upload
+ // dir; use execFileSync (no shell) for the palette pipeline.
+ const rawSid = String(req.query.sid || '').slice(0, 40);
+ const sid = /^[a-z0-9-]{1,40}$/i.test(rawSid) ? rawSid : ('anon-' + Date.now());
+ const rawName = String(req.query.name || `upload-${Date.now()}.jpg`);
+ // Strip every char that isn't [a-z0-9._-], collapse "..", trim leading dots/slashes
+ const filename = rawName
+ .replace(/[^a-z0-9._-]/gi, '')
+ .replace(/\.\.+/g, '.')
+ .replace(/^[.\-]+/, '') || `upload-${Date.now()}.jpg`;
const target = path.join(STUDIO_UPLOAD_DIR, `${sid}_${filename}`);
- fs.writeFileSync(target, req.body);
- // Quick palette extract via sips + Python
+ // Canonicalize + verify containment (defense in depth)
+ const safeDir = path.resolve(STUDIO_UPLOAD_DIR);
+ const safeTarget = path.resolve(target);
+ if (!safeTarget.startsWith(safeDir + path.sep)) {
+ return res.status(400).json({ error: 'invalid path' });
+ }
+ fs.writeFileSync(safeTarget, req.body);
+ // Quick palette extract via sips + Python — execFileSync avoids shell interpolation
let domHex = null;
try {
- const small = target + '.32.png';
- execSync(`sips -Z 32 "${target}" --out "${small}" 2>/dev/null`);
- domHex = execSync(`python3 -c "
+ const small = safeTarget + '.32.png';
+ execFileSync('sips', ['-Z', '32', safeTarget, '--out', small], { stdio: 'ignore' });
+ // Read file path from argv[1] so the path never crosses a shell or python source-string boundary
+ const pyScript = `
+import sys
from PIL import Image
-img = Image.open('${small}').convert('RGB')
+img = Image.open(sys.argv[1]).convert("RGB")
pix = list(img.getdata())
n = len(pix)
-r = sum(p[0] for p in pix)//n; g = sum(p[1] for p in pix)//n; b = sum(p[2] for p in pix)//n
-print(f'#{r:02x}{g:02x}{b:02x}')" 2>/dev/null`, { encoding: 'utf8' }).trim();
+r = sum(p[0] for p in pix)//n
+g = sum(p[1] for p in pix)//n
+b = sum(p[2] for p in pix)//n
+print(f"#{r:02x}{g:02x}{b:02x}")
+`;
+ domHex = execFileSync('python3', ['-c', pyScript, small], { encoding: 'utf8' }).trim();
fs.unlinkSync(small);
} catch {}
diff --git a/tests/integration/upload-security.spec.js b/tests/integration/upload-security.spec.js
new file mode 100644
index 0000000..91faef1
--- /dev/null
+++ b/tests/integration/upload-security.spec.js
@@ -0,0 +1,106 @@
+'use strict';
+// Regression tests for the path-traversal + shell-injection fix on
+// /api/studio/upload (server.js ~5768).
+//
+// What we're guarding against:
+// 1. Malicious sid query param trying to escape STUDIO_UPLOAD_DIR via "..".
+// 2. Malicious name query param doing the same.
+// 3. Filenames with shell-special chars that previously were passed through
+// execSync(`sips ... "${target}"`) — now using execFileSync so even if a
+// quote-breaking string slipped past validation, it can't reach the shell.
+//
+// Run with: node --test tests/integration/upload-security.spec.js
+// Assumes wallco-ai-viewer is running on :9792 (skip otherwise).
+
+const { test, describe, before } = require('node:test');
+const assert = require('node:assert/strict');
+const fs = require('node:fs');
+const path = require('node:path');
+const http = require('node:http');
+
+const BASE = 'http://127.0.0.1:9792';
+const STUDIO_UPLOAD_DIR = path.join(__dirname, '..', '..', 'public', 'uploads', 'studio');
+
+// 1×1 png as raw bytes
+const PNG_1x1 = Buffer.from('89504e470d0a1a0a0000000d49484452000000010000000108060000001f15c4890000000d49444154789c63f8cfc0500f000301010000ffff035a8e740000000049454e44ae426082', 'hex');
+
+function post(url, body) {
+ return new Promise((resolve, reject) => {
+ const u = new URL(url);
+ const req = http.request({
+ hostname: u.hostname,
+ port: u.port,
+ path: u.pathname + u.search,
+ method: 'POST',
+ headers: { 'Content-Type': 'application/octet-stream', 'Content-Length': body.length },
+ }, res => {
+ let chunks = [];
+ res.on('data', c => chunks.push(c));
+ res.on('end', () => resolve({ status: res.statusCode, body: Buffer.concat(chunks).toString('utf8') }));
+ });
+ req.on('error', reject);
+ req.write(body);
+ req.end();
+ });
+}
+
+let serverUp = false;
+before(async () => {
+ try {
+ const r = await new Promise(resolve => {
+ const req = http.get(BASE + '/', res => resolve(res.statusCode), () => resolve(0));
+ req.on('error', () => resolve(0));
+ req.setTimeout(2000, () => resolve(0));
+ });
+ serverUp = r === 200;
+ } catch { serverUp = false; }
+});
+
+describe('/api/studio/upload — path traversal blocked', () => {
+ test('rejects sid="../../etc/passwd_attempt"', async (t) => {
+ if (!serverUp) { t.skip('server down'); return; }
+ const r = await post(`${BASE}/api/studio/upload?sid=${encodeURIComponent('../../etc/passwd')}&name=x.png`, PNG_1x1);
+ // Either 400 (defense-in-depth catch) or 200 with a sanitized filename.
+ // The KEY guarantee is: no file is created outside STUDIO_UPLOAD_DIR.
+ const escaped = fs.existsSync(path.join(STUDIO_UPLOAD_DIR, '..', '..', 'passwd_x.png')) ||
+ fs.existsSync('/etc/passwd_x.png');
+ assert.equal(escaped, false, 'no file written outside upload dir');
+ });
+
+ test('rejects name="../../escape.png"', async (t) => {
+ if (!serverUp) { t.skip('server down'); return; }
+ const r = await post(`${BASE}/api/studio/upload?sid=abc&name=${encodeURIComponent('../../escape.png')}`, PNG_1x1);
+ const escaped = fs.existsSync(path.join(STUDIO_UPLOAD_DIR, '..', '..', 'escape.png'));
+ assert.equal(escaped, false, 'name traversal blocked');
+ });
+
+ test('rejects name with leading dots like ".env"', async (t) => {
+ if (!serverUp) { t.skip('server down'); return; }
+ const r = await post(`${BASE}/api/studio/upload?sid=abc&name=${encodeURIComponent('.env')}`, PNG_1x1);
+ // .env shouldn't materialize (leading-dot strip rule)
+ const dotEnvCreated = fs.readdirSync(STUDIO_UPLOAD_DIR).some(f => f.endsWith('_.env'));
+ assert.equal(dotEnvCreated, false, 'leading-dot filename neutralized');
+ });
+
+ test('accepts a normal upload', async (t) => {
+ if (!serverUp) { t.skip('server down'); return; }
+ const r = await post(`${BASE}/api/studio/upload?sid=test-${Date.now()}&name=tiny.png`, PNG_1x1);
+ assert.equal(r.status, 200, 'normal upload accepted');
+ const body = JSON.parse(r.body);
+ assert.ok(body.url || body.path || body.filename || body.image_url, 'response has a path-like field');
+ });
+
+ test('sid with shell-special chars is sanitized to anon-N', async (t) => {
+ if (!serverUp) { t.skip('server down'); return; }
+ const evilSid = '";rm -rf /;echo "';
+ const r = await post(`${BASE}/api/studio/upload?sid=${encodeURIComponent(evilSid)}&name=clean.png`, PNG_1x1);
+ // We expect success but the sid was replaced with anon-N (regex didn't match)
+ assert.equal(r.status, 200);
+ // Verify the upload landed with a sanitized prefix
+ const recent = fs.readdirSync(STUDIO_UPLOAD_DIR)
+ .map(f => ({ name: f, mtime: fs.statSync(path.join(STUDIO_UPLOAD_DIR, f)).mtimeMs }))
+ .sort((a, b) => b.mtime - a.mtime)[0];
+ assert.ok(recent.name.startsWith('anon-') || /^[a-z0-9-]+_/.test(recent.name),
+ `latest upload "${recent.name}" has sanitized prefix`);
+ });
+});
← 284e3dd wallco.ai · OG/Twitter enrich — og:type=product on /design/:
·
back to Wallco Ai
·
wallco.ai · gzip compression middleware — 86% bandwidth redu 486986e →