diff --git a/backend/__tests__/routes/passwordStrengthDos.test.js b/backend/__tests__/routes/passwordStrengthDos.test.js index 3759a0f0..4ff60a42 100644 --- a/backend/__tests__/routes/passwordStrengthDos.test.js +++ b/backend/__tests__/routes/passwordStrengthDos.test.js @@ -40,6 +40,20 @@ describe('password validation length cap (zxcvbn DoS)', () => { expect(result.valid).toBe(true); }); + it('does not spin when a caller asks for a length the cap forbids', async () => { + // Codex review. generateSecurePassword retried by recursing on any invalid + // candidate, so the new cap made every candidate invalid for length > 128 + // and turned the call into unbounded recursion. It now refuses up front, + // and the retry loop is bounded. + const { generateSecurePassword } = require('../../src/utils/passwordValidation'); + + expect(generateSecurePassword({ length: 16 })).toHaveLength(16); + expect(generateSecurePassword({ length: MAX_PASSWORD_LENGTH })) + .toHaveLength(MAX_PASSWORD_LENGTH); + expect(() => generateSecurePassword({ length: MAX_PASSWORD_LENGTH + 1 })) + .toThrow(/at most 128/); + }); + it('applies the cap through the context wrapper too', async () => { const { validatePasswordInContext } = require('../../src/utils/passwordValidation'); const huge = 'aA1!'.repeat(MAX_PASSWORD_LENGTH); diff --git a/backend/src/routes/auth.js b/backend/src/routes/auth.js index 3d341edb..98c4f3af 100644 --- a/backend/src/routes/auth.js +++ b/backend/src/routes/auth.js @@ -966,6 +966,14 @@ router.post('/password-strength', [ body('context').isIn(['admin', 'gallery']).optional() ], async (req, res) => { try { + // The validators above only RECORD errors; without this the oversized body + // reached zxcvbn anyway and the endpoint answered 200, so the edge cap was + // decorative. The cap in validatePassword() is still the real control. + const errors = validationResult(req); + if (!errors.isEmpty()) { + return res.status(400).json({ errors: errors.array() }); + } + const { password, context = 'gallery' } = req.body; // Get user data if available (for context-aware validation) @@ -975,7 +983,9 @@ router.post('/password-strength', [ userData.email = req.admin.email; } - const validation = validatePasswordInContext(password, context, userData); + // validatePasswordInContext is async; unawaited this resolved to a Promise + // and every field below came back undefined. + const validation = await validatePasswordInContext(password, context, userData); res.json({ valid: validation.valid, diff --git a/backend/src/utils/passwordValidation.js b/backend/src/utils/passwordValidation.js index 666e8943..b7a1cbc2 100644 --- a/backend/src/utils/passwordValidation.js +++ b/backend/src/utils/passwordValidation.js @@ -349,24 +349,35 @@ function generateSecurePassword(options = {}) { if (charset.length === 0) { throw new Error('At least one character type must be included'); } - - // Generate password + + // A requested length the validator will always reject makes the retry below + // unwinnable, so say so instead of spinning. MAX_PASSWORD_LENGTH is the cap + // validatePassword() applies; anything above it fails every candidate. + if (config.length > MAX_PASSWORD_LENGTH) { + throw new Error(`length must be at most ${MAX_PASSWORD_LENGTH}`); + } + const crypto = require('crypto'); - let password = ''; - - for (let i = 0; i < config.length; i++) { - const randomIndex = crypto.randomInt(charset.length); - password += charset[randomIndex]; + + // Bounded retry rather than unbounded recursion. Every candidate failing is + // possible for reasons other than bad luck -- a charset that cannot satisfy + // the configured policy (numbers excluded while requireNumbers is on, say) + // -- and the previous `return generateSecurePassword(options)` turned that + // into a stack overflow rather than an error anyone could act on. + const MAX_ATTEMPTS = 100; + for (let attempt = 0; attempt < MAX_ATTEMPTS; attempt += 1) { + let password = ''; + for (let i = 0; i < config.length; i++) { + const randomIndex = crypto.randomInt(charset.length); + password += charset[randomIndex]; + } + if (validatePassword(password).valid) return password; } - - // Ensure password meets requirements - const validation = validatePassword(password); - if (!validation.valid) { - // Recursively generate until we get a valid password - return generateSecurePassword(options); - } - - return password; + + throw new Error( + 'Could not generate a password satisfying the configured policy — ' + + 'check that the selected character types can meet it', + ); } /**