diff --git a/src/core/api/dynamic-router.js b/src/core/api/dynamic-router.js index eaef3c1..191bbf4 100644 --- a/src/core/api/dynamic-router.js +++ b/src/core/api/dynamic-router.js @@ -257,10 +257,16 @@ async function routeModuleRequest(request, path, method) { const params = extractPathParams(route.path, pathString); return await route.handler(request, params); } - } catch (authError) { + } catch (err) { + // Only known auth-error strings are safe to surface; all other + // exceptions are logged server-side and returned as a generic message. + const SAFE_AUTH_MESSAGES = new Set(['Unauthorized', 'Admin access required']); + if (!SAFE_AUTH_MESSAGES.has(err.message)) { + console.error('[ZEN] Module route handler error:', err); + } return { success: false, - error: authError.message + error: SAFE_AUTH_MESSAGES.has(err.message) ? err.message : 'Internal Server Error' }; } } diff --git a/src/core/api/handlers/users.js b/src/core/api/handlers/users.js index 592a612..1b2eedf 100644 --- a/src/core/api/handlers/users.js +++ b/src/core/api/handlers/users.js @@ -8,7 +8,7 @@ import { cookies } from 'next/headers'; import { query, updateById } from '@zen/core/database'; import { getSessionCookieName } from '../../../shared/lib/appConfig.js'; import { updateUser } from '../../../features/auth/lib/auth.js'; -import { uploadImage, deleteFile, generateUniqueFilename, generateUserFilePath, FILE_TYPE_PRESETS, FILE_SIZE_LIMITS, validateUpload } from '@zen/core/storage'; +import { uploadImage, deleteFile, generateUniqueFilename, generateUserFilePath, getFileExtension, FILE_TYPE_PRESETS, FILE_SIZE_LIMITS, validateUpload } from '@zen/core/storage'; // Get cookie name from environment or use default const COOKIE_NAME = getSessionCookieName(); @@ -425,11 +425,17 @@ export async function handleUploadProfilePicture(request) { // Generate storage path const key = generateUserFilePath(session.user.id, 'profile', uniqueFilename); - // Derive the authoritative content-type from the server-side whitelist — - // never trust the client-supplied file.type, which is fully attacker-controlled. - const contentType = ALLOWED_IMAGE_MIME_TYPES.has(file.type) - ? file.type - : 'application/octet-stream'; + // Derive the authoritative content-type from the *validated file extension*, + // never from file.type which is fully attacker-controlled. The extension + // has already been verified against the allowedTypes whitelist above, so + // mapping it deterministically here eliminates any client influence over + // the MIME type stored in R2 and subsequently served to other users. + const EXTENSION_TO_MIME = { + '.jpg': 'image/jpeg', '.jpeg': 'image/jpeg', + '.png': 'image/png', '.gif': 'image/gif', '.webp': 'image/webp', + }; + const ext = getFileExtension(file.name).toLowerCase(); + const contentType = EXTENSION_TO_MIME[ext] ?? 'application/octet-stream'; // Upload to storage const uploadResult = await uploadImage({ diff --git a/src/core/api/nx-route.js b/src/core/api/nx-route.js index 276c1b1..4c39f21 100644 --- a/src/core/api/nx-route.js +++ b/src/core/api/nx-route.js @@ -19,18 +19,33 @@ export async function GET(request, { params }) { // Check if this is a file response (from storage endpoint) if (response.success && response.file) { + const contentType = response.file.contentType || 'application/octet-stream'; const headers = { - 'Content-Type': response.file.contentType || 'application/octet-stream', + 'Content-Type': contentType, 'Content-Length': response.file.contentLength?.toString() || '', 'Cache-Control': 'private, max-age=3600', 'Last-Modified': response.file.lastModified?.toUTCString() || new Date().toUTCString(), 'X-Content-Type-Options': 'nosniff', 'X-Frame-Options': 'DENY', }; - if (response.file.filename) { + + // Always emit an explicit Content-Disposition header — omitting it leaves + // rendering decisions to browser heuristics, which varies by content-type + // and browser version. Image MIME types are served inline (required for + // tags); every other type forces a download to prevent in-browser + // rendering of potentially dangerous content. + const INLINE_MIME_TYPES = new Set([ + 'image/jpeg', 'image/png', 'image/gif', 'image/webp', + ]); + if (INLINE_MIME_TYPES.has(contentType)) { + headers['Content-Disposition'] = 'inline'; + } else if (response.file.filename) { const encoded = encodeURIComponent(response.file.filename); headers['Content-Disposition'] = `attachment; filename="${encoded}"; filename*=UTF-8''${encoded}`; + } else { + headers['Content-Disposition'] = 'attachment'; } + return new NextResponse(response.file.body, { status: 200, headers }); } diff --git a/src/core/api/router.js b/src/core/api/router.js index 0509895..1b89e73 100644 --- a/src/core/api/router.js +++ b/src/core/api/router.js @@ -279,10 +279,18 @@ async function routeModuleRequest(request, path, method) { const params = extractPathParams(route.path, pathString); return await route.handler(request, params); } - } catch (authError) { + } catch (err) { + // Only the two known auth-error strings are safe to surface verbatim. + // Any other exception (database errors, upstream API errors, etc.) must + // never reach the client raw — they can contain credentials, table names, + // or internal hostnames. Log the full error server-side only. + const SAFE_AUTH_MESSAGES = new Set(['Unauthorized', 'Admin access required']); + if (!SAFE_AUTH_MESSAGES.has(err.message)) { + console.error('[ZEN] Module route handler error:', err); + } return { success: false, - error: authError.message + error: SAFE_AUTH_MESSAGES.has(err.message) ? err.message : 'Internal Server Error' }; } } diff --git a/src/core/storage/utils.js b/src/core/storage/utils.js index 2221bc0..ec3196e 100644 --- a/src/core/storage/utils.js +++ b/src/core/storage/utils.js @@ -243,13 +243,56 @@ export const FILE_SIZE_LIMITS = { LARGE_FILE: 1024 * 1024 * 1024, // 1 GB }; +/** + * Known magic-byte sequences for each permitted image extension. + * Keyed by lower-case extension; value is the expected byte sequence at offset 0. + * WEBP requires a special compound check (RIFF....WEBP). + */ +const MAGIC_BYTES = { + '.jpg': [0xFF, 0xD8, 0xFF], + '.jpeg': [0xFF, 0xD8, 0xFF], + '.png': [0x89, 0x50, 0x4E, 0x47], + '.gif': [0x47, 0x49, 0x46, 0x38], // GIF87a or GIF89a +}; + +/** + * Confirm that the file buffer starts with the magic bytes expected for the + * file's declared extension. This is a *positive* assertion — the file must + * prove it is the type it claims to be, not merely fail to match a denylist. + * + * A polyglot that prefixes a real image header before malicious payload still + * satisfies this check, so this function must always be used together with + * inspectBufferForDangerousContent which scans a larger region for script tags. + * + * @param {Buffer} buffer + * @param {string} filename + * @returns {boolean} true when the magic bytes match the declared extension + */ +export function validateMagicBytes(buffer, filename) { + if (!Buffer.isBuffer(buffer) || buffer.length < 12) return false; + const ext = getFileExtension(filename).toLowerCase(); + + // WebP: 'RIFF' at bytes 0-3, 'WEBP' at bytes 8-11 + if (ext === '.webp') { + return buffer.slice(0, 4).toString('ascii') === 'RIFF' + && buffer.slice(8, 12).toString('ascii') === 'WEBP'; + } + + const expected = MAGIC_BYTES[ext]; + if (!expected) return false; + return expected.every((byte, i) => buffer[i] === byte); +} + /** * Inspect the first bytes of a buffer for known-dangerous content signatures * that could indicate an HTML, SVG, or XML file disguised with an image extension. * This is a defence-in-depth layer — it does not replace server-side magic-byte * validation via a dedicated library (e.g. 'sharp' or 'file-type'). * - * @param {Buffer} buffer - First bytes of the uploaded file (minimum 16 bytes) + * The scan window is extended to 512 bytes to make it harder to hide a script + * tag after a short but valid-looking binary header. + * + * @param {Buffer} buffer - File buffer (ideally the full file, at least 512 bytes) * @returns {{ safe: boolean, reason: string|null }} */ export function inspectBufferForDangerousContent(buffer) { @@ -257,13 +300,15 @@ export function inspectBufferForDangerousContent(buffer) { return { safe: false, reason: 'Buffer too short to inspect' }; } - // Convert the first 64 bytes to a lower-case ASCII string for pattern matching. - const head = buffer.slice(0, Math.min(buffer.length, 64)).toString('latin1').toLowerCase(); + // Scan the first 512 bytes (up from 64) to reduce polyglot attack surface. + const head = buffer.slice(0, Math.min(buffer.length, 512)).toString('latin1').toLowerCase(); // Detect HTML/SVG/XML content that could carry executable scripts. const dangerousPatterns = [ /^<(!doctype|html|svg|xml|script)/, /^[\s\ufeff]*<(!doctype|html|svg|xml|script)/, + /]/, + /]/, ]; for (const pattern of dangerousPatterns) { @@ -309,9 +354,17 @@ export function validateUpload({ filename, size, allowedTypes, maxSize, buffer } errors.push(`File size exceeds limit of ${formatFileSize(maxSize)}`); } - // If a buffer is provided, perform a best-effort content inspection to catch - // polyglot files (e.g., valid JPEG header with embedded SVG payload). + // If a buffer is provided, apply two independent layers of content inspection. if (buffer) { + // Layer 1 — Positive magic-byte assertion: the buffer must begin with the + // exact byte sequence that corresponds to the declared file extension. + // This catches files that lie about their extension entirely. + if (filename && !validateMagicBytes(buffer, filename)) { + errors.push('File magic bytes do not match the declared file type'); + } + + // Layer 2 — Dangerous-pattern denylist: scan a wider window for HTML/SVG/XML + // tags that could carry executable scripts (polyglot defence-in-depth). const inspection = inspectBufferForDangerousContent(buffer); if (!inspection.safe) { errors.push(inspection.reason || 'File content failed safety inspection'); diff --git a/src/features/auth/actions/authActions.js b/src/features/auth/actions/authActions.js index c95cb16..2e772a2 100644 --- a/src/features/auth/actions/authActions.js +++ b/src/features/auth/actions/authActions.js @@ -103,14 +103,25 @@ export async function loginAction(formData) { const password = formData.get('password'); const result = await login({ email, password }); - - // Return the token to be set by the client to avoid page refresh - // The client will call setSessionCookie after displaying the success message + + // Set the session cookie directly inside this server action so the token + // never travels through JavaScript-readable response payload. + // An HttpOnly cookie is the only safe transport for session tokens. + const cookieStore = await cookies(); + cookieStore.set(COOKIE_NAME, result.session.token, { + httpOnly: true, + secure: process.env.NODE_ENV === 'production', + sameSite: 'lax', + maxAge: 60 * 60 * 24 * 30, + path: '/' + }); + return { success: true, message: 'Connexion réussie', - user: result.user, - sessionToken: result.session.token + user: result.user + // sessionToken intentionally omitted — it must never appear in a + // JavaScript-accessible response body. }; } catch (error) { return { diff --git a/src/features/auth/lib/auth.js b/src/features/auth/lib/auth.js index eaeb5bf..768e5fd 100644 --- a/src/features/auth/lib/auth.js +++ b/src/features/auth/lib/auth.js @@ -6,7 +6,7 @@ import { create, findOne, updateById, count } from '../../../core/database/crud.js'; import { hashPassword, verifyPassword, generateId } from './password.js'; import { createSession } from './session.js'; -import { createEmailVerification, createPasswordReset, deleteResetToken, sendPasswordChangedEmail } from './email.js'; +import { createEmailVerification, createPasswordReset, verifyResetToken, deleteResetToken, sendPasswordChangedEmail } from './email.js'; /** * Register a new user @@ -210,9 +210,14 @@ async function resetPassword(resetData) { throw new Error('Le mot de passe doit contenir au moins une majuscule, une minuscule et un chiffre'); } - // Verify token is handled in the email module - // For now, we'll assume token is valid if it exists in the database - + // Authoritative token verification — this check must live here so that any + // caller that imports resetPassword() directly (bypassing the server-action + // layer) cannot reset a password with an arbitrary or omitted token. + const tokenValid = await verifyResetToken(email, token); + if (!tokenValid) { + throw new Error('Jeton de réinitialisation invalide ou expiré'); + } + // Find user const user = await findOne('zen_auth_users', { email }); if (!user) { diff --git a/src/features/auth/lib/email.js b/src/features/auth/lib/email.js index cd5b56b..2382983 100644 --- a/src/features/auth/lib/email.js +++ b/src/features/auth/lib/email.js @@ -3,6 +3,7 @@ * Handles email verification tokens and password reset tokens */ +import crypto from 'crypto'; import { create, findOne, deleteWhere } from '../../../core/database/crud.js'; import { generateToken, generateId } from './password.js'; import { sendAuthEmail } from '../../../core/email/index.js'; @@ -55,19 +56,27 @@ async function verifyEmailToken(email, token) { }); if (!verification) return false; - - // Verify token matches - if (verification.token !== token) return false; - + + // Timing-safe comparison — always operate on same-length buffers so that a + // wrong-length guess yields no measurable timing difference from a wrong-value guess. + const storedBuf = Buffer.from(verification.token, 'utf8'); + const providedBuf = Buffer.from( + token.length === verification.token.length ? token : verification.token, + 'utf8' + ); + const tokensMatch = crypto.timingSafeEqual(storedBuf, providedBuf) + && token.length === verification.token.length; + if (!tokensMatch) return false; + // Check if token is expired if (new Date(verification.expires_at) < new Date()) { await deleteWhere('zen_auth_verifications', { id: verification.id }); return false; } - + // Delete the verification token after use await deleteWhere('zen_auth_verifications', { id: verification.id }); - + return true; } @@ -118,16 +127,23 @@ async function verifyResetToken(email, token) { }); if (!reset) return false; - - // Verify token matches - if (reset.token !== token) return false; - + + // Timing-safe comparison — same rationale as verifyEmailToken above. + const storedBuf = Buffer.from(reset.token, 'utf8'); + const providedBuf = Buffer.from( + token.length === reset.token.length ? token : reset.token, + 'utf8' + ); + const tokensMatch = crypto.timingSafeEqual(storedBuf, providedBuf) + && token.length === reset.token.length; + if (!tokensMatch) return false; + // Check if token is expired if (new Date(reset.expires_at) < new Date()) { await deleteWhere('zen_auth_verifications', { id: reset.id }); return false; } - + return true; } diff --git a/src/features/auth/lib/password.js b/src/features/auth/lib/password.js index 0c0a434..36598bf 100644 --- a/src/features/auth/lib/password.js +++ b/src/features/auth/lib/password.js @@ -32,10 +32,18 @@ async function hashPassword(password) { async function verifyPassword(password, hash) { return new Promise((resolve, reject) => { const [salt, key] = hash.split(':'); - + crypto.scrypt(password, salt, 64, (err, derivedKey) => { - if (err) reject(err); - resolve(key === derivedKey.toString('hex')); + if (err) { reject(err); return; } + try { + const storedKey = Buffer.from(key, 'hex'); + // timingSafeEqual requires identical lengths; if the stored hash is + // malformed the lengths will differ and we reject without leaking timing. + if (storedKey.length !== derivedKey.length) { resolve(false); return; } + resolve(crypto.timingSafeEqual(storedKey, derivedKey)); + } catch { + resolve(false); + } }); }); } diff --git a/src/features/auth/lib/rateLimit.js b/src/features/auth/lib/rateLimit.js index 18a29c9..176591d 100644 --- a/src/features/auth/lib/rateLimit.js +++ b/src/features/auth/lib/rateLimit.js @@ -3,8 +3,16 @@ * Stores counters in a Map — resets on server restart, no DB required. */ +// Persist the store on globalThis so that module-cache invalidation (e.g. during +// Next.js hot reload) does not silently reset all counters within the same process. +// CRITICAL LIMITATION: this Map is process-local. In serverless or multi-worker +// deployments every instance maintains its own store and rate limits do not +// distribute across instances. For production deployments with multiple workers +// replace this Map with a shared atomic store (e.g. Redis / Upstash). +const STORE_KEY = Symbol.for('__ZEN_RATE_LIMIT_STORE__'); +if (!globalThis[STORE_KEY]) globalThis[STORE_KEY] = new Map(); /** @type {Map} */ -const store = new Map(); +const store = globalThis[STORE_KEY]; // Purge expired entries every 10 minutes to avoid memory leak const cleanup = setInterval(() => { @@ -76,29 +84,63 @@ export function checkRateLimit(identifier, action) { } /** - * Extract the best-effort client IP from Next.js headers() (server actions). + * Return true only when the string resembles a valid IPv4 or IPv6 address. + * This prevents arbitrary attacker-supplied strings from being used as + * rate-limit identifiers (which could allow bucket manipulation). + * @param {string|null|undefined} ip + * @returns {boolean} + */ +function isValidIp(ip) { + if (!ip || typeof ip !== 'string') return false; + // IPv4 — four decimal octets, each 0-255 + if (/^(\d{1,3}\.){3}\d{1,3}$/.test(ip)) { + return ip.split('.').every(octet => parseInt(octet, 10) <= 255); + } + // IPv6 — simplified structural check (colons + hex groups) + return /^[0-9a-fA-F:]+$/.test(ip) && ip.includes(':'); +} + +/** + * Extract the client IP from Next.js headers() (server actions). + * + * X-Forwarded-For and X-Real-IP are only trusted when ZEN_TRUST_PROXY=true is + * explicitly set, confirming a trusted reverse proxy populates those headers. + * Without this flag the headers are fully attacker-controlled and MUST NOT be + * used as rate-limit keys — an attacker would trivially rotate identifiers. + * + * Set ZEN_TRUST_PROXY=true only when a verified reverse proxy (e.g. Nginx, + * Cloudflare, AWS ALB) strips and rewrites forwarded headers before they reach + * this application. + * * @param {import('next/headers').ReadonlyHeaders} headersList * @returns {string} */ export function getIpFromHeaders(headersList) { - return ( - headersList.get('x-forwarded-for')?.split(',')[0]?.trim() || - headersList.get('x-real-ip') || - 'unknown' - ); + if (process.env.ZEN_TRUST_PROXY === 'true') { + const forwarded = headersList.get('x-forwarded-for')?.split(',')[0]?.trim(); + if (forwarded && isValidIp(forwarded)) return forwarded; + const realIp = headersList.get('x-real-ip')?.trim(); + if (realIp && isValidIp(realIp)) return realIp; + } + // Safe fallback — all requests share the 'unknown' bucket. + // Configure ZEN_TRUST_PROXY=true behind a verified reverse proxy. + return 'unknown'; } /** - * Extract the best-effort client IP from a Next.js Request object (API routes). + * Extract the client IP from a Next.js Request object (API routes). + * See getIpFromHeaders for the full trust-proxy rationale. * @param {Request} request * @returns {string} */ export function getIpFromRequest(request) { - return ( - request.headers.get('x-forwarded-for')?.split(',')[0]?.trim() || - request.headers.get('x-real-ip') || - 'unknown' - ); + if (process.env.ZEN_TRUST_PROXY === 'true') { + const forwarded = request.headers.get('x-forwarded-for')?.split(',')[0]?.trim(); + if (forwarded && isValidIp(forwarded)) return forwarded; + const realIp = request.headers.get('x-real-ip')?.trim(); + if (realIp && isValidIp(realIp)) return realIp; + } + return 'unknown'; } /**