diff --git a/server/routes/admin.js b/server/routes/admin.js index fc6c2f6..431a786 100644 --- a/server/routes/admin.js +++ b/server/routes/admin.js @@ -38,6 +38,9 @@ router.post('/users', authenticateToken, requireAdmin, async (req, res) => { if (password.length < 8) { return res.status(400).json({ error: 'Password must be at least 8 characters long' }); } + if (password.length > 64) { + return res.status(400).json({ error: 'Password must not exceed 64 characters' }); + } // M9: email format validation if (!EMAIL_RE.test(email)) { @@ -160,6 +163,9 @@ router.put('/users/:id/password', authenticateToken, requireAdmin, async (req, r if (!newPassword || typeof newPassword !== 'string' || newPassword.length < 8) { return res.status(400).json({ error: 'Password must be at least 8 characters long' }); } + if (newPassword.length > 64) { + return res.status(400).json({ error: 'Password must not exceed 64 characters' }); + } const db = getDb(); const hash = await bcrypt.hash(newPassword, 12); diff --git a/server/routes/analytics.js b/server/routes/analytics.js index 291864b..0a3f6b2 100644 --- a/server/routes/analytics.js +++ b/server/routes/analytics.js @@ -15,8 +15,10 @@ router.post('/callback/:uid', async (req, res) => { const { token } = req.query; const expectedToken = getAnalyticsToken(req.params.uid); - // Constant-time comparison to prevent timing attacks - if (!token || token.length !== expectedToken.length || + // Constant-time comparison to prevent timing attacks. + // Reject non-string tokens (e.g. ?token=a&token=b would yield an array and + // crash Buffer.from). + if (typeof token !== 'string' || token.length !== expectedToken.length || !crypto.timingSafeEqual(Buffer.from(token), Buffer.from(expectedToken))) { return res.status(403).json({ error: 'Invalid token' }); } @@ -216,15 +218,20 @@ router.get('/:id/export/:format', authenticateToken, async (req, res) => { const safeName = (entry.meeting_name || 'analytics').replace(/[^a-zA-Z0-9_-]/g, '_'); if (format === 'csv') { + // Prefix-escape values that start with a formula trigger character so that + // Excel/LibreOffice do not evaluate them as formulas (CSV injection). + const escapeCsv = (val) => { + if (val === null || val === undefined) return ''; + let s = String(val); + if (/^[=+\-@\t\r]/.test(s)) s = "'" + s; + if (s.includes(',') || s.includes('"') || s.includes('\n') || s.includes('\r')) { + return '"' + s.replace(/"/g, '""') + '"'; + } + return s; + }; const header = COLUMNS.map(c => c.header).join(','); const csvRows = rows.map(r => - COLUMNS.map(c => { - const val = r[c.key]; - if (typeof val === 'string' && (val.includes(',') || val.includes('"') || val.includes('\n'))) { - return '"' + val.replace(/"/g, '""') + '"'; - } - return val; - }).join(',') + COLUMNS.map(c => escapeCsv(r[c.key])).join(',') ); const csv = [header, ...csvRows].join('\n'); res.setHeader('Content-Type', 'text/csv; charset=utf-8'); @@ -233,10 +240,19 @@ router.get('/:id/export/:format', authenticateToken, async (req, res) => { } if (format === 'xlsx') { + // Prefix-escape strings that would otherwise be evaluated as formulas. + const sanitizeXlsx = (r) => { + const out = {}; + for (const k of Object.keys(r)) { + const v = r[k]; + out[k] = (typeof v === 'string' && /^[=+\-@\t\r]/.test(v)) ? "'" + v : v; + } + return out; + }; const workbook = new ExcelJS.Workbook(); const sheet = workbook.addWorksheet('Analytics'); sheet.columns = COLUMNS; - rows.forEach(r => sheet.addRow(r)); + rows.forEach(r => sheet.addRow(sanitizeXlsx(r))); // Style header row sheet.getRow(1).font = { bold: true }; sheet.getRow(1).fill = { type: 'pattern', pattern: 'solid', fgColor: { argb: 'FFE0E0E0' } }; diff --git a/server/routes/auth.js b/server/routes/auth.js index c070506..09f18a2 100644 --- a/server/routes/auth.js +++ b/server/routes/auth.js @@ -43,6 +43,13 @@ const SAFE_ID_RE = /^[a-zA-Z0-9_-]{1,50}$/; const SAFE_COLOR_RE = /^(?:#[0-9a-fA-F]{3,8}|hsl\(\d{1,3},\s*\d{1,3}%,\s*\d{1,3}%\)|[a-zA-Z]{1,30})$/; const MIN_PASSWORD_LENGTH = 8; +// bcrypt only uses the first 72 bytes; cap input to prevent CPU-DoS on hashing. +const MAX_PASSWORD_LENGTH = 64; + +// Pre-computed bcrypt hash of a random string used as a dummy comparison +// target when the requested account does not exist. Keeps login timing +// roughly constant so we do not leak whether an email is registered. +const DUMMY_BCRYPT_HASH = bcrypt.hashSync('dummy-password-for-timing-' + Math.random(), 12); // ── Rate Limiters ──────────────────────────────────────────────────────────── const loginLimiter = rateLimit({ @@ -168,6 +175,9 @@ router.post('/register', registerLimiter, async (req, res) => { if (password.length < MIN_PASSWORD_LENGTH) { return res.status(400).json({ error: `Password must be at least ${MIN_PASSWORD_LENGTH} characters long` }); } + if (password.length > MAX_PASSWORD_LENGTH) { + return res.status(400).json({ error: `Password must not exceed ${MAX_PASSWORD_LENGTH} characters` }); + } const existing = await db.get('SELECT id FROM users WHERE email = ?', [email]); if (existing) { @@ -351,10 +361,18 @@ router.post('/login', loginLimiter, async (req, res) => { return res.status(401).json({ error: 'Invalid credentials' }); } + // Cap password length to keep bcrypt CPU work bounded. + if (typeof password !== 'string' || password.length > MAX_PASSWORD_LENGTH) { + return res.status(401).json({ error: 'Invalid credentials' }); + } + const db = getDb(); const user = await db.get('SELECT * FROM users WHERE email = ?', [email.toLowerCase()]); - if (!user || !bcrypt.compareSync(password, user.password_hash)) { + // Always run bcrypt against either the real hash or a dummy hash so login + // timing does not reveal whether the email is registered. + const passwordOk = bcrypt.compareSync(password, user?.password_hash || DUMMY_BCRYPT_HASH); + if (!user || !passwordOk) { return res.status(401).json({ error: 'Invalid credentials' }); } @@ -560,6 +578,9 @@ router.put('/password', authenticateToken, passwordLimiter, async (req, res) => if (typeof currentPassword !== 'string' || typeof newPassword !== 'string') { return res.status(400).json({ error: 'Invalid input' }); } + if (currentPassword.length > MAX_PASSWORD_LENGTH || newPassword.length > MAX_PASSWORD_LENGTH) { + return res.status(400).json({ error: `Password must not exceed ${MAX_PASSWORD_LENGTH} characters` }); + } const db = getDb(); @@ -823,6 +844,9 @@ router.post('/2fa/disable', authenticateToken, twoFaLimiter, async (req, res) => if (!password || !code) { return res.status(400).json({ error: 'Password and code are required' }); } + if (typeof password !== 'string' || password.length > MAX_PASSWORD_LENGTH) { + return res.status(400).json({ error: 'Invalid password' }); + } const db = getDb(); const user = await db.get('SELECT password_hash, totp_secret, totp_enabled FROM users WHERE id = ?', [req.user.id]); diff --git a/server/routes/oauth.js b/server/routes/oauth.js index 811e7de..902012a 100644 --- a/server/routes/oauth.js +++ b/server/routes/oauth.js @@ -200,7 +200,13 @@ router.get('/callback', callbackLimiter, async (req, res) => { ); if (user) { - // Link OAuth to existing account + // Only auto-link to an existing local account if the IdP has actually + // verified the email. Otherwise an attacker who registers at the IdP + // with someone else's email could take over the local account. + if (userInfo.email_verified !== true) { + log.auth.warn(`OAuth account-linking blocked: provider did not assert email_verified=true for ${email}`); + return errorRedirect('Your OAuth provider has not verified this email address. Please verify it with the provider before logging in here.'); + } await db.run( 'UPDATE users SET oauth_provider = ?, oauth_provider_id = ?, email_verified = 1, updated_at = CURRENT_TIMESTAMP WHERE id = ?', ['oidc', sub, user.id], diff --git a/server/routes/rooms.js b/server/routes/rooms.js index 6ebbc78..9d2750d 100644 --- a/server/routes/rooms.js +++ b/server/routes/rooms.js @@ -24,6 +24,14 @@ import { discoverInstance, } from '../config/federation.js'; +// Avatar image filenames are produced by the upload endpoint as +// "_.". Reject anything that doesn't match this shape +// so a guest can't smuggle path traversal or arbitrary URL fragments through +// the unauthenticated guest-join endpoint. +const SAFE_AVATAR_FILENAME_RE = /^[0-9]+_[0-9]+\.(jpg|png|gif|webp)$/; +// Mirrors auth.js: only allow hex/hsl/named CSS colors. +const SAFE_AVATAR_COLOR_RE = /^(?:#[0-9a-fA-F]{3,8}|hsl\(\d{1,3},\s*\d{1,3}%,\s*\d{1,3}%\)|[a-zA-Z]{1,30})$/; + // L6: constant-time string comparison for access/moderator codes function timingSafeEqual(a, b) { if (typeof a !== 'string' || typeof b !== 'string') return false; @@ -664,15 +672,18 @@ router.post('/:uid/guest-join', guestJoinLimiter, async (req, res) => { } const baseUrl = getBaseUrl(req); + // Validate client-supplied avatar fields before embedding them in a URL + // that BBB will fetch — guest-join is unauthenticated. + const safeAvatarImage = (typeof avatar_image === 'string' && SAFE_AVATAR_FILENAME_RE.test(avatar_image)) + ? avatar_image : null; + const safeAvatarColor = (typeof avatar_color === 'string' && SAFE_AVATAR_COLOR_RE.test(avatar_color)) + ? avatar_color : null; let guestAvatarURL; - if (avatar_image) { - // Use avatar image of the logged-in user - guestAvatarURL = `${baseUrl}/api/auth/avatar/${avatar_image}`; - } else if (avatar_color) { - // Initials with user color - guestAvatarURL = `${baseUrl}/api/auth/avatar/initials/${encodeURIComponent(name.trim())}?color=${encodeURIComponent(avatar_color)}`; + if (safeAvatarImage) { + guestAvatarURL = `${baseUrl}/api/auth/avatar/${encodeURIComponent(safeAvatarImage)}`; + } else if (safeAvatarColor) { + guestAvatarURL = `${baseUrl}/api/auth/avatar/initials/${encodeURIComponent(name.trim())}?color=${encodeURIComponent(safeAvatarColor)}`; } else { - // Default: initials without color guestAvatarURL = `${baseUrl}/api/auth/avatar/initials/${encodeURIComponent(name.trim())}`; } const joinUrl = await joinMeeting(room.uid, name.trim(), isModerator, guestAvatarURL);