fix(security): stop reflecting submitted passwords in validation errors

Codex review round 2. The 400 I added in the previous commit returned
errors.array() verbatim, and express-validator puts the submitted `value` in
each error -- so rejecting an oversized password echoed that password back, and
re-allocated up to the 50mb body limit on an unauthenticated endpoint, partly
undoing the denial-of-service fix this branch exists for.

The same call appeared at seven sites in this file, five of which validate a
password field: /admin/login, /gallery/verify, /gallery/:slug/client-login,
/admin/change-password and /password-strength. Every failed login was returning
the attempted password in its response body, where it reaches proxy logs, error
monitoring and browser tooling. Fixed at all seven rather than only the one the
review pointed at.

Only `value` is dropped. `msg`, `path` and the rest are kept, because the two
shapes express-validator produces are both consumed in the frontend -- AcceptInvite
reads {field, message} from routeHelpers.validateRequest, EventDetails reads
{msg, path} from raw errors.array() -- and switching auth.js to the helper's
shape would have broken the latter for a reason unrelated to security.

1 more test. Backend suite: 2744 passed.

(cherry picked from commit 903e471753)
This commit is contained in:
Paul Nothaft
2026-09-03 12:12:12 +02:00
parent 706d402c1e
commit 6481708def
2 changed files with 32 additions and 7 deletions
@@ -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);