fix(security): stop reflecting submitted values in validation errors everywhere, cap credential lengths, close the login timing oracle

safeValidationErrors moves to utils/routeHelpers and replaces every
res.status(400).json({ errors: errors.array() }) in the routes, so no 400
body carries the submitted value any more (setup, customer auth and
customer change-password were still echoing rejected passwords).

Admin login, gallery verify, customer login/register/reset, customer
change-password and setup now cap username/slug at 255 and passwords at
MAX_PASSWORD_LENGTH at the validator, so an oversized value never reaches
the lockout lookup, bcrypt or the failed-attempt log.

Admin and customer login run one bcrypt compare on every path; the unknown
account branch used to return in microseconds against ~100ms for a wrong
password, which enumerated usernames despite the generic message.

(cherry picked from commit 40a8a9882a)
This commit is contained in:
Paul Nothaft
2026-09-03 12:14:42 +02:00
parent b1369068ae
commit 406c638451
23 changed files with 117 additions and 74 deletions
@@ -65,8 +65,10 @@ describe('password validation length cap (zxcvbn DoS)', () => {
// No route may hand errors.array() straight to the response. // No route may hand errors.array() straight to the response.
expect(src).not.toMatch(/errors:\s*errors\.array\(\)/); expect(src).not.toMatch(/errors:\s*errors\.array\(\)/);
// ...and the helper that replaces it must drop `value`. // ...and the shared helper that replaces it must drop `value`.
expect(src).toMatch(/safeValidationErrors\s*=\s*\(errors\)\s*=>\s*errors\.array\(\)\.map\(\(\{ value, \.\.\.rest \}\)/); const helper = require('fs').readFileSync(
require('path').join(__dirname, '../../src/utils/routeHelpers.js'), 'utf8');
expect(helper).toMatch(/safeValidationErrors\s*=\s*\(errors\)\s*=>\s*errors\.array\(\)\.map\(\(\{ value, \.\.\.rest \}\)/);
}); });
it('applies the cap through the context wrapper too', async () => { it('applies the cap through the context wrapper too', async () => {
+2 -1
View File
@@ -7,6 +7,7 @@
const express = require('express'); const express = require('express');
const { body, validationResult } = require('express-validator'); const { body, validationResult } = require('express-validator');
const { safeValidationErrors } = require('../utils/routeHelpers');
const { db, logActivity } = require('../database/db'); const { db, logActivity } = require('../database/db');
const { adminAuth } = require('./../middleware/auth'); const { adminAuth } = require('./../middleware/auth');
const { requirePermission } = require('./../middleware/permissions'); const { requirePermission } = require('./../middleware/permissions');
@@ -65,7 +66,7 @@ router.post(
try { try {
const errors = validationResult(req); const errors = validationResult(req);
if (!errors.isEmpty()) { if (!errors.isEmpty()) {
return res.status(400).json({ errors: errors.array() }); return res.status(400).json({ errors: safeValidationErrors(errors) });
} }
const { name, scopes, expires_at } = req.body; const { name, scopes, expires_at } = req.body;
const { plaintext, hashed, preview } = generateApiToken(); const { plaintext, hashed, preview } = generateApiToken();
+2 -1
View File
@@ -3,6 +3,7 @@ const path = require('path');
const fs = require('fs').promises; const fs = require('fs').promises;
const multer = require('multer'); const multer = require('multer');
const { body, validationResult } = require('express-validator'); const { body, validationResult } = require('express-validator');
const { safeValidationErrors } = require('../utils/routeHelpers');
const { db, logActivity } = require('../database/db'); const { db, logActivity } = require('../database/db');
const { adminAuth } = require('../middleware/auth'); const { adminAuth } = require('../middleware/auth');
const { requirePermission } = require('../middleware/permissions'); const { requirePermission } = require('../middleware/permissions');
@@ -80,7 +81,7 @@ router.put('/pages/:slug', adminAuth, requirePermission('cms.edit'), [
try { try {
const errors = validationResult(req); const errors = validationResult(req);
if (!errors.isEmpty()) { if (!errors.isEmpty()) {
return res.status(400).json({ errors: errors.array() }); return res.status(400).json({ errors: safeValidationErrors(errors) });
} }
const { slug } = req.params; const { slug } = req.params;
+4 -3
View File
@@ -1,5 +1,6 @@
const express = require('express'); const express = require('express');
const { body, validationResult } = require('express-validator'); const { body, validationResult } = require('express-validator');
const { safeValidationErrors } = require('../utils/routeHelpers');
const { db, logActivity } = require('../database/db'); const { db, logActivity } = require('../database/db');
const { formatBoolean } = require('../utils/dbCompat'); const { formatBoolean } = require('../utils/dbCompat');
const { adminAuth } = require('../middleware/auth'); const { adminAuth } = require('../middleware/auth');
@@ -51,7 +52,7 @@ router.post('/', adminAuth, requirePermission('settings.edit'), [
try { try {
const errors = validationResult(req); const errors = validationResult(req);
if (!errors.isEmpty()) { if (!errors.isEmpty()) {
return res.status(400).json({ errors: errors.array() }); return res.status(400).json({ errors: safeValidationErrors(errors) });
} }
const { name, slug, is_global = true, event_id = null } = req.body; const { name, slug, is_global = true, event_id = null } = req.body;
@@ -119,7 +120,7 @@ router.put('/:id', adminAuth, requirePermission('settings.edit'), [
try { try {
const errors = validationResult(req); const errors = validationResult(req);
if (!errors.isEmpty()) { if (!errors.isEmpty()) {
return res.status(400).json({ errors: errors.array() }); return res.status(400).json({ errors: safeValidationErrors(errors) });
} }
const { id } = req.params; const { id } = req.params;
@@ -191,7 +192,7 @@ router.put('/:id/hero', adminAuth, requirePermission('settings.edit'), [
try { try {
const errors = validationResult(req); const errors = validationResult(req);
if (!errors.isEmpty()) { if (!errors.isEmpty()) {
return res.status(400).json({ errors: errors.array() }); return res.status(400).json({ errors: safeValidationErrors(errors) });
} }
const { id } = req.params; const { id } = req.params;
+4 -3
View File
@@ -6,6 +6,7 @@
const express = require('express'); const express = require('express');
const router = express.Router(); const router = express.Router();
const { body, param, validationResult } = require('express-validator'); const { body, param, validationResult } = require('express-validator');
const { safeValidationErrors } = require('../utils/routeHelpers');
const { db, withRetry } = require('../database/db'); const { db, withRetry } = require('../database/db');
const { adminAuth } = require('../middleware/auth'); const { adminAuth } = require('../middleware/auth');
const { requirePermission } = require('../middleware/permissions'); const { requirePermission } = require('../middleware/permissions');
@@ -58,7 +59,7 @@ router.get('/:slotNumber', adminAuth, requirePermission('branding.view'), [
try { try {
const errors = validationResult(req); const errors = validationResult(req);
if (!errors.isEmpty()) { if (!errors.isEmpty()) {
return res.status(400).json({ errors: errors.array() }); return res.status(400).json({ errors: safeValidationErrors(errors) });
} }
const { slotNumber } = req.params; const { slotNumber } = req.params;
@@ -92,7 +93,7 @@ router.put('/:slotNumber', adminAuth, requirePermission('branding.edit'), [
try { try {
const errors = validationResult(req); const errors = validationResult(req);
if (!errors.isEmpty()) { if (!errors.isEmpty()) {
return res.status(400).json({ errors: errors.array() }); return res.status(400).json({ errors: safeValidationErrors(errors) });
} }
const { slotNumber } = req.params; const { slotNumber } = req.params;
@@ -166,7 +167,7 @@ router.post('/:slotNumber/reset', adminAuth, requirePermission('branding.edit'),
try { try {
const errors = validationResult(req); const errors = validationResult(req);
if (!errors.isEmpty()) { if (!errors.isEmpty()) {
return res.status(400).json({ errors: errors.array() }); return res.status(400).json({ errors: safeValidationErrors(errors) });
} }
await withRetry(() => await withRetry(() =>
+4 -4
View File
@@ -9,7 +9,7 @@ const { requirePermission } = require('../middleware/permissions');
const { requireFeatureFlag } = require('../middleware/requireFeatureFlag'); const { requireFeatureFlag } = require('../middleware/requireFeatureFlag');
const messagingGate = requireFeatureFlag('messaging'); const messagingGate = requireFeatureFlag('messaging');
const { wrapEmailHtml, processEmailQueue } = require('../services/emailProcessor'); const { wrapEmailHtml, processEmailQueue } = require('../services/emailProcessor');
const { errorResponse } = require('../utils/routeHelpers'); const { errorResponse, safeValidationErrors } = require('../utils/routeHelpers');
const logger = require('../utils/logger'); const logger = require('../utils/logger');
const router = express.Router(); const router = express.Router();
@@ -52,7 +52,7 @@ router.post('/config', [
try { try {
const errors = validationResult(req); const errors = validationResult(req);
if (!errors.isEmpty()) { if (!errors.isEmpty()) {
return res.status(400).json({ errors: errors.array() }); return res.status(400).json({ errors: safeValidationErrors(errors) });
} }
const { const {
@@ -152,7 +152,7 @@ router.post('/incoming-config', [
], async (req, res) => { ], async (req, res) => {
try { try {
const errors = validationResult(req); const errors = validationResult(req);
if (!errors.isEmpty()) return res.status(400).json({ errors: errors.array() }); if (!errors.isEmpty()) return res.status(400).json({ errors: safeValidationErrors(errors) });
const { imap_host, imap_port, imap_secure, imap_user, imap_pass, imap_folder } = req.body; const { imap_host, imap_port, imap_secure, imap_user, imap_pass, imap_folder } = req.body;
const { isHostAllowed } = require('../utils/networkValidation'); const { isHostAllowed } = require('../utils/networkValidation');
if (!(await isHostAllowed(imap_host))) { if (!(await isHostAllowed(imap_host))) {
@@ -635,7 +635,7 @@ router.get('/queue', adminAuth, requirePermission('email.view'), [
try { try {
const errors = validationResult(req); const errors = validationResult(req);
if (!errors.isEmpty()) { if (!errors.isEmpty()) {
return res.status(400).json({ errors: errors.array() }); return res.status(400).json({ errors: safeValidationErrors(errors) });
} }
const page = req.query.page ? parseInt(req.query.page, 10) : 1; const page = req.query.page ? parseInt(req.query.page, 10) : 1;
+3 -2
View File
@@ -5,6 +5,7 @@
const express = require('express'); const express = require('express');
const { body, validationResult } = require('express-validator'); const { body, validationResult } = require('express-validator');
const { safeValidationErrors } = require('../utils/routeHelpers');
const { adminAuth } = require('../middleware/auth'); const { adminAuth } = require('../middleware/auth');
const { requirePermission } = require('../middleware/permissions'); const { requirePermission } = require('../middleware/permissions');
const { requireEventOwnership } = require('../middleware/ownership'); const { requireEventOwnership } = require('../middleware/ownership');
@@ -29,7 +30,7 @@ router.post('/:eventId/rename', adminAuth, requirePermission('events.edit'), req
try { try {
const errors = validationResult(req); const errors = validationResult(req);
if (!errors.isEmpty()) { if (!errors.isEmpty()) {
return res.status(400).json({ success: false, errors: errors.array() }); return res.status(400).json({ success: false, errors: safeValidationErrors(errors) });
} }
const { eventId } = req.params; const { eventId } = req.params;
@@ -70,7 +71,7 @@ router.post('/:eventId/validate-rename', adminAuth, requirePermission('events.ed
try { try {
const errors = validationResult(req); const errors = validationResult(req);
if (!errors.isEmpty()) { if (!errors.isEmpty()) {
return res.status(400).json({ valid: false, errors: errors.array() }); return res.status(400).json({ valid: false, errors: safeValidationErrors(errors) });
} }
const { eventId } = req.params; const { eventId } = req.params;
+6 -5
View File
@@ -7,6 +7,7 @@
const express = require('express'); const express = require('express');
const { body, param, validationResult } = require('express-validator'); const { body, param, validationResult } = require('express-validator');
const { safeValidationErrors } = require('../utils/routeHelpers');
const { logActivity } = require('../database/db'); const { logActivity } = require('../database/db');
const { adminAuth } = require('../middleware/auth'); const { adminAuth } = require('../middleware/auth');
const { requirePermission } = require('../middleware/permissions'); const { requirePermission } = require('../middleware/permissions');
@@ -57,7 +58,7 @@ router.get('/:id', adminAuth, requirePermission('settings.view'), [
try { try {
const errors = validationResult(req); const errors = validationResult(req);
if (!errors.isEmpty()) { if (!errors.isEmpty()) {
return res.status(400).json({ errors: errors.array() }); return res.status(400).json({ errors: safeValidationErrors(errors) });
} }
const { id } = req.params; const { id } = req.params;
@@ -94,7 +95,7 @@ router.post('/', adminAuth, requirePermission('settings.edit'), [
try { try {
const errors = validationResult(req); const errors = validationResult(req);
if (!errors.isEmpty()) { if (!errors.isEmpty()) {
return res.status(400).json({ errors: errors.array() }); return res.status(400).json({ errors: safeValidationErrors(errors) });
} }
const { const {
@@ -156,7 +157,7 @@ router.put('/:id', adminAuth, requirePermission('settings.edit'), [
try { try {
const errors = validationResult(req); const errors = validationResult(req);
if (!errors.isEmpty()) { if (!errors.isEmpty()) {
return res.status(400).json({ errors: errors.array() }); return res.status(400).json({ errors: safeValidationErrors(errors) });
} }
const { id } = req.params; const { id } = req.params;
@@ -196,7 +197,7 @@ router.delete('/:id', adminAuth, requirePermission('settings.edit'), [
try { try {
const errors = validationResult(req); const errors = validationResult(req);
if (!errors.isEmpty()) { if (!errors.isEmpty()) {
return res.status(400).json({ errors: errors.array() }); return res.status(400).json({ errors: safeValidationErrors(errors) });
} }
const { id } = req.params; const { id } = req.params;
@@ -235,7 +236,7 @@ router.post('/reorder', adminAuth, requirePermission('settings.edit'), [
try { try {
const errors = validationResult(req); const errors = validationResult(req);
if (!errors.isEmpty()) { if (!errors.isEmpty()) {
return res.status(400).json({ errors: errors.array() }); return res.status(400).json({ errors: safeValidationErrors(errors) });
} }
const { orderedIds } = req.body; const { orderedIds } = req.body;
@@ -9,7 +9,7 @@ const { adminAuth } = require('../../middleware/auth');
const { requirePermission } = require('../../middleware/permissions'); const { requirePermission } = require('../../middleware/permissions');
const { archiveEvent } = require('../../services/archiveService'); const { archiveEvent } = require('../../services/archiveService');
const logger = require('../../utils/logger'); const logger = require('../../utils/logger');
const { errorResponse } = require('../../utils/routeHelpers'); const { errorResponse, safeValidationErrors } = require('../../utils/routeHelpers');
const { requireEventOwnership, filterOwnedEventIds } = require('../../middleware/ownership'); const { requireEventOwnership, filterOwnedEventIds } = require('../../middleware/ownership');
const { deleteEventCascade } = require('./helpers'); const { deleteEventCascade } = require('./helpers');
@@ -70,7 +70,7 @@ module.exports = (router) => {
try { try {
const errors = validationResult(req); const errors = validationResult(req);
if (!errors.isEmpty()) { if (!errors.isEmpty()) {
return res.status(400).json({ errors: errors.array() }); return res.status(400).json({ errors: safeValidationErrors(errors) });
} }
const { eventIds } = req.body; const { eventIds } = req.body;
@@ -160,7 +160,7 @@ module.exports = (router) => {
try { try {
const errors = validationResult(req); const errors = validationResult(req);
if (!errors.isEmpty()) { if (!errors.isEmpty()) {
return res.status(400).json({ errors: errors.array() }); return res.status(400).json({ errors: safeValidationErrors(errors) });
} }
const { eventIds } = req.body; const { eventIds } = req.body;
+6 -6
View File
@@ -17,7 +17,7 @@ const { escapeLikePattern } = require('../../utils/sqlSecurity');
const { validatePasswordInContext, getBcryptRounds } = require('../../utils/passwordValidation'); const { validatePasswordInContext, getBcryptRounds } = require('../../utils/passwordValidation');
const logger = require('../../utils/logger'); const logger = require('../../utils/logger');
const { sanitizeForLog, sanitizeValidationErrors } = require('../../utils/sanitizeForLog'); const { sanitizeForLog, sanitizeValidationErrors } = require('../../utils/sanitizeForLog');
const { errorResponse } = require('../../utils/routeHelpers'); const { errorResponse, safeValidationErrors } = require('../../utils/routeHelpers');
const { buildShareLinkVariants } = require('../../services/shareLinkService'); const { buildShareLinkVariants } = require('../../services/shareLinkService');
const { parseBooleanInput } = require('../../utils/parsers'); const { parseBooleanInput } = require('../../utils/parsers');
const eventTypeService = require('../../services/eventTypeService'); const eventTypeService = require('../../services/eventTypeService');
@@ -157,7 +157,7 @@ module.exports = (router) => {
// errors.array() embeds the SUBMITTED value per field — including a // errors.array() embeds the SUBMITTED value per field — including a
// rejected plaintext password (GHSA-r794). // rejected plaintext password (GHSA-r794).
logger.error('Validation errors:', sanitizeValidationErrors(errors.array())); logger.error('Validation errors:', sanitizeValidationErrors(errors.array()));
return res.status(400).json({ errors: errors.array() }); return res.status(400).json({ errors: safeValidationErrors(errors) });
} }
// Get field requirements from settings // Get field requirements from settings
@@ -856,7 +856,7 @@ module.exports = (router) => {
try { try {
const errors = validationResult(req); const errors = validationResult(req);
if (!errors.isEmpty()) { if (!errors.isEmpty()) {
return res.status(400).json({ errors: errors.array() }); return res.status(400).json({ errors: safeValidationErrors(errors) });
} }
const { id } = req.params; const { id } = req.params;
@@ -1017,7 +1017,7 @@ module.exports = (router) => {
try { try {
const errors = validationResult(req); const errors = validationResult(req);
if (!errors.isEmpty()) { if (!errors.isEmpty()) {
return res.status(400).json({ errors: errors.array() }); return res.status(400).json({ errors: safeValidationErrors(errors) });
} }
const { id } = req.params; const { id } = req.params;
@@ -1295,7 +1295,7 @@ module.exports = (router) => {
if (!errors.isEmpty()) { if (!errors.isEmpty()) {
// Redact credentials — an invalid update still logs the whole body (GHSA-pgmp). // Redact credentials — an invalid update still logs the whole body (GHSA-pgmp).
logger.debug('Update event validation errors', { errors: sanitizeValidationErrors(errors.array()), body: sanitizeForLog(req.body) }); logger.debug('Update event validation errors', { errors: sanitizeValidationErrors(errors.array()), body: sanitizeForLog(req.body) });
return res.status(400).json({ errors: errors.array() }); return res.status(400).json({ errors: safeValidationErrors(errors) });
} }
const { id } = req.params; const { id } = req.params;
@@ -1695,7 +1695,7 @@ module.exports = (router) => {
try { try {
const errors = validationResult(req); const errors = validationResult(req);
if (!errors.isEmpty()) { if (!errors.isEmpty()) {
return res.status(400).json({ errors: errors.array() }); return res.status(400).json({ errors: safeValidationErrors(errors) });
} }
const { id } = req.params; const { id } = req.params;
+2 -2
View File
@@ -8,7 +8,7 @@ const { formatBoolean } = require('../../utils/dbCompat');
const { adminAuth } = require('../../middleware/auth'); const { adminAuth } = require('../../middleware/auth');
const { requirePermission } = require('../../middleware/permissions'); const { requirePermission } = require('../../middleware/permissions');
const crypto = require('crypto'); const crypto = require('crypto');
const { errorResponse } = require('../../utils/routeHelpers'); const { errorResponse, safeValidationErrors } = require('../../utils/routeHelpers');
const { parseBooleanInput } = require('../../utils/parsers'); const { parseBooleanInput } = require('../../utils/parsers');
const { requireEventOwnership } = require('../../middleware/ownership'); const { requireEventOwnership } = require('../../middleware/ownership');
const { requireFeatureFlag } = require('../../middleware/requireFeatureFlag'); const { requireFeatureFlag } = require('../../middleware/requireFeatureFlag');
@@ -110,7 +110,7 @@ module.exports = (router) => {
try { try {
const errors = validationResult(req); const errors = validationResult(req);
if (!errors.isEmpty()) { if (!errors.isEmpty()) {
return res.status(400).json({ error: 'Invalid slideshow settings', details: errors.array() }); return res.status(400).json({ error: 'Invalid slideshow settings', details: safeValidationErrors(errors) });
} }
const event = await loadOwnedEvent(req); const event = await loadOwnedEvent(req);
+3 -3
View File
@@ -11,7 +11,7 @@ const { adminAuth } = require('../middleware/auth');
const { requirePermission } = require('../middleware/permissions'); const { requirePermission } = require('../middleware/permissions');
const { requireEventOwnership } = require('../middleware/ownership'); const { requireEventOwnership } = require('../middleware/ownership');
const { PhotoFilterBuilder } = require('../utils/photoFilterBuilder'); const { PhotoFilterBuilder } = require('../utils/photoFilterBuilder');
const { getPagination } = require('../utils/routeHelpers'); const { getPagination, safeValidationErrors } = require('../utils/routeHelpers');
const { PhotoExportService } = require('../services/photoExportService'); const { PhotoExportService } = require('../services/photoExportService');
const logger = require('../utils/logger'); const logger = require('../utils/logger');
@@ -39,7 +39,7 @@ router.get('/:eventId/filtered', adminAuth, requirePermission('photos.view'), re
try { try {
const errors = validationResult(req); const errors = validationResult(req);
if (!errors.isEmpty()) { if (!errors.isEmpty()) {
return res.status(400).json({ errors: errors.array() }); return res.status(400).json({ errors: safeValidationErrors(errors) });
} }
const eventId = parseInt(req.params.eventId); const eventId = parseInt(req.params.eventId);
@@ -166,7 +166,7 @@ router.post('/:eventId/export', adminAuth, requirePermission('photos.download'),
try { try {
const errors = validationResult(req); const errors = validationResult(req);
if (!errors.isEmpty()) { if (!errors.isEmpty()) {
return res.status(400).json({ errors: errors.array() }); return res.status(400).json({ errors: safeValidationErrors(errors) });
} }
const eventId = parseInt(req.params.eventId); const eventId = parseInt(req.params.eventId);
+4 -4
View File
@@ -5,7 +5,7 @@ const { adminAuth } = require('../middleware/auth');
const { requirePermission } = require('../middleware/permissions'); const { requirePermission } = require('../middleware/permissions');
const { body, query, validationResult } = require('express-validator'); const { body, query, validationResult } = require('express-validator');
const logger = require('../utils/logger'); const logger = require('../utils/logger');
const { getPagination } = require('../utils/routeHelpers'); const { getPagination, safeValidationErrors } = require('../utils/routeHelpers');
const { db } = require('../database/db'); const { db } = require('../database/db');
const path = require('path'); const path = require('path');
const fs = require('fs').promises; const fs = require('fs').promises;
@@ -86,7 +86,7 @@ router.post('/validate', requirePermission('backup.restore'), [
if (!errors.isEmpty()) { if (!errors.isEmpty()) {
return res.status(400).json({ return res.status(400).json({
success: false, success: false,
errors: errors.array() errors: safeValidationErrors(errors)
}); });
} }
@@ -158,7 +158,7 @@ router.post('/start', requirePermission('backup.restore'), [
if (!errors.isEmpty()) { if (!errors.isEmpty()) {
return res.status(400).json({ return res.status(400).json({
success: false, success: false,
errors: errors.array() errors: safeValidationErrors(errors)
}); });
} }
@@ -690,7 +690,7 @@ router.put('/settings', requirePermission('backup.restore'), [
if (!errors.isEmpty()) { if (!errors.isEmpty()) {
return res.status(400).json({ return res.status(400).json({
success: false, success: false,
errors: errors.array() errors: safeValidationErrors(errors)
}); });
} }
+2 -2
View File
@@ -23,7 +23,7 @@ const { sanitizeCss } = require('../utils/cssSanitizer');
const { upsertAppSetting } = require('../utils/appSettings'); const { upsertAppSetting } = require('../utils/appSettings');
const { clearShareLinkSettingsCache } = require('../services/shareLinkService'); const { clearShareLinkSettingsCache } = require('../services/shareLinkService');
const { resetSecurityConfigCache } = require('../utils/authSecurity'); const { resetSecurityConfigCache } = require('../utils/authSecurity');
const { errorResponse } = require('../utils/routeHelpers'); const { errorResponse, safeValidationErrors } = require('../utils/routeHelpers');
const logger = require('../utils/logger'); const logger = require('../utils/logger');
const { measureLocalStorageUsage } = require('../services/localStorageUsage'); const { measureLocalStorageUsage } = require('../services/localStorageUsage');
const router = express.Router(); const router = express.Router();
@@ -1477,7 +1477,7 @@ router.put('/security/rate-limit', adminAuth, requirePermission('settings.edit')
try { try {
const errors = validationResult(req); const errors = validationResult(req);
if (!errors.isEmpty()) { if (!errors.isEmpty()) {
return res.status(400).json({ errors: errors.array() }); return res.status(400).json({ errors: safeValidationErrors(errors) });
} }
const { const {
+4 -3
View File
@@ -11,6 +11,7 @@
*/ */
const express = require('express'); const express = require('express');
const { body, param, validationResult } = require('express-validator'); const { body, param, validationResult } = require('express-validator');
const { safeValidationErrors } = require('../utils/routeHelpers');
const { adminAuth } = require('../middleware/auth'); const { adminAuth } = require('../middleware/auth');
const { requirePermission } = require('../middleware/permissions'); const { requirePermission } = require('../middleware/permissions');
const { requireEventOwnership } = require('../middleware/ownership'); const { requireEventOwnership } = require('../middleware/ownership');
@@ -32,7 +33,7 @@ router.get(
requireEventOwnership, requireEventOwnership,
async (req, res) => { async (req, res) => {
const errors = validationResult(req); const errors = validationResult(req);
if (!errors.isEmpty()) return res.status(400).json({ errors: errors.array() }); if (!errors.isEmpty()) return res.status(400).json({ errors: safeValidationErrors(errors) });
try { try {
const rows = await galleryShortUrlService.listForEvent(parseInt(req.params.eventId, 10)); const rows = await galleryShortUrlService.listForEvent(parseInt(req.params.eventId, 10));
res.json({ shortUrls: rows }); res.json({ shortUrls: rows });
@@ -56,7 +57,7 @@ router.post(
requireEventOwnership, requireEventOwnership,
async (req, res) => { async (req, res) => {
const errors = validationResult(req); const errors = validationResult(req);
if (!errors.isEmpty()) return res.status(400).json({ errors: errors.array() }); if (!errors.isEmpty()) return res.status(400).json({ errors: safeValidationErrors(errors) });
try { try {
const row = await galleryShortUrlService.createShortUrl({ const row = await galleryShortUrlService.createShortUrl({
eventId: parseInt(req.params.eventId, 10), eventId: parseInt(req.params.eventId, 10),
@@ -96,7 +97,7 @@ router.delete(
param('id').isInt({ min: 1 }), param('id').isInt({ min: 1 }),
async (req, res) => { async (req, res) => {
const errors = validationResult(req); const errors = validationResult(req);
if (!errors.isEmpty()) return res.status(400).json({ errors: errors.array() }); if (!errors.isEmpty()) return res.status(400).json({ errors: safeValidationErrors(errors) });
try { try {
const ok = await galleryShortUrlService.softDelete( const ok = await galleryShortUrlService.softDelete(
parseInt(req.params.id, 10), parseInt(req.params.id, 10),
+5 -4
View File
@@ -17,6 +17,7 @@
const express = require('express'); const express = require('express');
const { body, query, validationResult } = require('express-validator'); const { body, query, validationResult } = require('express-validator');
const { safeValidationErrors } = require('../utils/routeHelpers');
const { db, logActivity } = require('../database/db'); const { db, logActivity } = require('../database/db');
const { adminAuth } = require('../middleware/auth'); const { adminAuth } = require('../middleware/auth');
const { requirePermission } = require('../middleware/permissions'); const { requirePermission } = require('../middleware/permissions');
@@ -106,7 +107,7 @@ router.post(
async (req, res) => { async (req, res) => {
try { try {
const errors = validationResult(req); const errors = validationResult(req);
if (!errors.isEmpty()) return res.status(400).json({ errors: errors.array() }); if (!errors.isEmpty()) return res.status(400).json({ errors: safeValidationErrors(errors) });
const { name, url, events, active = true, filter, template } = req.body; const { name, url, events, active = true, filter, template } = req.body;
const { plaintext, preview } = webhookService.generateSecret(); const { plaintext, preview } = webhookService.generateSecret();
@@ -188,7 +189,7 @@ router.put(
async (req, res) => { async (req, res) => {
try { try {
const errors = validationResult(req); const errors = validationResult(req);
if (!errors.isEmpty()) return res.status(400).json({ errors: errors.array() }); if (!errors.isEmpty()) return res.status(400).json({ errors: safeValidationErrors(errors) });
const row = await db('webhooks').where({ id: req.params.id }).first(); const row = await db('webhooks').where({ id: req.params.id }).first();
if (!row) return res.status(404).json({ error: 'Webhook not found' }); if (!row) return res.status(404).json({ error: 'Webhook not found' });
@@ -241,7 +242,7 @@ router.post(
async (req, res) => { async (req, res) => {
try { try {
const errors = validationResult(req); const errors = validationResult(req);
if (!errors.isEmpty()) return res.status(400).json({ errors: errors.array() }); if (!errors.isEmpty()) return res.status(400).json({ errors: safeValidationErrors(errors) });
const row = await db('webhooks').where({ id: req.params.id }).first(); const row = await db('webhooks').where({ id: req.params.id }).first();
if (!row) return res.status(404).json({ error: 'Webhook not found' }); if (!row) return res.status(404).json({ error: 'Webhook not found' });
@@ -293,7 +294,7 @@ router.get(
async (req, res) => { async (req, res) => {
try { try {
const errors = validationResult(req); const errors = validationResult(req);
if (!errors.isEmpty()) return res.status(400).json({ errors: errors.array() }); if (!errors.isEmpty()) return res.status(400).json({ errors: safeValidationErrors(errors) });
const webhookId = req.params.id; const webhookId = req.params.id;
const exists = await db('webhooks').where({ id: webhookId }).first(); const exists = await db('webhooks').where({ id: webhookId }).first();
+18 -7
View File
@@ -26,6 +26,9 @@ const {
const { endSession } = require('../middleware/sessionTimeout'); const { endSession } = require('../middleware/sessionTimeout');
const { revokeToken } = require('../utils/tokenRevocation'); const { revokeToken } = require('../utils/tokenRevocation');
const { timingSafeEqualStr } = require('../utils/timingSafe'); const { timingSafeEqualStr } = require('../utils/timingSafe');
// Well-formed bcrypt hash that matches nothing; compared against when there is
// no account so the unknown-user path costs the same as a wrong password.
const DUMMY_BCRYPT_HASH = '$2b$10$abcdefghijklmnopqrstuuABCDEFGHIJKLMNOPQRSTUVWXYZ01234';
const logger = require('../utils/logger'); const logger = require('../utils/logger');
const { errorResponse } = require('../utils/routeHelpers'); const { errorResponse } = require('../utils/routeHelpers');
const { const {
@@ -91,8 +94,10 @@ async function completeAdminLogin(req, res, admin, ipAddress, userAgent, lockout
// Admin login with enhanced security // Admin login with enhanced security
router.post('/admin/login', [ router.post('/admin/login', [
body('username').notEmpty().trim(), // Length caps: an unbounded username reached the lockout lookup, bcrypt,
body('password').notEmpty() // the failed-attempt log line and login_attempts.identifier as sent.
body('username').isString().trim().notEmpty().isLength({ max: 255 }),
body('password').isString().notEmpty().isLength({ max: MAX_PASSWORD_LENGTH })
], async (req, res) => { ], async (req, res) => {
try { try {
const errors = validationResult(req); const errors = validationResult(req);
@@ -141,7 +146,13 @@ router.post('/admin/login', [
.first(); .first();
// Use generic error to prevent user enumeration // Use generic error to prevent user enumeration
if (!admin || !await bcrypt.compare(password, admin.password_hash)) { // Always run one bcrypt compare so an unknown username costs the same
// ~100ms as a wrong password; short-circuiting here was a timing oracle
// for username enumeration despite the generic message.
const passwordMatches = admin
? await bcrypt.compare(password, admin.password_hash)
: await bcrypt.compare(password, DUMMY_BCRYPT_HASH).then(() => false);
if (!passwordMatches) {
await trackFailedAttempt(username, ipAddress, userAgent); await trackFailedAttempt(username, ipAddress, userAgent);
return res.status(401).json({ error: getGenericAuthError() }); return res.status(401).json({ error: getGenericAuthError() });
} }
@@ -320,8 +331,8 @@ router.post('/logout', async (req, res) => {
// Gallery password verification with enhanced security // Gallery password verification with enhanced security
router.post('/gallery/verify', [ router.post('/gallery/verify', [
body('slug').notEmpty().trim(), body('slug').isString().trim().notEmpty().isLength({ max: 255 }),
body('password').optional().isString() body('password').optional().isString().isLength({ max: MAX_PASSWORD_LENGTH })
], async (req, res) => { ], async (req, res) => {
try { try {
const errors = validationResult(req); const errors = validationResult(req);
@@ -338,7 +349,7 @@ router.post('/gallery/verify', [
if (!event) { if (!event) {
// Perform a dummy bcrypt compare to prevent timing-based slug enumeration // Perform a dummy bcrypt compare to prevent timing-based slug enumeration
await bcrypt.compare(password || '', '$2b$10$abcdefghijklmnopqrstuuABCDEFGHIJKLMNOPQRSTUVWXYZ01234'); await bcrypt.compare(password || '', DUMMY_BCRYPT_HASH);
await trackFailedAttempt(`gallery:${slug}`, ipAddress, userAgent); await trackFailedAttempt(`gallery:${slug}`, ipAddress, userAgent);
return res.status(401).json({ error: 'Invalid gallery or password' }); return res.status(401).json({ error: 'Invalid gallery or password' });
} }
@@ -432,7 +443,7 @@ router.post('/gallery/verify', [
// Client access login (PIN-based) // Client access login (PIN-based)
router.post('/gallery/:slug/client-login', [ router.post('/gallery/:slug/client-login', [
body('password').notEmpty().isString() body('password').notEmpty().isString().isLength({ max: MAX_PASSWORD_LENGTH })
], async (req, res) => { ], async (req, res) => {
try { try {
const errors = validationResult(req); const errors = validationResult(req);
+7 -7
View File
@@ -17,9 +17,9 @@ const jwt = require('jsonwebtoken');
const { body, param, validationResult } = require('express-validator'); const { body, param, validationResult } = require('express-validator');
const { db, logActivity } = require('../database/db'); const { db, logActivity } = require('../database/db');
const { formatBoolean } = require('../utils/dbCompat'); const { formatBoolean } = require('../utils/dbCompat');
const { getBcryptRounds } = require('../utils/passwordValidation'); const { getBcryptRounds, MAX_PASSWORD_LENGTH } = require('../utils/passwordValidation');
const logger = require('../utils/logger'); const logger = require('../utils/logger');
const { errorResponse } = require('../utils/routeHelpers'); const { errorResponse, safeValidationErrors } = require('../utils/routeHelpers');
const { getClientIp } = require('../utils/requestIp'); const { getClientIp } = require('../utils/requestIp');
const { customerAuth } = require('../middleware/customerAuth'); const { customerAuth } = require('../middleware/customerAuth');
const { setGalleryAuthCookies } = require('../utils/tokenUtils'); const { setGalleryAuthCookies } = require('../utils/tokenUtils');
@@ -147,7 +147,7 @@ router.get('/events/:slug/access-token', [
try { try {
const errors = validationResult(req); const errors = validationResult(req);
if (!errors.isEmpty()) { if (!errors.isEmpty()) {
return res.status(400).json({ errors: errors.array() }); return res.status(400).json({ errors: safeValidationErrors(errors) });
} }
const { slug } = req.params; const { slug } = req.params;
@@ -284,7 +284,7 @@ router.put('/profile', [
try { try {
const errors = validationResult(req); const errors = validationResult(req);
if (!errors.isEmpty()) { if (!errors.isEmpty()) {
return res.status(400).json({ errors: errors.array() }); return res.status(400).json({ errors: safeValidationErrors(errors) });
} }
// Normalise incoming values: trim strings, drop empty → null so the DB // Normalise incoming values: trim strings, drop empty → null so the DB
@@ -329,14 +329,14 @@ router.put('/profile', [
*/ */
router.post('/profile/password', [ router.post('/profile/password', [
customerAuth, customerAuth,
body('currentPassword').isString().isLength({ min: 1 }), body('currentPassword').isString().isLength({ min: 1, max: MAX_PASSWORD_LENGTH }),
body('newPassword').isString().isLength({ min: 8 }) body('newPassword').isString().isLength({ min: 8, max: MAX_PASSWORD_LENGTH })
.withMessage('Password must be at least 8 characters'), .withMessage('Password must be at least 8 characters'),
], async (req, res) => { ], async (req, res) => {
try { try {
const errors = validationResult(req); const errors = validationResult(req);
if (!errors.isEmpty()) { if (!errors.isEmpty()) {
return res.status(400).json({ errors: errors.array() }); return res.status(400).json({ errors: safeValidationErrors(errors) });
} }
const { currentPassword, newPassword } = req.body; const { currentPassword, newPassword } = req.body;
+15 -7
View File
@@ -16,6 +16,9 @@ const express = require('express');
const bcrypt = require('bcrypt'); const bcrypt = require('bcrypt');
const jwt = require('jsonwebtoken'); const jwt = require('jsonwebtoken');
const { body, param, validationResult } = require('express-validator'); const { body, param, validationResult } = require('express-validator');
const { safeValidationErrors } = require('../utils/routeHelpers');
const { MAX_PASSWORD_LENGTH } = require('../utils/passwordValidation');
const DUMMY_BCRYPT_HASH = '$2b$10$abcdefghijklmnopqrstuuABCDEFGHIJKLMNOPQRSTUVWXYZ01234';
const { db, logActivity } = require('../database/db'); const { db, logActivity } = require('../database/db');
const { formatBoolean } = require('../utils/dbCompat'); const { formatBoolean } = require('../utils/dbCompat');
const { verifyRecaptcha } = require('../services/recaptcha'); const { verifyRecaptcha } = require('../services/recaptcha');
@@ -65,12 +68,12 @@ const TOKEN_TTL_SECONDS = 24 * 60 * 60; // mirrors admin tokens
// gallery JWTs (instant per-gallery revocation). // gallery JWTs (instant per-gallery revocation).
router.post('/login', [ router.post('/login', [
body('email').isEmail().normalizeEmail(IDENTITY_PRESERVING_NORMALIZE_EMAIL).withMessage('Valid email is required'), body('email').isEmail().normalizeEmail(IDENTITY_PRESERVING_NORMALIZE_EMAIL).withMessage('Valid email is required'),
body('password').isString().notEmpty(), body('password').isString().notEmpty().isLength({ max: MAX_PASSWORD_LENGTH }),
], async (req, res) => { ], async (req, res) => {
try { try {
const errors = validationResult(req); const errors = validationResult(req);
if (!errors.isEmpty()) { if (!errors.isEmpty()) {
return res.status(400).json({ errors: errors.array() }); return res.status(400).json({ errors: safeValidationErrors(errors) });
} }
const { email, password, recaptchaToken } = req.body; const { email, password, recaptchaToken } = req.body;
@@ -99,7 +102,12 @@ router.post('/login', [
const customer = await db('customer_accounts').where('email', email).first(); const customer = await db('customer_accounts').where('email', email).first();
// Generic error to prevent user enumeration — same wording as admin login. // Generic error to prevent user enumeration — same wording as admin login.
if (!customer || !customer.password_hash || !await bcrypt.compare(password, customer.password_hash)) { // One bcrypt compare on every path so an unknown email is not a timing
// oracle (the dummy hash matches nothing).
const passwordMatches = customer && customer.password_hash
? await bcrypt.compare(password, customer.password_hash)
: await bcrypt.compare(password, DUMMY_BCRYPT_HASH).then(() => false);
if (!passwordMatches) {
await trackFailedAttempt(lockoutKey, ipAddress, userAgent); await trackFailedAttempt(lockoutKey, ipAddress, userAgent);
return res.status(401).json({ error: getGenericAuthError() }); return res.status(401).json({ error: getGenericAuthError() });
} }
@@ -273,7 +281,7 @@ router.post('/accept-invite', [
// Length floor enforced again here for an early reject; the full // Length floor enforced again here for an early reject; the full
// policy (uppercase + digit) is checked below so we can surface a // policy (uppercase + digit) is checked below so we can surface a
// specific message rather than a generic validator error. // specific message rather than a generic validator error.
body('password').isString().isLength({ min: 8 }) body('password').isString().isLength({ min: 8, max: MAX_PASSWORD_LENGTH })
.withMessage('Password must be at least 8 characters'), .withMessage('Password must be at least 8 characters'),
// Optional structured profile from the accept-invite form. Mirrors // Optional structured profile from the accept-invite form. Mirrors
// the admin prefill shape — anything the customer types here wins // the admin prefill shape — anything the customer types here wins
@@ -296,7 +304,7 @@ router.post('/accept-invite', [
try { try {
const errors = validationResult(req); const errors = validationResult(req);
if (!errors.isEmpty()) { if (!errors.isEmpty()) {
return res.status(400).json({ errors: errors.array() }); return res.status(400).json({ errors: safeValidationErrors(errors) });
} }
const { token, name, password, profile } = req.body; const { token, name, password, profile } = req.body;
@@ -355,11 +363,11 @@ router.get('/password-reset/:token', [
*/ */
router.post('/password-reset', [ router.post('/password-reset', [
body('token').isLength({ min: 64, max: 64 }).matches(/^[a-f0-9]+$/i), body('token').isLength({ min: 64, max: 64 }).matches(/^[a-f0-9]+$/i),
body('password').isString().isLength({ min: 8 }).withMessage('Password must be at least 8 characters'), body('password').isString().isLength({ min: 8, max: MAX_PASSWORD_LENGTH }).withMessage('Password must be at least 8 characters'),
], async (req, res) => { ], async (req, res) => {
try { try {
const errors = validationResult(req); const errors = validationResult(req);
if (!errors.isEmpty()) return res.status(400).json({ errors: errors.array() }); if (!errors.isEmpty()) return res.status(400).json({ errors: safeValidationErrors(errors) });
const policyError = validateCustomerPassword(req.body.password); const policyError = validateCustomerPassword(req.body.password);
if (policyError) { if (policyError) {
return res.status(400).json({ return res.status(400).json({
+5 -3
View File
@@ -7,6 +7,8 @@
// rate-limited at the mount point in server.js (authRateLimiter). // rate-limited at the mount point in server.js (authRateLimiter).
const express = require('express'); const express = require('express');
const { body, validationResult } = require('express-validator'); const { body, validationResult } = require('express-validator');
const { safeValidationErrors } = require('../utils/routeHelpers');
const { MAX_PASSWORD_LENGTH } = require('../utils/passwordValidation');
const setupService = require('../services/setupService'); const setupService = require('../services/setupService');
const { getClientIp } = require('../utils/requestIp'); const { getClientIp } = require('../utils/requestIp');
const { setAdminAuthCookie } = require('../utils/tokenUtils'); const { setAdminAuthCookie } = require('../utils/tokenUtils');
@@ -31,7 +33,7 @@ router.post('/verify-token', [
], async (req, res) => { ], async (req, res) => {
const errors = validationResult(req); const errors = validationResult(req);
if (!errors.isEmpty()) { if (!errors.isEmpty()) {
return res.status(400).json({ errors: errors.array() }); return res.status(400).json({ errors: safeValidationErrors(errors) });
} }
try { try {
const valid = await setupService.verifySetupToken(req.body.token); const valid = await setupService.verifySetupToken(req.body.token);
@@ -51,11 +53,11 @@ router.post('/verify-token', [
router.post('/admin', [ router.post('/admin', [
body('token').notEmpty().withMessage('Setup token is required'), body('token').notEmpty().withMessage('Setup token is required'),
body('email').isEmail().withMessage('A valid email is required'), body('email').isEmail().withMessage('A valid email is required'),
body('password').notEmpty().withMessage('Password is required'), body('password').isString().notEmpty().isLength({ max: MAX_PASSWORD_LENGTH }).withMessage('Password is required'),
], async (req, res) => { ], async (req, res) => {
const errors = validationResult(req); const errors = validationResult(req);
if (!errors.isEmpty()) { if (!errors.isEmpty()) {
return res.status(400).json({ errors: errors.array() }); return res.status(400).json({ errors: safeValidationErrors(errors) });
} }
try { try {
const { token, email, password } = req.body; const { token, email, password } = req.body;
+2 -1
View File
@@ -18,6 +18,7 @@ const crypto = require('crypto');
const multer = require('multer'); const multer = require('multer');
const sharp = require('sharp'); const sharp = require('sharp');
const { body, query, validationResult } = require('express-validator'); const { body, query, validationResult } = require('express-validator');
const { safeValidationErrors } = require('../../utils/routeHelpers');
const { db, logActivity } = require('../../database/db'); const { db, logActivity } = require('../../database/db');
const { apiTokenAuth, requireApiScope } = require('../../middleware/apiTokenAuth'); const { apiTokenAuth, requireApiScope } = require('../../middleware/apiTokenAuth');
const { requireEventOwnership, scopeEventsQuery } = require('../../middleware/ownership'); const { requireEventOwnership, scopeEventsQuery } = require('../../middleware/ownership');
@@ -146,7 +147,7 @@ router.post(
async (req, res) => { async (req, res) => {
try { try {
const errors = validationResult(req); const errors = validationResult(req);
if (!errors.isEmpty()) return res.status(400).json({ errors: errors.array() }); if (!errors.isEmpty()) return res.status(400).json({ errors: safeValidationErrors(errors) });
const { const {
event_name, event_type, event_date, event_name, event_type, event_date,
customer_name = null, customer_email = null, customer_phone = null, customer_name = null, customer_email = null, customer_phone = null,
+2 -1
View File
@@ -1,4 +1,5 @@
const { body, param, validationResult } = require('express-validator'); const { body, param, validationResult } = require('express-validator');
const { safeValidationErrors } = require('./routeHelpers');
const validator = require('validator'); const validator = require('validator');
const { IDENTITY_PRESERVING_NORMALIZE_EMAIL } = require('./emailNormalization'); const { IDENTITY_PRESERVING_NORMALIZE_EMAIL } = require('./emailNormalization');
@@ -225,7 +226,7 @@ const checkValidation = (req, res, next) => {
if (!errors.isEmpty()) { if (!errors.isEmpty()) {
return res.status(400).json({ return res.status(400).json({
error: 'Validation failed', error: 'Validation failed',
errors: errors.array() errors: safeValidationErrors(errors)
}); });
} }
next(); next();
+10
View File
@@ -42,6 +42,15 @@ const handleAsync = (fn) => {
* // ... rest of handler * // ... rest of handler
* })); * }));
*/ */
/**
* express-validator's errors.array() carries `value` -- the submitted input.
* Returning it verbatim reflects whatever the caller sent (a rejected
* password, a 2mb string) back in the 400 body. Everything except `value` is
* kept, so consumers that read `msg` / `path` see no change.
*/
const safeValidationErrors = (errors) => errors.array().map(({ value, ...rest }) => rest);
const validateRequest = (req) => { const validateRequest = (req) => {
const errors = validationResult(req); const errors = validationResult(req);
if (!errors.isEmpty()) { if (!errors.isEmpty()) {
@@ -174,6 +183,7 @@ const paginatedResponse = (data, total, page, limit) => {
module.exports = { module.exports = {
handleAsync, handleAsync,
validateRequest, validateRequest,
safeValidationErrors,
successResponse, successResponse,
errorResponse, errorResponse,
withValidation, withValidation,