fix: harden API security against info leakage and MIME sniffing
- Sanitize route handler errors: only surface known auth messages
('Unauthorized', 'Admin access required'); log all other exceptions
server-side and return a generic 'Internal Server Error' to clients
- Derive profile picture content-type from validated file extension
instead of attacker-controlled file.type to prevent MIME spoofing
- Always emit explicit Content-Disposition headers on file responses;
serve known image types as 'inline', force download for all others
to prevent in-browser rendering of potentially dangerous content
- Add X-Content-Type-Options: nosniff and X-Frame-Options: DENY to
file response headers
This commit is contained in:
@@ -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'
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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({
|
||||
|
||||
@@ -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
|
||||
// <img> 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 });
|
||||
}
|
||||
|
||||
|
||||
+10
-2
@@ -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'
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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)/,
|
||||
/<script[\s>]/,
|
||||
/<svg[\s>]/,
|
||||
];
|
||||
|
||||
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');
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
@@ -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<string, { count: number, windowStart: number, windowMs: number, blockedUntil: number|null }>} */
|
||||
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';
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
Reference in New Issue
Block a user