feat(security): enhance input validation and security measures across various routes

2026-03-04 08:39:29 +01:00
parent ba096a31a2
commit e22a895672
13 changed files with 222 additions and 29 deletions
--- a/server/config/bbb.js
+++ b/server/config/bbb.js
@@ -5,6 +5,20 @@ import { log, fmtDuration, fmtStatus, fmtMethod, fmtReturncode, sanitizeBBBParam
 const BBB_URL = process.env.BBB_URL || 'https://your-bbb-server.com/bigbluebutton/api/';
 const BBB_SECRET = process.env.BBB_SECRET || '';

+if (!BBB_SECRET) {
+  log.bbb.warn('WARNING: BBB_SECRET is not set. BBB API calls will use an empty secret.');
+}
+
+// HTML-escape for safe embedding in BBB welcome messages
+function escapeHtml(str) {
+  return String(str)
+    .replace(/&/g, '&amp;')
+    .replace(/</g, '&lt;')
+    .replace(/>/g, '&gt;')
+    .replace(/"/g, '&quot;')
+    .replace(/'/g, '&#39;');
+}
+
 function getChecksum(apiCall, params) {
  const queryString = new URLSearchParams(params).toString();
  const raw = apiCall + queryString + BBB_SECRET;
@@ -63,13 +77,13 @@ export async function createMeeting(room, logoutURL, loginURL = null, presentati
  const { moderatorPW, attendeePW } = getRoomPasswords(room.uid);

  // Build welcome message with guest invite link
-  let welcome = room.welcome_message || t('defaultWelcome');
+  // HTML-escape user-controlled content to prevent stored XSS via BBB
+  let welcome = room.welcome_message ? escapeHtml(room.welcome_message) : t('defaultWelcome');
  if (logoutURL) {
    const guestLink = `${logoutURL}/join/${room.uid}`;
-    welcome += `<br><br>To invite other participants, share this link:<br><a href="${guestLink}">${guestLink}</a>`;
-    if (room.access_code) {
-      welcome += `<br>Access Code: <b>${room.access_code}</b>`;
-    }
+    welcome += `<br><br>To invite other participants, share this link:<br><a href="${escapeHtml(guestLink)}">${escapeHtml(guestLink)}</a>`;
+    // Access code is intentionally NOT shown in the welcome message to prevent
+    // leaking it to all meeting participants.
  }

  const params = {
--- a/server/config/federation.js
+++ b/server/config/federation.js
@@ -4,6 +4,9 @@ import path from 'path';
 import { fileURLToPath } from 'url';
 import { log } from './logger.js';

+import dns from 'dns';
+import net from 'net';
+
 const __filename = fileURLToPath(import.meta.url);
 const __dirname = path.dirname(__filename);

@@ -93,13 +96,69 @@ export function verifyPayload(payload, signature, remotePublicKeyPem) {
    }
 }

+/**
+ * Check if a domain resolves to a private/internal IP address (SSRF protection).
+ * Blocks RFC 1918, loopback, link-local, and cloud metadata IPs.
+ * @param {string} domain
+ * @returns {Promise<void>} throws if domain resolves to a blocked IP
+ */
+async function assertPublicDomain(domain) {
+  // Allow localhost only in development
+  if (domain === 'localhost' || domain === '127.0.0.1' || domain === '::1') {
+    if (process.env.NODE_ENV === 'production') {
+      throw new Error('Federation to localhost is blocked in production');
+    }
+    return; // allow in dev
+  }
+
+  // If domain is a raw IP, check it directly
+  if (net.isIP(domain)) {
+    if (isPrivateIP(domain)) {
+      throw new Error(`Federation blocked: ${domain} resolves to a private IP`);
+    }
+    return;
+  }
+
+  // Resolve domain and check all resulting IPs
+  const { resolve4, resolve6 } = dns.promises;
+  const ips = [];
+  try { ips.push(...await resolve4(domain)); } catch {}
+  try { ips.push(...await resolve6(domain)); } catch {}
+
+  if (ips.length === 0) {
+    throw new Error(`Federation blocked: could not resolve ${domain}`);
+  }
+
+  for (const ip of ips) {
+    if (isPrivateIP(ip)) {
+      throw new Error(`Federation blocked: ${domain} resolves to a private IP (${ip})`);
+    }
+  }
+}
+
+function isPrivateIP(ip) {
+  // IPv4 private ranges
+  if (/^10\./.test(ip)) return true;
+  if (/^172\.(1[6-9]|2\d|3[01])\./.test(ip)) return true;
+  if (/^192\.168\./.test(ip)) return true;
+  if (/^127\./.test(ip)) return true;
+  if (/^0\./.test(ip)) return true;
+  if (/^169\.254\./.test(ip)) return true; // link-local
+  if (ip === '::1' || ip === '::' || ip.startsWith('fe80:') || ip.startsWith('fc') || ip.startsWith('fd')) return true;
+  return false;
+}
+
 /**
 * Discover a remote Redlight instance's federation API base URL.
 * Fetches https://{domain}/.well-known/redlight and caches the result.
+ * Includes SSRF protection: blocks private/internal IPs.
 * @param {string} domain
 * @returns {Promise<{ baseUrl: string, publicKey: string }>}
 */
 export async function discoverInstance(domain) {
+    // SSRF protection: validate domain doesn't resolve to internal IP
+    await assertPublicDomain(domain);
+
    const cached = discoveryCache.get(domain);
    if (cached && (Date.now() - cached.cachedAt) < DISCOVERY_TTL_MS) {
        return cached;
@@ -112,7 +171,8 @@ export async function discoverInstance(domain) {
        try {
            response = await fetch(wellKnownUrl, { signal: AbortSignal.timeout(TIMEOUT_MS) });
        } catch (e) {
-            if (e.message.includes('fetch') && wellKnownUrl.startsWith('https://localhost')) {
+            // HTTP fallback only allowed in development for localhost
+            if (e.message.includes('fetch') && domain === 'localhost' && process.env.NODE_ENV !== 'production') {
                response = await fetch(`http://${domain}/.well-known/redlight`, { signal: AbortSignal.timeout(TIMEOUT_MS) });
            } else throw e;
        }
@@ -128,7 +188,9 @@ export async function discoverInstance(domain) {

        const baseUrl = `https://${domain}${data.federation_api || '/api/federation'}`;
        const result = {
-            baseUrl: baseUrl.replace('https://localhost', 'http://localhost'),
+            baseUrl: (domain === 'localhost' && process.env.NODE_ENV !== 'production')
+                ? baseUrl.replace('https://localhost', 'http://localhost')
+                : baseUrl,
            publicKey: data.public_key,
            cachedAt: Date.now(),
        };
--- a/server/index.js
+++ b/server/index.js
@@ -30,13 +30,25 @@ const rawTrustProxy = process.env.TRUST_PROXY ?? 'loopback';
 const trustProxy = /^\d+$/.test(rawTrustProxy) ? parseInt(rawTrustProxy, 10) : rawTrustProxy;
 app.set('trust proxy', trustProxy);

+// ── Security headers ───────────────────────────────────────────────────────
+app.use((req, res, next) => {
+  res.setHeader('X-Content-Type-Options', 'nosniff');
+  res.setHeader('X-Frame-Options', 'SAMEORIGIN');
+  res.setHeader('Referrer-Policy', 'strict-origin-when-cross-origin');
+  res.setHeader('Permissions-Policy', 'camera=(), microphone=(), geolocation=()');
+  if (process.env.NODE_ENV === 'production') {
+    res.setHeader('Strict-Transport-Security', 'max-age=31536000; includeSubDomains');
+  }
+  next();
+});
+
 // Middleware
-// M10: restrict CORS in production; allow all in development
+// M10: restrict CORS in production; deny cross-origin by default
 const corsOptions = process.env.APP_URL
  ? { origin: process.env.APP_URL, credentials: true }
-  : {};
+  : { origin: false };
 app.use(cors(corsOptions));
-app.use(express.json());
+app.use(express.json({ limit: '100kb' }));
 // Request/Response logging (filters sensitive fields)
 app.use(requestResponseLogger);

@@ -45,9 +57,10 @@ async function start() {
  await initDatabase();
  initMailer();

-  // Serve uploaded files (avatars, presentations)
+  // Serve uploaded files (avatars, branding only — presentations require auth)
  const uploadsPath = path.join(__dirname, '..', 'uploads');
-  app.use('/uploads', express.static(uploadsPath));
+  app.use('/uploads/avatars', express.static(path.join(uploadsPath, 'avatars')));
+  app.use('/uploads/branding', express.static(path.join(uploadsPath, 'branding')));

  // API Routes
  app.use('/api/auth', authRoutes);
--- a/server/routes/admin.js
+++ b/server/routes/admin.js
@@ -51,7 +51,7 @@ router.post('/users', authenticateToken, requireAdmin, async (req, res) => {
      return res.status(409).json({ error: 'Username is already taken' });
    }

-    const hash = bcrypt.hashSync(password, 12);
+    const hash = await bcrypt.hash(password, 12);
    const result = await db.run(
      'INSERT INTO users (name, display_name, email, password_hash, role, email_verified) VALUES (?, ?, ?, ?, ?, 1)',
      [name, display_name, email.toLowerCase(), hash, validRole]
@@ -156,7 +156,7 @@ router.put('/users/:id/password', authenticateToken, requireAdmin, async (req, r
    }

    const db = getDb();
-    const hash = bcrypt.hashSync(newPassword, 12);
+    const hash = await bcrypt.hash(newPassword, 12);
    await db.run('UPDATE users SET password_hash = ?, updated_at = CURRENT_TIMESTAMP WHERE id = ?', [hash, req.params.id]);

    res.json({ message: 'Password reset' });
--- a/server/routes/auth.js
+++ b/server/routes/auth.js
@@ -168,7 +168,7 @@ router.post('/register', registerLimiter, async (req, res) => {
      return res.status(409).json({ error: 'Username is already taken' });
    }

-    const hash = bcrypt.hashSync(password, 12);
+    const hash = await bcrypt.hash(password, 12);

    // If SMTP is configured, require email verification
    if (isMailerConfigured()) {
@@ -352,7 +352,7 @@ router.post('/login', loginLimiter, async (req, res) => {
    }

    const token = generateToken(user.id);
-    const { password_hash, ...safeUser } = user;
+    const { password_hash, verification_token, verification_token_expires, verification_resend_at, ...safeUser } = user;

    res.json({ token, user: safeUser });
  } catch (err) {
@@ -485,7 +485,7 @@ router.put('/password', authenticateToken, passwordLimiter, async (req, res) =>
      return res.status(400).json({ error: `New password must be at least ${MIN_PASSWORD_LENGTH} characters long` });
    }

-    const hash = bcrypt.hashSync(newPassword, 12);
+    const hash = await bcrypt.hash(newPassword, 12);
    await db.run('UPDATE users SET password_hash = ?, updated_at = CURRENT_TIMESTAMP WHERE id = ?', [hash, req.user.id]);

    res.json({ message: 'Password changed successfully' });
@@ -498,7 +498,7 @@ router.put('/password', authenticateToken, passwordLimiter, async (req, res) =>
 // POST /api/auth/avatar - Upload avatar image
 router.post('/avatar', authenticateToken, avatarLimiter, async (req, res) => {
  try {
-    // Validate content type
+    // Validate file content by checking magic bytes (file signatures)
    const contentType = req.headers['content-type'];
    if (!contentType || !contentType.startsWith('image/')) {
      return res.status(400).json({ error: 'Only image files are allowed' });
@@ -528,7 +528,18 @@ router.post('/avatar', authenticateToken, avatarLimiter, async (req, res) => {
      return res.status(400).json({ error: 'Image must not exceed 2MB' });
    }

-    const ext = contentType.includes('png') ? 'png' : contentType.includes('gif') ? 'gif' : contentType.includes('webp') ? 'webp' : 'jpg';
+    // Validate magic bytes to prevent Content-Type spoofing
+    const magicBytes = buffer.slice(0, 8);
+    const isJPEG = magicBytes[0] === 0xFF && magicBytes[1] === 0xD8 && magicBytes[2] === 0xFF;
+    const isPNG = magicBytes[0] === 0x89 && magicBytes[1] === 0x50 && magicBytes[2] === 0x4E && magicBytes[3] === 0x47;
+    const isGIF = magicBytes[0] === 0x47 && magicBytes[1] === 0x49 && magicBytes[2] === 0x46;
+    const isWEBP = magicBytes[0] === 0x52 && magicBytes[1] === 0x49 && magicBytes[2] === 0x46 && magicBytes[3] === 0x46
+      && buffer.length > 11 && buffer[8] === 0x57 && buffer[9] === 0x45 && buffer[10] === 0x42 && buffer[11] === 0x50;
+    if (!isJPEG && !isPNG && !isGIF && !isWEBP) {
+      return res.status(400).json({ error: 'File content does not match a supported image format (JPEG, PNG, GIF, WebP)' });
+    }
+
+    const ext = isPNG ? 'png' : isGIF ? 'gif' : isWEBP ? 'webp' : 'jpg';
    const filename = `${req.user.id}_${Date.now()}.${ext}`;
    const filepath = path.join(uploadsDir, filename);

--- a/server/routes/branding.js
+++ b/server/routes/branding.js
@@ -14,6 +14,16 @@ const router = Router();

 const SAFE_ID_RE = /^[a-zA-Z0-9_-]{1,50}$/;

+// Validate that a URL uses a safe scheme (http/https only)
+function isSafeUrl(url) {
+  try {
+    const parsed = new URL(url);
+    return parsed.protocol === 'https:' || parsed.protocol === 'http:';
+  } catch {
+    return false;
+  }
+}
+
 // Ensure uploads/branding directory exists
 const brandingDir = path.join(__dirname, '..', '..', 'uploads', 'branding');
 if (!fs.existsSync(brandingDir)) {
@@ -221,6 +231,9 @@ router.put('/imprint-url', authenticateToken, requireAdmin, async (req, res) =>
    if (imprintUrl && imprintUrl.length > 500) {
      return res.status(400).json({ error: 'URL must not exceed 500 characters' });
    }
+    if (imprintUrl && imprintUrl.trim() && !isSafeUrl(imprintUrl.trim())) {
+      return res.status(400).json({ error: 'URL must start with http:// or https://' });
+    }
    if (imprintUrl && imprintUrl.trim()) {
      await setSetting('imprint_url', imprintUrl.trim());
    } else {
@@ -240,6 +253,9 @@ router.put('/privacy-url', authenticateToken, requireAdmin, async (req, res) =>
    if (privacyUrl && privacyUrl.length > 500) {
      return res.status(400).json({ error: 'URL must not exceed 500 characters' });
    }
+    if (privacyUrl && privacyUrl.trim() && !isSafeUrl(privacyUrl.trim())) {
+      return res.status(400).json({ error: 'URL must start with http:// or https://' });
+    }
    if (privacyUrl && privacyUrl.trim()) {
      await setSetting('privacy_url', privacyUrl.trim());
    } else {
--- a/server/routes/caldav.js
+++ b/server/routes/caldav.js
@@ -192,16 +192,28 @@ async function caldavAuth(req, res, next) {
      res.set('WWW-Authenticate', 'Basic realm="Redlight CalDAV"');
      return res.status(401).end();
    }
+    // Hash the provided token with SHA-256 for constant-time comparison in SQL
+    const tokenHash = crypto.createHash('sha256').update(token).digest('hex');
    const tokenRow = await db.get(
+      'SELECT * FROM caldav_tokens WHERE user_id = ? AND token_hash = ?',
+      [user.id, tokenHash],
+    );
+    // Fallback: also check legacy plaintext tokens for backward compatibility
+    const tokenRowLegacy = !tokenRow ? await db.get(
      'SELECT * FROM caldav_tokens WHERE user_id = ? AND token = ?',
      [user.id, token],
-    );
-    if (!tokenRow) {
+    ) : null;
+    const matchedToken = tokenRow || tokenRowLegacy;
+    if (!matchedToken) {
      res.set('WWW-Authenticate', 'Basic realm="Redlight CalDAV"');
      return res.status(401).end();
    }
+    // Migrate legacy plaintext token to hashed version
+    if (tokenRowLegacy && !tokenRow) {
+      db.run('UPDATE caldav_tokens SET token_hash = ?, token = NULL WHERE id = ?', [tokenHash, matchedToken.id]).catch(() => {});
+    }
    // Update last_used_at (fire and forget)
-    db.run('UPDATE caldav_tokens SET last_used_at = CURRENT_TIMESTAMP WHERE id = ?', [tokenRow.id]).catch(() => {});
+    db.run('UPDATE caldav_tokens SET last_used_at = CURRENT_TIMESTAMP WHERE id = ?', [matchedToken.id]).catch(() => {});
    req.caldavUser = user;
    next();
  } catch (err) {
@@ -218,6 +230,15 @@ function setDAVHeaders(res) {
  res.set('MS-Author-Via', 'DAV');
 }

+// ── CalDAV username authorization ──────────────────────────────────────────
+// Ensures the :username param matches the authenticated user's email
+function validateCalDAVUser(req, res, next) {
+  if (req.params.username && decodeURIComponent(req.params.username) !== req.caldavUser.email) {
+    return res.status(403).end();
+  }
+  next();
+}
+
 // ── Base URL helper ────────────────────────────────────────────────────────
 function baseUrl(req) {
  const proto = req.get('x-forwarded-proto') || req.protocol;
@@ -280,7 +301,7 @@ router.all('/', caldavAuth, async (req, res) => {

 // ── PROPFIND /{username}/ ──────────────────────────────────────────────────
 // User principal: tells the client where the calendar home is.
-router.all('/:username/', caldavAuth, async (req, res) => {
+router.all('/:username/', caldavAuth, validateCalDAVUser, async (req, res) => {
  if (req.method !== 'PROPFIND') {
    setDAVHeaders(res);
    return res.status(405).end();
@@ -302,7 +323,7 @@ router.all('/:username/', caldavAuth, async (req, res) => {
 });

 // ── PROPFIND + REPORT /{username}/calendar/ ────────────────────────────────
-router.all('/:username/calendar/', caldavAuth, async (req, res) => {
+router.all('/:username/calendar/', caldavAuth, validateCalDAVUser, async (req, res) => {
  const db = getDb();
  const calendarHref = `/caldav/${encodeURIComponent(req.caldavUser.email)}/calendar/`;

@@ -411,7 +432,7 @@ router.all('/:username/calendar/', caldavAuth, async (req, res) => {
 });

 // ── GET /{username}/calendar/{uid}.ics ────────────────────────────────────
-router.get('/:username/calendar/:filename', caldavAuth, async (req, res) => {
+router.get('/:username/calendar/:filename', caldavAuth, validateCalDAVUser, async (req, res) => {
  const uid = req.params.filename.replace(/\.ics$/, '');
  const db = getDb();
  const ev = await db.get(
@@ -426,7 +447,7 @@ router.get('/:username/calendar/:filename', caldavAuth, async (req, res) => {
 });

 // ── PUT /{username}/calendar/{uid}.ics — create or update ─────────────────
-router.put('/:username/calendar/:filename', caldavAuth, async (req, res) => {
+router.put('/:username/calendar/:filename', caldavAuth, validateCalDAVUser, async (req, res) => {
  const uid = req.params.filename.replace(/\.ics$/, '');
  const body = typeof req.body === 'string' ? req.body : '';

@@ -488,7 +509,7 @@ router.put('/:username/calendar/:filename', caldavAuth, async (req, res) => {
 });

 // ── DELETE /{username}/calendar/{uid}.ics ─────────────────────────────────
-router.delete('/:username/calendar/:filename', caldavAuth, async (req, res) => {
+router.delete('/:username/calendar/:filename', caldavAuth, validateCalDAVUser, async (req, res) => {
  const uid = req.params.filename.replace(/\.ics$/, '');
  const db = getDb();
  const ev = await db.get(
--- a/server/routes/calendar.js
+++ b/server/routes/calendar.js
@@ -16,6 +16,9 @@ import { rateLimit } from 'express-rate-limit';

 const router = Router();

+// Allowlist for CSS color values - only permits hsl(), hex (#rgb/#rrggbb) and plain names
+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})$/;
+
 // Rate limit for federation calendar receive
 const calendarFederationLimiter = rateLimit({
  windowMs: 15 * 60 * 1000,
@@ -121,6 +124,11 @@ router.post('/events', authenticateToken, async (req, res) => {
    if (title.length > 200) return res.status(400).json({ error: 'Title must not exceed 200 characters' });
    if (description && description.length > 5000) return res.status(400).json({ error: 'Description must not exceed 5000 characters' });

+    // Validate color format
+    if (color && !SAFE_COLOR_RE.test(color)) {
+      return res.status(400).json({ error: 'Invalid color format' });
+    }
+
    const startDate = new Date(start_time);
    const endDate = new Date(end_time);
    if (isNaN(startDate.getTime()) || isNaN(endDate.getTime())) {
@@ -172,6 +180,11 @@ router.put('/events/:id', authenticateToken, async (req, res) => {
    if (title && title.length > 200) return res.status(400).json({ error: 'Title must not exceed 200 characters' });
    if (description && description.length > 5000) return res.status(400).json({ error: 'Description must not exceed 5000 characters' });

+    // Validate color format
+    if (color && !SAFE_COLOR_RE.test(color)) {
+      return res.status(400).json({ error: 'Invalid color format' });
+    }
+
    if (start_time && end_time) {
      const s = new Date(start_time);
      const e = new Date(end_time);
--- a/server/routes/federation.js
+++ b/server/routes/federation.js
@@ -161,6 +161,16 @@ router.post('/receive', federationReceiveLimiter, async (req, res) => {
            return res.status(400).json({ error: 'Incomplete invitation payload' });
        }

+        // Validate join_url scheme to prevent javascript: or other malicious URIs
+        try {
+            const parsedUrl = new URL(join_url);
+            if (parsedUrl.protocol !== 'https:' && parsedUrl.protocol !== 'http:') {
+                return res.status(400).json({ error: 'join_url must use https:// or http://' });
+            }
+        } catch {
+            return res.status(400).json({ error: 'Invalid join_url format' });
+        }
+
        // S4: validate field lengths from remote to prevent oversized DB entries
        if (invite_id.length > 100 || from_user.length > 200 || to_user.length > 200 ||
            room_name.length > 200 || join_url.length > 2000 || (message && message.length > 5000)) {
--- a/server/routes/rooms.js
+++ b/server/routes/rooms.js
@@ -709,6 +709,15 @@ router.post('/:uid/presentation', authenticateToken, async (req, res) => {
    const ext = extMap[contentType];
    if (!ext) return res.status(400).json({ error: 'Unsupported file type. Allowed: PDF, PPT, PPTX, ODP, DOC, DOCX' });

+    // Validate magic bytes to prevent Content-Type spoofing
+    const magic = buffer.slice(0, 8);
+    const isPDF = magic[0] === 0x25 && magic[1] === 0x50 && magic[2] === 0x44 && magic[3] === 0x46; // %PDF
+    const isZip = magic[0] === 0x50 && magic[1] === 0x4B && magic[2] === 0x03 && magic[3] === 0x04; // PK (PPTX, DOCX, ODP, etc.)
+    const isOle = magic[0] === 0xD0 && magic[1] === 0xCF && magic[2] === 0x11 && magic[3] === 0xE0; // OLE2 (PPT, DOC)
+    if (ext === 'pdf' && !isPDF) return res.status(400).json({ error: 'File content does not match PDF format' });
+    if (['pptx', 'docx', 'odp'].includes(ext) && !isZip) return res.status(400).json({ error: 'File content does not match expected archive format' });
+    if (['ppt', 'doc'].includes(ext) && !isOle) return res.status(400).json({ error: 'File content does not match expected document format' });
+
    // Preserve original filename (sent as X-Filename header)
    const rawName = req.headers['x-filename'];
    const originalName = rawName