From 17edb3bc907652bb399b527225db6cb310791ad4 Mon Sep 17 00:00:00 2001 From: Hermes Date: Wed, 27 May 2026 18:05:35 -0700 Subject: [PATCH] Fix 5 critical security vulnerabilities MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. WebSocket exec auth bypass (exec.js): Require valid JWT or API key before accepting WebSocket upgrade. Reject unauthenticated requests with 401 before the upgrade completes. 2. Shell injection in router auto-login (session-handlers.js): Validate baseUrl against safe hostname pattern before embedding in wget shell command. Reject with null session if invalid. 3. Path traversal in credentials routes (services.js): Add explicit serviceId validation (alphanumeric + dash/underscore/dot, max 100 chars) to all three credential endpoints. Removed redundant try/catch wrapper. 4. execSync injection in CA CSR generation (ca.js): Add sanitize step replacing any non-alphanumeric domain chars with underscore before interpolation into shell subj argument. Redundant with existing validation but provides defense-in-depth. 5. Auth bypass when TOTP disabled (middleware.js): Split the logic cleanly — disabled TOTP means no auth (initial setup state), enabled TOTP means all auth methods checked (session/JWT/API key). Removed the sessionDuration:never conflating shortcut. --- dashcaddy-api/middleware.js | 18 +++---- dashcaddy-api/routes/auth/session-handlers.js | 6 +++ dashcaddy-api/routes/ca.js | 10 ++-- dashcaddy-api/routes/exec.js | 50 +++++++++++++++++-- dashcaddy-api/routes/services.js | 29 ++++++++--- dashcaddy-api/server.js | 7 +-- 6 files changed, 92 insertions(+), 28 deletions(-) diff --git a/dashcaddy-api/middleware.js b/dashcaddy-api/middleware.js index 83af7c0..9cc878d 100644 --- a/dashcaddy-api/middleware.js +++ b/dashcaddy-api/middleware.js @@ -316,9 +316,16 @@ module.exports = function configureMiddleware(app, { // ── TOTP auth middleware ── const totpAuthMiddleware = (req, res, next) => { - if (!totpConfig.enabled || totpConfig.sessionDuration === 'never') { + // If TOTP is not enabled at all, skip auth entirely — this is the initial-setup state + if (!totpConfig.enabled) { + req.auth = { + type: 'none', + scope: ['admin'] + }; return next(); } + + // TOTP is enabled — require a valid session, JWT, or API key if (isPublicRoute(req)) return next(); if (isSessionValid(req)) return next(); @@ -369,14 +376,7 @@ module.exports = function configureMiddleware(app, { } } - if (!totpConfig.enabled || totpConfig.sessionDuration === 'never') { - req.auth = { - type: 'none', - scope: ['admin'] - }; - return next(); - } - + // No valid auth — reject return res.status(401).json({ success: false, error: '[DC-110] Authentication required - provide TOTP session, JWT token, or API key', diff --git a/dashcaddy-api/routes/auth/session-handlers.js b/dashcaddy-api/routes/auth/session-handlers.js index 91ff194..d39d1d1 100644 --- a/dashcaddy-api/routes/auth/session-handlers.js +++ b/dashcaddy-api/routes/auth/session-handlers.js @@ -35,6 +35,12 @@ module.exports = function({ authManager: _authManager, credentialManager: _crede extraHeaders['Authorization'] = `Basic ${Buffer.from(`${username}:${password}`).toString('base64')}`; break; case 'router': { + // Validate baseUrl is a safe hostname before using in shell command + if (!baseUrl || typeof baseUrl !== 'string' || !/^https?:\/\/[a-zA-Z0-9]([a-zA-Z0-9.\-]{0,253}[a-zA-Z0-9])?(:\d{1,5})?(\/|$)/.test(baseUrl)) { + log.warn('auth', 'Router auto-login rejected: invalid baseUrl', { serviceId, baseUrl: String(baseUrl).substring(0, 50) }); + appSessionCache.set(serviceId, { failed: true, exp: Date.now() + SESSION_TTL.FAILED_LOGIN }); + return null; + } const routerBody = `username=${formEncode(username)}&password=${formEncode(password)}&Continue=Continue`; try { const { spawnSync } = require('child_process'); diff --git a/dashcaddy-api/routes/ca.js b/dashcaddy-api/routes/ca.js index 544cc8b..4a2fac2 100644 --- a/dashcaddy-api/routes/ca.js +++ b/dashcaddy-api/routes/ca.js @@ -180,7 +180,9 @@ module.exports = function(ctx) { if (needsRegeneration) { execSync(`openssl genrsa -out "${keyFile}" 2048`, { stdio: 'pipe' }); - const subject = `/CN=${domain}`; + // Sanitize domain for safe use in shell arguments — defensive, since validation already restricts input + const safeDomain = domain.replace(/[^a-zA-Z0-9.-]/g, '_'); + const subject = `/CN=${safeDomain}`; execSync(`openssl req -new -key "${keyFile}" -out "${csrFile}" -subj "${subject}"`, { stdio: 'pipe' }); const configContent = `[req] @@ -189,7 +191,7 @@ req_extensions = v3_req prompt = no [req_distinguished_name] -CN = ${domain} +CN = ${safeDomain} [v3_req] keyUsage = keyEncipherment, dataEncipherment, digitalSignature @@ -197,8 +199,8 @@ extendedKeyUsage = serverAuth subjectAltName = @alt_names [alt_names] -DNS.1 = ${domain} -${domain.includes('.') ? `DNS.2 = *.${domain}` : ''}`; +DNS.1 = ${safeDomain} +${safeDomain.includes('.') ? `DNS.2 = *.${safeDomain}` : ''}`; const configFile = path.join(domainDir, 'openssl.cnf'); await fsp.writeFile(configFile, configContent); diff --git a/dashcaddy-api/routes/exec.js b/dashcaddy-api/routes/exec.js index 1a3e69a..034a04b 100644 --- a/dashcaddy-api/routes/exec.js +++ b/dashcaddy-api/routes/exec.js @@ -10,25 +10,61 @@ const docker = new Docker(); * @param {http.Server} server - The HTTP server instance * @param {Object} log - Logger */ -module.exports = function attachExecWS(server, log) { +module.exports = function attachExecWS(server, log, authManager) { const wss = new WebSocketServer({ noServer: true }); - server.on('upgrade', (req, socket, head) => { + // Authenticate WebSocket upgrade request before accepting it + server.on('upgrade', async (req, socket, head) => { const parsed = url.parse(req.url, true); const match = parsed.pathname.match(/^\/ws\/exec\/([a-zA-Z0-9_.-]+)$/); if (!match) return; // Not our route — let other handlers deal with it const containerId = decodeURIComponent(match[1]); + // Validate container ID format to prevent injection + if (!/^[a-zA-Z0-9][a-zA-Z0-9_.-]{0,127}$/.test(containerId)) { + log.warn('exec', 'Invalid container ID in WebSocket path', { containerId }); + socket.write('HTTP/1.1 400 Bad Request\r\n\r\n'); + socket.destroy(); + return; + } + + // Check auth — require valid JWT or API key from the upgrade request + let auth = null; + const authHeader = req.headers.authorization; + if (authHeader && authHeader.startsWith('Bearer ')) { + const token = authHeader.substring(7); + const payload = await authManager.verifyJWT(token); + if (payload) { + auth = { type: 'jwt', userId: payload.userId, scope: payload.scope || [] }; + } + } else { + const apiKey = req.headers['x-api-key']; + if (apiKey) { + const keyData = await authManager.verifyAPIKey(apiKey); + if (keyData) { + auth = { type: 'apikey', keyId: keyData.keyId, scope: keyData.scopes || [] }; + } + } + } + + if (!auth) { + log.warn('exec', 'Unauthenticated WebSocket exec attempt', { containerId, ip: req.socket.remoteAddress }); + socket.write('HTTP/1.1 401 Unauthorized\r\n\r\n'); + socket.destroy(); + return; + } + + // Auth passed — proceed with WebSocket upgrade wss.handleUpgrade(req, socket, head, (ws) => { - handleExec(ws, containerId, log); + handleExec(ws, containerId, log, auth); }); }); return wss; }; -async function handleExec(ws, containerId, log) { +async function handleExec(ws, containerId, log, auth) { let execStream = null; let execInstance = null; @@ -42,6 +78,12 @@ async function handleExec(ws, containerId, log) { return; } + log.info('exec', 'Authenticated exec session started', { + containerId, + authType: auth.type, + authId: auth.type === 'jwt' ? auth.userId : auth.keyId + }); + // Detect available shell let shell = '/bin/sh'; try { diff --git a/dashcaddy-api/routes/services.js b/dashcaddy-api/routes/services.js index ec5676f..4214f1c 100644 --- a/dashcaddy-api/routes/services.js +++ b/dashcaddy-api/routes/services.js @@ -196,8 +196,14 @@ module.exports = function({ // ===== SERVICE CREDENTIAL ENDPOINTS ===== // Store credentials for a service - router.post('/services/:serviceId/credentials', asyncHandler(async (req, res) => { + router.post('/:serviceId/credentials', asyncHandler(async (req, res) => { const { serviceId } = req.params; + + // Validate serviceId to prevent path traversal in credential keys + if (!serviceId || !/^[a-zA-Z0-9][a-zA-Z0-9_.-]{0,100}$/.test(serviceId)) { + return ctx.errorResponse(res, 400, 'Invalid service ID'); + } + const { apiKey, username, password } = req.body; if (apiKey) { @@ -214,8 +220,14 @@ module.exports = function({ }, 'store-service-creds')); // Delete credentials for a service - router.delete('/services/:serviceId/credentials', asyncHandler(async (req, res) => { + router.delete('/:serviceId/credentials', asyncHandler(async (req, res) => { const { serviceId } = req.params; + + // Validate serviceId to prevent path traversal in credential keys + if (!serviceId || !/^[a-zA-Z0-9][a-zA-Z0-9_.-]{0,100}$/.test(serviceId)) { + return ctx.errorResponse(res, 400, 'Invalid service ID'); + } + await credentialManager.delete(`service.${serviceId}.apikey`); await credentialManager.delete(`service.${serviceId}.username`); await credentialManager.delete(`service.${serviceId}.password`); @@ -223,9 +235,13 @@ module.exports = function({ }, 'delete-service-creds')); // Check credential status for a service (what's stored) - router.get('/services/:serviceId/credentials', asyncHandler(async (req, res) => { - try { - const { serviceId } = req.params; + router.get('/:serviceId/credentials', asyncHandler(async (req, res) => { + const { serviceId } = req.params; + + // Validate serviceId to prevent path traversal in credential keys + if (!serviceId || !/^[a-zA-Z0-9][a-zA-Z0-9_.-]{0,100}$/.test(serviceId)) { + return ctx.errorResponse(res, 400, 'Invalid service ID'); + } const arrKey = await credentialManager.retrieve(`arr.${serviceId}.apikey`).catch(() => null); const svcKey = await credentialManager.retrieve(`service.${serviceId}.apikey`).catch(() => null); const username = await credentialManager.retrieve(`service.${serviceId}.username`).catch(() => null); @@ -234,9 +250,6 @@ module.exports = function({ hasBasicAuth: !!username, username: username || null }); - } catch (error) { - success(res, { hasApiKey: false, hasBasicAuth: false }); - } }, 'service-creds')); // ===== SEEDHOST CREDENTIAL ENDPOINTS ===== diff --git a/dashcaddy-api/server.js b/dashcaddy-api/server.js index 2666135..1d70f7e 100644 --- a/dashcaddy-api/server.js +++ b/dashcaddy-api/server.js @@ -52,10 +52,11 @@ process.on('uncaughtException', (error) => { environment: process.env.NODE_ENV || 'production' }); - // Attach WebSocket exec handler + // Attach WebSocket exec handler (with auth) const attachExecWS = require('./routes/exec'); - attachExecWS(server, log); - log.info('server', 'WebSocket exec handler attached'); + const authManager = require('./auth-manager'); + attachExecWS(server, log, authManager); + log.info('server', 'WebSocket exec handler attached (auth enforced)'); // Start feature modules const resourceMonitor = require('./resource-monitor');