diff --git a/backend/__tests__/routes/passwordStrengthDos.test.js b/backend/__tests__/routes/passwordStrengthDos.test.js index 4ff60a42..7e60320d 100644 --- a/backend/__tests__/routes/passwordStrengthDos.test.js +++ b/backend/__tests__/routes/passwordStrengthDos.test.js @@ -54,6 +54,21 @@ describe('password validation length cap (zxcvbn DoS)', () => { .toThrow(/at most 128/); }); + it('does not echo the rejected password back in the error body', async () => { + // Codex review round 2. express-validator's errors.array() carries the + // submitted `value`, so the 400 for an oversized password returned the + // password itself -- reflecting a credential, and re-allocating up to the + // 50mb body limit on an unauthenticated endpoint, which partly undid the + // DoS fix this branch exists for. + const src = require('fs').readFileSync( + require('path').join(__dirname, '../../src/routes/auth.js'), 'utf8'); + + // No route may hand errors.array() straight to the response. + expect(src).not.toMatch(/errors:\s*errors\.array\(\)/); + // ...and the helper that replaces it must drop `value`. + expect(src).toMatch(/safeValidationErrors\s*=\s*\(errors\)\s*=>\s*errors\.array\(\)\.map\(\(\{ value, \.\.\.rest \}\)/); + }); + 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 98c4f3af..f028e862 100644 --- a/backend/src/routes/auth.js +++ b/backend/src/routes/auth.js @@ -2,6 +2,16 @@ const express = require('express'); const bcrypt = require('bcrypt'); const jwt = require('jsonwebtoken'); const { body, validationResult } = require('express-validator'); + +/** + * express-validator's errors.array() carries `value` -- the submitted input -- + * so returning it verbatim reflects the caller's password back in the 400 body. + * Five routes in this file validate a password field, and the strength endpoint + * is unauthenticated behind a 50mb JSON limit, which also made the rejection + * itself an allocation amplifier. Everything except `value` is kept, so the + * response shape both frontend consumers rely on (`msg`, `path`) is unchanged. + */ +const safeValidationErrors = (errors) => errors.array().map(({ value, ...rest }) => rest); const { db, logActivity } = require('../database/db'); const { formatBoolean } = require('../utils/dbCompat'); const { verifyRecaptcha } = require('../services/recaptcha'); @@ -122,7 +132,7 @@ router.post('/admin/login', [ try { const errors = validationResult(req); if (!errors.isEmpty()) { - return res.status(400).json({ errors: errors.array() }); + return res.status(400).json({ errors: safeValidationErrors(errors) }); } const { username, password, recaptchaToken } = req.body; @@ -229,7 +239,7 @@ router.post('/admin/login/mfa', [ try { const errors = validationResult(req); if (!errors.isEmpty()) { - return res.status(400).json({ errors: errors.array() }); + return res.status(400).json({ errors: safeValidationErrors(errors) }); } const { mfaToken, code } = req.body; @@ -398,7 +408,7 @@ router.post('/gallery/verify', [ try { const errors = validationResult(req); if (!errors.isEmpty()) { - return res.status(400).json({ errors: errors.array() }); + return res.status(400).json({ errors: safeValidationErrors(errors) }); } const { slug, password, recaptchaToken } = req.body; @@ -521,7 +531,7 @@ router.post('/gallery/:slug/client-login', [ try { const errors = validationResult(req); if (!errors.isEmpty()) { - return res.status(400).json({ errors: errors.array() }); + return res.status(400).json({ errors: safeValidationErrors(errors) }); } const { slug } = req.params; @@ -597,7 +607,7 @@ router.post('/gallery/share-login', [ try { const errors = validationResult(req); if (!errors.isEmpty()) { - return res.status(400).json({ errors: errors.array() }); + return res.status(400).json({ errors: safeValidationErrors(errors) }); } const { slug, token } = req.body; @@ -882,7 +892,7 @@ router.post('/admin/change-password', [ try { const errors = validationResult(req); if (!errors.isEmpty()) { - return res.status(400).json({ errors: errors.array() }); + return res.status(400).json({ errors: safeValidationErrors(errors) }); } const { currentPassword, newPassword } = req.body; @@ -971,7 +981,7 @@ router.post('/password-strength', [ // 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() }); + return res.status(400).json({ errors: safeValidationErrors(errors) }); } const { password, context = 'gallery' } = req.body;