From 23cd9cb680eb77b94a97266c3353dfc835f0cc69 Mon Sep 17 00:00:00 2001 From: Paul Nothaft <53005142+the-luap@users.noreply.github.com> Date: Sun, 22 Mar 2026 12:40:01 +0100 Subject: [PATCH] fix: address Shannon security assessment findings (37 vulnerabilities) (#254) Remediate 4 Critical, 18 High, 10 Medium, and 5 Low vulnerabilities identified in the Shannon security assessment (2026-03-20). Critical fixes: - Command injection via rsync SSH key path (INJ-VULN-01) - Self-escalation to super_admin role (AUTHZ-VULN-11) - Invite super_admin backdoor (AUTHZ-VULN-12) - Handlebars SSTI in email templates (INJ-VULN-05) Authentication hardening: - Rate limit on share-link login (AUTH-VULN-01) - X-Forwarded-For spoofing bypass (AUTH-VULN-02) - reCAPTCHA fails closed when misconfigured (AUTH-VULN-03) - Token revocation on admin/gallery logout (AUTH-VULN-04/05) - Cookie Secure flag defaults true in production (AUTH-VULN-06) - Remove JWT from admin login response body (AUTH-VULN-07) - Timing-safe gallery slug validation (AUTH-VULN-09) - Account lockout fails closed on DB error (AUTH-VULN-12) - Session endpoint checks token revocation Path traversal & file access: - checksums endpoint path containment (INJ-VULN-03) - manifest validate path containment (INJ-VULN-04) XSS prevention: - Block SVG data URIs in CSS sanitizer (XSS-VULN-01) - Email preview iframe sandbox (XSS-VULN-02) - SSR branding HTML escaping (XSS-VULN-03) - User-Agent sanitization in feedback (XSS-VULN-04) Authorization (IDOR): - Event ownership middleware for all admin routes - Cross-admin user profile read restriction (AUTHZ-VULN-10) SSRF & infrastructure: - Private IP validation for SMTP, S3, rsync hosts - Replace inline JWT with standard adminAuth middleware - CSRF Content-Type enforcement on mutating API endpoints - CSP headers in nginx location blocks Token revocation fix: - Remove overly broad orWhere clause that invalidated all future tokens - Allow empty-body POST requests (logout) in CSRF middleware Co-authored-by: Paul Nothaft --- backend/server.js | 41 ++++++-- backend/src/middleware/ownership.js | 35 +++++++ backend/src/routes/adminArchives.js | 9 +- backend/src/routes/adminBackup.js | 39 ++++++-- backend/src/routes/adminEmail.js | 20 +++- backend/src/routes/adminEvents.js | 17 ++-- backend/src/routes/adminFeedback.js | 6 ++ backend/src/routes/adminPhotos.js | 31 +++--- backend/src/routes/adminUsers.js | 13 ++- backend/src/routes/auth.js | 38 ++++++-- backend/src/routes/galleryFeedback.js | 2 +- backend/src/routes/secureImages.js | 31 +----- backend/src/services/backupService.js | 43 +++++++- backend/src/services/emailProcessor.js | 17 ++-- backend/src/services/rateLimitService.js | 26 +---- backend/src/services/recaptcha.js | 6 +- backend/src/services/storage/s3Storage.js | 10 ++ backend/src/services/userManagementService.js | 37 ++++++- backend/src/utils/authSecurity.js | 2 +- backend/src/utils/cssSanitizer.js | 7 +- backend/src/utils/networkValidation.js | 97 +++++++++++++++++++ backend/src/utils/passwordValidation.js | 22 ++--- backend/src/utils/requestIp.js | 24 +---- backend/src/utils/tokenRevocation.js | 5 - backend/src/utils/tokenUtils.js | 5 +- frontend/nginx.conf | 10 ++ .../components/admin/EmailPreviewModal.tsx | 1 + 27 files changed, 425 insertions(+), 169 deletions(-) create mode 100644 backend/src/middleware/ownership.js create mode 100644 backend/src/utils/networkValidation.js diff --git a/backend/server.js b/backend/server.js index 839e4351..5e977a40 100644 --- a/backend/server.js +++ b/backend/server.js @@ -194,13 +194,23 @@ function composeInlineStyles(payload) { return cssSegments.join('\n\n'); } +function escapeHtml(str) { + if (!str) return ''; + return String(str) + .replace(/&/g, '&') + .replace(//g, '>') + .replace(/"/g, '"') + .replace(/'/g, '''); +} + function renderBrandHeader(branding) { - const displayName = branding.companyName || 'PicPeak'; - const logoSrc = branding.logoUrl || '/picpeak-logo-transparent.png'; + const displayName = escapeHtml(branding.companyName || 'PicPeak'); + const logoSrc = encodeURI(branding.logoUrl || '/picpeak-logo-transparent.png'); const logo = ``; const tagline = branding.companyTagline - ? `

${branding.companyTagline}

` + ? `

${escapeHtml(branding.companyTagline)}

` : ''; return `
@@ -224,13 +234,14 @@ function renderBrandHeader(branding) { } function renderBrandFooter(branding) { - const displayName = branding.companyName || 'PicPeak'; + const displayName = escapeHtml(branding.companyName || 'PicPeak'); const footerNote = branding.footerText - ? `

${branding.footerText}

` + ? `

${escapeHtml(branding.footerText)}

` : '

Powered by PicPeak to keep every celebration beautifully organised.

'; - const supportLink = branding.supportEmail - ? `Support` + const supportEmail = escapeHtml(branding.supportEmail || ''); + const supportLink = supportEmail + ? `Support` : ''; const legalLinks = ` @@ -282,7 +293,7 @@ function buildPublicSiteDocument(payload) { - ${payload.title} + ${escapeHtml(payload.title)} ${seoMeta} @@ -362,6 +373,20 @@ async function initializeRateLimiters() { app.use(express.json({ limit: '50mb' })); app.use(express.urlencoded({ extended: true, limit: '50mb' })); +// CSRF protection: require JSON Content-Type on mutating API requests +// This blocks cross-origin form submissions which cannot set Content-Type: application/json +app.use('/api', (req, res, next) => { + if (['POST', 'PUT', 'DELETE', 'PATCH'].includes(req.method)) { + const contentType = req.headers['content-type'] || ''; + const contentLength = parseInt(req.headers['content-length'] || '0', 10); + // Allow empty-body requests (e.g. logout), multipart for uploads, and JSON for API calls + if (contentLength > 0 && !contentType.includes('application/json') && !contentType.includes('multipart/form-data')) { + return res.status(415).json({ error: 'Unsupported Content-Type. Use application/json or multipart/form-data.' }); + } + } + next(); +}); + // Request logging for API routes (with timestamps) const apiRequestLogger = (req, res, next) => { try { diff --git a/backend/src/middleware/ownership.js b/backend/src/middleware/ownership.js new file mode 100644 index 00000000..7ebad2ff --- /dev/null +++ b/backend/src/middleware/ownership.js @@ -0,0 +1,35 @@ +const { db } = require('../database/db'); + +/** + * Middleware to enforce event ownership for non-super_admin users. + * Super admins bypass the check. Other admins can only access events they created. + */ +function requireEventOwnership(req, res, next) { + if (req.admin.roleName === 'super_admin') { + return next(); + } + + const eventId = req.params.eventId || req.params.id; + if (!eventId) { + return res.status(400).json({ error: 'Event ID is required' }); + } + + db('events') + .where('id', eventId) + .first() + .then((event) => { + if (!event) { + return res.status(404).json({ error: 'Event not found' }); + } + // Allow access if: event has no owner (legacy/system), or admin owns it + if (event.created_by && event.created_by !== req.admin.id) { + return res.status(403).json({ error: 'Access denied' }); + } + next(); + }) + .catch((err) => { + res.status(500).json({ error: 'Failed to verify ownership' }); + }); +} + +module.exports = { requireEventOwnership }; diff --git a/backend/src/routes/adminArchives.js b/backend/src/routes/adminArchives.js index ef6d2bc1..57fdecbf 100644 --- a/backend/src/routes/adminArchives.js +++ b/backend/src/routes/adminArchives.js @@ -7,6 +7,7 @@ const { adminAuth } = require('../middleware/auth'); const { requirePermission } = require('../middleware/permissions'); const archiver = require('archiver'); const AdmZip = require('adm-zip'); +const { requireEventOwnership } = require('../middleware/ownership'); const router = express.Router(); // Get all archived events @@ -82,7 +83,7 @@ router.get('/', adminAuth, requirePermission('archives.view'), async (req, res) }); // Get single archive details -router.get('/:id', adminAuth, requirePermission('archives.view'), async (req, res) => { +router.get('/:id', adminAuth, requirePermission('archives.view'), requireEventOwnership, async (req, res) => { try { const archive = await db('events') .where('id', req.params.id) @@ -138,7 +139,7 @@ router.get('/:id', adminAuth, requirePermission('archives.view'), async (req, re }); // Restore archive -router.post('/:id/restore', adminAuth, requirePermission('archives.restore'), async (req, res) => { +router.post('/:id/restore', adminAuth, requirePermission('archives.restore'), requireEventOwnership, async (req, res) => { try { const archive = await db('events') .where('id', req.params.id) @@ -301,7 +302,7 @@ router.post('/:id/restore', adminAuth, requirePermission('archives.restore'), as }); // Download archive -router.get('/:id/download', adminAuth, requirePermission('archives.download'), async (req, res) => { +router.get('/:id/download', adminAuth, requirePermission('archives.download'), requireEventOwnership, async (req, res) => { try { const archive = await db('events') .where('id', req.params.id) @@ -350,7 +351,7 @@ router.get('/:id/download', adminAuth, requirePermission('archives.download'), a }); // Delete archive permanently -router.delete('/:id', adminAuth, requirePermission('archives.delete'), async (req, res) => { +router.delete('/:id', adminAuth, requirePermission('archives.delete'), requireEventOwnership, async (req, res) => { try { const archive = await db('events') .where('id', req.params.id) diff --git a/backend/src/routes/adminBackup.js b/backend/src/routes/adminBackup.js index 2539666a..7a030edc 100644 --- a/backend/src/routes/adminBackup.js +++ b/backend/src/routes/adminBackup.js @@ -258,6 +258,13 @@ router.post('/test-connection', adminAuth, requirePermission('backup.create'), a break; } + // SSRF protection: block connections to private/internal addresses + const { isPrivateIP } = require('../utils/networkValidation'); + if (isPrivateIP(host)) { + res.json({ success: false, message: 'Host cannot be a private or internal network address' }); + break; + } + // Validate username format if provided if (user && !/^[a-zA-Z_][a-zA-Z0-9_-]*$/.test(user)) { res.json({ success: false, message: 'Invalid username format' }); @@ -355,12 +362,17 @@ router.get('/manifest/:backupRunId', adminAuth, requirePermission('backup.view') router.post('/manifest/validate', adminAuth, requirePermission('backup.view'), async (req, res) => { try { const { manifestPath } = req.body; - + if (!manifestPath) { return res.status(400).json({ error: 'manifestPath is required' }); } - - const result = await validateBackupManifest(manifestPath); + + // Prevent path traversal — manifest must be within backup directory + const backupBasePath = process.env.BACKUP_PATH || path.join(__dirname, '../../../backups'); + const { safePathJoin } = require('../utils/fileSecurityUtils'); + const safePath = safePathJoin(backupBasePath, manifestPath); + + const result = await validateBackupManifest(safePath); res.json({ valid: result.valid, @@ -456,19 +468,24 @@ router.get('/manifests/:backupId/download', adminAuth, requirePermission('backup router.post('/manifests/validate', adminAuth, requirePermission('backup.view'), async (req, res) => { try { const { manifestPath, manifestData } = req.body; - + if (!manifestPath && !manifestData) { return res.status(400).json({ error: 'Either manifestPath or manifestData is required' }); } - + if (manifestData) { // Validate provided manifest data directly const validationResult = await validateManifestData(manifestData); return res.json(validationResult); } - + + // Prevent path traversal — manifest must be within backup directory + const backupBasePath = process.env.BACKUP_PATH || path.join(__dirname, '../../../backups'); + const { safePathJoin } = require('../utils/fileSecurityUtils'); + const safePath = safePathJoin(backupBasePath, manifestPath); + // Use existing validation function for path - const result = await validateBackupManifest(manifestPath); + const result = await validateBackupManifest(safePath); res.json({ valid: result.valid, @@ -757,10 +774,14 @@ router.get('/checksums', adminAuth, requirePermission('backup.view'), async (req try { const { path: targetPath = '', recursive = true } = req.query; const checksums = {}; - + // Get storage path const storagePath = process.env.STORAGE_PATH || path.join(__dirname, '../../../storage'); - const basePath = targetPath ? path.join(storagePath, targetPath) : storagePath; + let basePath = storagePath; + if (targetPath) { + const { safePathJoin } = require('../utils/fileSecurityUtils'); + basePath = safePathJoin(storagePath, targetPath); + } // Calculate checksums for files async function calculateDirChecksums(dirPath, relative = '') { diff --git a/backend/src/routes/adminEmail.js b/backend/src/routes/adminEmail.js index 7f47f405..6bb29c97 100644 --- a/backend/src/routes/adminEmail.js +++ b/backend/src/routes/adminEmail.js @@ -61,6 +61,12 @@ router.post('/config', [ tls_reject_unauthorized } = req.body; + // Validate SMTP host is not a private/internal address (SSRF protection) + const { isPrivateIP } = require('../utils/networkValidation'); + if (isPrivateIP(smtp_host)) { + return res.status(400).json({ error: 'SMTP host cannot point to a private or internal network address' }); + } + // Check if config exists const existingConfig = await db('email_configs').first(); @@ -455,11 +461,19 @@ router.post('/templates/:key/preview', adminAuth, requirePermission('email.view' let subject = template[subjectField] || template.subject || ''; if (preview_data) { + const escapeHtml = (str) => String(str) + .replace(/&/g, '&') + .replace(//g, '>') + .replace(/"/g, '"') + .replace(/'/g, '''); + Object.keys(preview_data).forEach(key => { const regex = new RegExp(`{{${key}}}`, 'g'); - htmlContent = htmlContent.replace(regex, preview_data[key]); - textContent = textContent.replace(regex, preview_data[key]); - subject = subject.replace(regex, preview_data[key]); + const escapedValue = escapeHtml(preview_data[key]); + htmlContent = htmlContent.replace(regex, escapedValue); + textContent = textContent.replace(regex, preview_data[key]); // text doesn't need HTML escaping + subject = subject.replace(regex, escapeHtml(preview_data[key])); }); } diff --git a/backend/src/routes/adminEvents.js b/backend/src/routes/adminEvents.js index 935b30db..24d49650 100644 --- a/backend/src/routes/adminEvents.js +++ b/backend/src/routes/adminEvents.js @@ -20,6 +20,7 @@ const { buildShareLinkVariants } = require('../services/shareLinkService'); const { parseBooleanInput, parseStringInput } = require('../utils/parsers'); const eventTypeService = require('../services/eventTypeService'); const { validateFileType } = require('../utils/fileSecurityUtils'); +const { requireEventOwnership } = require('../middleware/ownership'); // Shared validator for hero_image_anchor – accepts legacy keywords or "X% Y%" focal point const validateHeroImageAnchor = (value) => { @@ -680,7 +681,7 @@ router.get('/:id', adminAuth, requirePermission('events.view'), async (req, res) }); // Update event -router.put('/:id', adminAuth, requirePermission('events.edit'), [ +router.put('/:id', adminAuth, requirePermission('events.edit'), requireEventOwnership, [ body('event_name').optional().trim().notEmpty(), body('admin_email').optional().isEmail(), body('is_active').optional().isBoolean(), @@ -926,7 +927,7 @@ router.put('/:id', adminAuth, requirePermission('events.edit'), [ }); // Delete event -router.delete('/:id', adminAuth, requirePermission('events.delete'), async (req, res) => { +router.delete('/:id', adminAuth, requirePermission('events.delete'), requireEventOwnership, async (req, res) => { try { const { id } = req.params; @@ -1017,7 +1018,7 @@ router.delete('/:id', adminAuth, requirePermission('events.delete'), async (req, }); // Toggle event status -router.post('/:id/toggle-status', adminAuth, requirePermission('events.edit'), async (req, res) => { +router.post('/:id/toggle-status', adminAuth, requirePermission('events.edit'), requireEventOwnership, async (req, res) => { try { const { id } = req.params; @@ -1057,7 +1058,7 @@ router.post('/:id/toggle-status', adminAuth, requirePermission('events.edit'), a }); // Reset event password -router.post('/:id/reset-password', adminAuth, requirePermission('events.edit'), async (req, res) => { +router.post('/:id/reset-password', adminAuth, requirePermission('events.edit'), requireEventOwnership, async (req, res) => { try { const { id } = req.params; const { sendEmail = true } = req.body; @@ -1124,7 +1125,7 @@ router.post('/:id/reset-password', adminAuth, requirePermission('events.edit'), }); // Resend creation email -router.post('/:id/resend-email', adminAuth, requirePermission('events.edit'), async (req, res) => { +router.post('/:id/resend-email', adminAuth, requirePermission('events.edit'), requireEventOwnership, async (req, res) => { try { const { id } = req.params; @@ -1208,7 +1209,7 @@ router.post('/:id/resend-email', adminAuth, requirePermission('events.edit'), as }); // Archive event -router.post('/:id/archive', adminAuth, requirePermission('events.archive'), async (req, res) => { +router.post('/:id/archive', adminAuth, requirePermission('events.archive'), requireEventOwnership, async (req, res) => { try { const { id } = req.params; @@ -1318,7 +1319,7 @@ router.post('/bulk-archive', adminAuth, requirePermission('events.archive'), [ }); // Upload event custom logo -router.post('/:id/logo', adminAuth, requirePermission('events.edit'), eventLogoUpload.single('logo'), async (req, res) => { +router.post('/:id/logo', adminAuth, requirePermission('events.edit'), requireEventOwnership, eventLogoUpload.single('logo'), async (req, res) => { try { const { id } = req.params; @@ -1373,7 +1374,7 @@ router.post('/:id/logo', adminAuth, requirePermission('events.edit'), eventLogoU }); // Delete event custom logo -router.delete('/:id/logo', adminAuth, requirePermission('events.edit'), async (req, res) => { +router.delete('/:id/logo', adminAuth, requirePermission('events.edit'), requireEventOwnership, async (req, res) => { try { const { id } = req.params; diff --git a/backend/src/routes/adminFeedback.js b/backend/src/routes/adminFeedback.js index 0a38c1a2..03cad49a 100644 --- a/backend/src/routes/adminFeedback.js +++ b/backend/src/routes/adminFeedback.js @@ -12,11 +12,13 @@ const { validateWordFilter, checkValidation } = require('../utils/feedbackValidation'); +const { requireEventOwnership } = require('../middleware/ownership'); // Get event feedback settings router.get('/events/:eventId/feedback-settings', adminAuth, requirePermission('events.view'), + requireEventOwnership, validateEventId, checkValidation, async (req, res) => { @@ -42,6 +44,7 @@ router.get('/events/:eventId/feedback-settings', router.put('/events/:eventId/feedback-settings', adminAuth, requirePermission('events.edit'), + requireEventOwnership, validateEventId, validateFeedbackSettings, checkValidation, @@ -79,6 +82,7 @@ router.put('/events/:eventId/feedback-settings', router.get('/events/:eventId/feedback', adminAuth, requirePermission('events.view'), + requireEventOwnership, validateEventId, checkValidation, async (req, res) => { @@ -204,6 +208,7 @@ router.delete('/feedback/:feedbackId', router.get('/events/:eventId/feedback-analytics', adminAuth, requirePermission('events.view'), + requireEventOwnership, validateEventId, checkValidation, async (req, res) => { @@ -304,6 +309,7 @@ router.get('/events/:eventId/feedback-analytics', router.get('/events/:eventId/feedback/export', adminAuth, requirePermission('events.view'), + requireEventOwnership, validateEventId, checkValidation, async (req, res) => { diff --git a/backend/src/routes/adminPhotos.js b/backend/src/routes/adminPhotos.js index 5aaeaaf6..6882cddf 100644 --- a/backend/src/routes/adminPhotos.js +++ b/backend/src/routes/adminPhotos.js @@ -14,6 +14,7 @@ const { getMaxFilesPerUpload, getAllowedMimeTypes } = require('../services/uploa const { processUploadedPhotos } = require('../services/photoProcessor'); const chunkedUpload = require('../services/chunkedUploadService'); const watermarkGeneratorService = require('../services/watermarkGeneratorService'); +const { requireEventOwnership } = require('../middleware/ownership'); const router = express.Router(); // Get storage path from environment or default @@ -120,7 +121,7 @@ const uploadTimeout = (timeout = 300000) => { // 5 minutes default // Upload photos for an event // Max file count is configurable via general settings -router.post('/:eventId/upload', adminAuth, requirePermission('photos.upload'), uploadTimeout(600000), resolveAllowedTypes, async (req, res, next) => { // 10 minute timeout +router.post('/:eventId/upload', adminAuth, requirePermission('photos.upload'), requireEventOwnership, uploadTimeout(600000), resolveAllowedTypes, async (req, res, next) => { // 10 minute timeout let maxFilesPerUpload; try { maxFilesPerUpload = await getMaxFilesPerUpload(); @@ -522,7 +523,7 @@ router.post('/:eventId/upload', adminAuth, requirePermission('photos.upload'), u }); // Delete a photo -router.delete('/:eventId/photos/:photoId', adminAuth, requirePermission('photos.delete'), async (req, res) => { +router.delete('/:eventId/photos/:photoId', adminAuth, requirePermission('photos.delete'), requireEventOwnership, async (req, res) => { try { const { eventId, photoId } = req.params; @@ -584,7 +585,7 @@ router.delete('/:eventId/photos/:photoId', adminAuth, requirePermission('photos. }); // Update a photo (e.g., change category) -router.patch('/:eventId/photos/:photoId', adminAuth, requirePermission('photos.edit'), async (req, res) => { +router.patch('/:eventId/photos/:photoId', adminAuth, requirePermission('photos.edit'), requireEventOwnership, async (req, res) => { try { const { eventId, photoId } = req.params; const { category_id, visibility } = req.body; @@ -647,7 +648,7 @@ router.patch('/:eventId/photos/:photoId', adminAuth, requirePermission('photos.e }); // Bulk delete photos -router.post('/:eventId/photos/bulk-delete', adminAuth, requirePermission('photos.delete'), async (req, res) => { +router.post('/:eventId/photos/bulk-delete', adminAuth, requirePermission('photos.delete'), requireEventOwnership, async (req, res) => { try { const { eventId } = req.params; const { photoIds } = req.body; @@ -720,7 +721,7 @@ router.post('/:eventId/photos/bulk-delete', adminAuth, requirePermission('photos }); // Bulk update photos -router.post('/:eventId/photos/bulk-update', adminAuth, requirePermission('photos.edit'), async (req, res) => { +router.post('/:eventId/photos/bulk-update', adminAuth, requirePermission('photos.edit'), requireEventOwnership, async (req, res) => { try { const { eventId } = req.params; const { photoIds, updates } = req.body; @@ -783,7 +784,7 @@ router.post('/:eventId/photos/bulk-update', adminAuth, requirePermission('photos }); // Download a photo -router.get('/:eventId/photos/:photoId/download', adminAuth, requirePermission('photos.download'), async (req, res) => { +router.get('/:eventId/photos/:photoId/download', adminAuth, requirePermission('photos.download'), requireEventOwnership, async (req, res) => { try { const { eventId, photoId } = req.params; @@ -815,7 +816,7 @@ router.get('/:eventId/photos/:photoId/download', adminAuth, requirePermission('p }); // Get all photos for an event -router.get('/:eventId/photos', adminAuth, requirePermission('photos.view'), async (req, res) => { +router.get('/:eventId/photos', adminAuth, requirePermission('photos.view'), requireEventOwnership, async (req, res) => { try { const { eventId } = req.params; const { category_id, type, search, sort = 'date' } = req.query; @@ -914,7 +915,7 @@ router.get('/:eventId/photos', adminAuth, requirePermission('photos.view'), asyn }); // Serve photo with admin authentication -router.get('/:eventId/photo/:photoId', adminAuth, requirePermission('photos.view'), async (req, res) => { +router.get('/:eventId/photo/:photoId', adminAuth, requirePermission('photos.view'), requireEventOwnership, async (req, res) => { try { const { eventId, photoId } = req.params; @@ -951,7 +952,7 @@ router.get('/:eventId/photo/:photoId', adminAuth, requirePermission('photos.view }); // Serve thumbnail with admin authentication -router.get('/:eventId/thumbnail/:photoId', adminAuth, requirePermission('photos.view'), async (req, res) => { +router.get('/:eventId/thumbnail/:photoId', adminAuth, requirePermission('photos.view'), requireEventOwnership, async (req, res) => { try { const { eventId, photoId } = req.params; @@ -991,7 +992,7 @@ router.get('/:eventId/thumbnail/:photoId', adminAuth, requirePermission('photos. }); // Debug endpoint to check photo existence -router.get('/:eventId/debug', adminAuth, requirePermission('photos.view'), async (req, res) => { +router.get('/:eventId/debug', adminAuth, requirePermission('photos.view'), requireEventOwnership, async (req, res) => { try { const { eventId } = req.params; @@ -1017,7 +1018,7 @@ router.get('/:eventId/debug', adminAuth, requirePermission('photos.view'), async // ============================================ // Initialize a chunked upload -router.post('/:eventId/chunked-upload/init', adminAuth, requirePermission('photos.upload'), async (req, res) => { +router.post('/:eventId/chunked-upload/init', adminAuth, requirePermission('photos.upload'), requireEventOwnership, async (req, res) => { try { const { eventId } = req.params; const { filename, fileSize, mimeType, totalChunks } = req.body; @@ -1055,7 +1056,7 @@ router.post('/:eventId/chunked-upload/init', adminAuth, requirePermission('photo }); // Upload a chunk -router.post('/:eventId/chunked-upload/:uploadId/chunk/:chunkIndex', adminAuth, requirePermission('photos.upload'), async (req, res) => { +router.post('/:eventId/chunked-upload/:uploadId/chunk/:chunkIndex', adminAuth, requirePermission('photos.upload'), requireEventOwnership, async (req, res) => { try { const { uploadId, chunkIndex } = req.params; @@ -1076,7 +1077,7 @@ router.post('/:eventId/chunked-upload/:uploadId/chunk/:chunkIndex', adminAuth, r }); // Complete chunked upload and process the file -router.post('/:eventId/chunked-upload/:uploadId/complete', adminAuth, requirePermission('photos.upload'), async (req, res) => { +router.post('/:eventId/chunked-upload/:uploadId/complete', adminAuth, requirePermission('photos.upload'), requireEventOwnership, async (req, res) => { try { const { eventId, uploadId } = req.params; const { category_id } = req.body; @@ -1118,7 +1119,7 @@ router.post('/:eventId/chunked-upload/:uploadId/complete', adminAuth, requirePer }); // Get upload status -router.get('/:eventId/chunked-upload/:uploadId/status', adminAuth, requirePermission('photos.view'), async (req, res) => { +router.get('/:eventId/chunked-upload/:uploadId/status', adminAuth, requirePermission('photos.view'), requireEventOwnership, async (req, res) => { try { const { uploadId } = req.params; @@ -1136,7 +1137,7 @@ router.get('/:eventId/chunked-upload/:uploadId/status', adminAuth, requirePermis }); // Abort chunked upload -router.delete('/:eventId/chunked-upload/:uploadId', adminAuth, requirePermission('photos.delete'), async (req, res) => { +router.delete('/:eventId/chunked-upload/:uploadId', adminAuth, requirePermission('photos.delete'), requireEventOwnership, async (req, res) => { try { const { uploadId } = req.params; diff --git a/backend/src/routes/adminUsers.js b/backend/src/routes/adminUsers.js index 38d5ded3..a236b175 100644 --- a/backend/src/routes/adminUsers.js +++ b/backend/src/routes/adminUsers.js @@ -114,7 +114,8 @@ router.post('/invite', [ const invitation = await userManagementService.createInvitation({ email: req.body.email, roleId: req.body.role_id, - invitedById: req.admin.id + invitedById: req.admin.id, + inviterRoleName: req.admin.roleName }); successResponse(res, { invitation }, 201); @@ -146,7 +147,12 @@ router.get('/:id', [ param('id').isInt({ min: 1 }).withMessage('Valid user ID is required') ], handleAsync(async (req, res) => { validateRequest(req); - const user = await userManagementService.getAdminUserById(parseInt(req.params.id)); + const targetId = parseInt(req.params.id); + // Non-super_admin users can only view their own profile + if (req.admin.roleName !== 'super_admin' && targetId !== req.admin.id) { + return res.status(403).json({ error: 'Access denied' }); + } + const user = await userManagementService.getAdminUserById(targetId); res.json({ user: transformUser(user) }); })); @@ -169,7 +175,8 @@ router.put('/:id', [ const user = await userManagementService.updateAdminUser( parseInt(req.params.id), req.body, - req.admin.id + req.admin.id, + { roleName: req.admin.roleName } ); successResponse(res, { user: transformUser(user), message: 'User updated successfully' }); diff --git a/backend/src/routes/auth.js b/backend/src/routes/auth.js index 229f5bc1..a427a8c6 100644 --- a/backend/src/routes/auth.js +++ b/backend/src/routes/auth.js @@ -13,6 +13,7 @@ const { getGenericAuthError } = require('../utils/authSecurity'); const { endSession } = require('../middleware/sessionTimeout'); +const { revokeToken } = require('../utils/tokenRevocation'); const logger = require('../utils/logger'); const { setAdminAuthCookie, @@ -117,9 +118,8 @@ router.post('/admin/login', [ setAdminAuthCookie(res, token); - // Include role in response + // Token is delivered via HttpOnly cookie only (not in response body) res.json({ - token, user: { id: admin.id, username: admin.username, @@ -145,7 +145,8 @@ router.post('/logout', async (req, res) => { const token = adminToken || galleryToken; if (token) { - // End the session + // Revoke the token so it can't be reused, then end the session + await revokeToken(token, 'user_logout'); endSession(token); try { @@ -198,6 +199,8 @@ router.post('/gallery/verify', [ .first(); if (!event) { + // Perform a dummy bcrypt compare to prevent timing-based slug enumeration + await bcrypt.compare(password || '', '$2b$10$abcdefghijklmnopqrstuuABCDEFGHIJKLMNOPQRSTUVWXYZ01234'); await trackFailedAttempt(`gallery:${slug}`, ipAddress, userAgent); return res.status(401).json({ error: 'Invalid gallery or password' }); } @@ -381,6 +384,17 @@ router.post('/gallery/share-login', [ const ipAddress = getClientIp(req); const userAgent = req.headers['user-agent'] || ''; + // Rate limit share-link login attempts + const shareIdentifier = `gallery:${slug}:share`; + const lockoutStatus = await checkAccountLockout(shareIdentifier, ipAddress); + if (lockoutStatus.isLocked) { + logger.warn('Share link login attempt on locked gallery', { slug, ipAddress }); + return res.status(423).json({ + error: 'Too many failed attempts. Please try again later.', + retryAfter: lockoutStatus.remainingTime + }); + } + let event = await db('events') .where({ slug, is_active: formatBoolean(true), is_archived: formatBoolean(false) }) .first(); @@ -393,12 +407,14 @@ router.post('/gallery/share-login', [ } if (!event) { + await trackFailedAttempt(shareIdentifier, ipAddress, userAgent); return res.status(404).json({ error: 'Gallery not found' }); } const expectedToken = getEventShareToken(event); if (!expectedToken || token !== expectedToken) { + await trackFailedAttempt(shareIdentifier, ipAddress, userAgent); return res.status(401).json({ error: 'Invalid or expired share link' }); } @@ -440,10 +456,14 @@ router.post('/gallery/share-login', [ } }); -// Gallery logout to clear cookies +// Gallery logout to clear cookies and revoke token router.post('/gallery/logout', async (req, res) => { try { const { slug } = req.body || {}; + const token = getGalleryTokenFromRequest(req, slug); + if (token) { + await revokeToken(token, 'gallery_logout'); + } clearGalleryAuthCookies(res, slug); res.json({ message: 'Logged out successfully' }); } catch (error) { @@ -464,11 +484,17 @@ router.get('/session', async (req, res) => { try { const decoded = jwt.verify(token, process.env.JWT_SECRET); - + + // Check if token has been revoked (e.g. after logout) + const { isTokenRevoked } = require('../utils/tokenRevocation'); + if (await isTokenRevoked(decoded)) { + return res.status(401).json({ valid: false, error: 'Session has been invalidated' }); + } + // Calculate remaining time const now = Date.now() / 1000; const remainingTime = Math.max(0, decoded.exp - now); - + res.json({ valid: true, type: decoded.type, diff --git a/backend/src/routes/galleryFeedback.js b/backend/src/routes/galleryFeedback.js index 4d3920eb..a65ef327 100644 --- a/backend/src/routes/galleryFeedback.js +++ b/backend/src/routes/galleryFeedback.js @@ -204,7 +204,7 @@ router.post('/:slug/photos/:photoId/feedback', guest_name: req.body.guest_name, guest_email: req.body.guest_email, ip_address: req.ip || req.connection.remoteAddress, - user_agent: req.headers['user-agent'], + user_agent: (req.headers['user-agent'] || '').replace(/[<>&"']/g, '').substring(0, 255), moderate_comments: settings.moderate_comments }; diff --git a/backend/src/routes/secureImages.js b/backend/src/routes/secureImages.js index e1cdf67f..63256211 100644 --- a/backend/src/routes/secureImages.js +++ b/backend/src/routes/secureImages.js @@ -350,34 +350,11 @@ router.get('/:slug/secure-download/:photoId/:token', /** * Get security statistics for monitoring */ -router.get('/security/stats', async (req, res) => { - try { - // Only allow admin access - const token = req.headers.authorization?.split(' ')[1]; - if (!token) { - return res.status(401).json({ error: 'No token provided' }); - } +const { adminAuth } = require('../middleware/auth'); +const { requirePermission } = require('../middleware/permissions'); - const jwt = require('jsonwebtoken'); - // Try to verify with issuer first, fallback to no issuer for backward compatibility - let decoded; - try { - decoded = jwt.verify(token, process.env.JWT_SECRET, { - issuer: 'picpeak-auth' - }); - } catch (issuerError) { - // If verification fails with issuer, try without issuer (backward compatibility) - if (issuerError.name === 'JsonWebTokenError' && issuerError.message.includes('jwt issuer invalid')) { - decoded = jwt.verify(token, process.env.JWT_SECRET); - } else { - throw issuerError; - } - } - const admin = await db('admin_users').where({ id: decoded.id }).first(); - - if (!admin) { - return res.status(401).json({ error: 'Invalid token' }); - } +router.get('/security/stats', adminAuth, requirePermission('settings.view'), async (req, res) => { + try { // Get security statistics const stats = { diff --git a/backend/src/services/backupService.js b/backend/src/services/backupService.js index acbb18f5..69eba927 100644 --- a/backend/src/services/backupService.js +++ b/backend/src/services/backupService.js @@ -437,26 +437,59 @@ async function performLocalBackup(config, files) { }; } +function validateRsyncParam(value, label) { + if (!value || typeof value !== 'string') return null; + if (!/^[a-zA-Z0-9._\/@:-]+$/.test(value)) { + throw new Error(`Invalid ${label}: contains disallowed characters`); + } + if (value.length > 1024) { + throw new Error(`Invalid ${label}: too long`); + } + return value; +} + function buildRsyncArgs(config) { const storagePath = getStoragePath(); - const host = config.backup_rsync_host; - const remotePath = config.backup_rsync_path; + const host = validateRsyncParam(config.backup_rsync_host, 'host'); + const remotePath = validateRsyncParam(config.backup_rsync_path, 'remote path'); if (!host || !remotePath) { throw new Error('Rsync configuration incomplete'); } + // Validate host format (hostname or IP only) + const hostRegex = /^[a-zA-Z0-9]([a-zA-Z0-9-]*[a-zA-Z0-9])?(\.[a-zA-Z0-9]([a-zA-Z0-9-]*[a-zA-Z0-9])?)*$/; + const ipRegex = /^(\d{1,3}\.){3}\d{1,3}$/; + if (!hostRegex.test(host) && !ipRegex.test(host)) { + throw new Error('Invalid rsync host format'); + } + const args = ['-avz', '--delete', '--stats']; if (config.backup_rsync_ssh_key) { - args.push('-e', `ssh -i ${config.backup_rsync_ssh_key} -o StrictHostKeyChecking=no`); + const sshKey = validateRsyncParam(config.backup_rsync_ssh_key, 'SSH key path'); + const fs = require('fs'); + if (!fs.existsSync(sshKey) || !fs.statSync(sshKey).isFile()) { + throw new Error('SSH key file not found or is not a file'); + } + // Pass SSH options as separate array elements to avoid shell interpretation + args.push('-e', `ssh -i ${sshKey} -o StrictHostKeyChecking=no`); } const excludePatterns = config.backup_exclude_patterns || []; excludePatterns.forEach(pattern => args.push('--exclude', pattern)); const source = `${storagePath}/`; - const destination = config.backup_rsync_user - ? `${config.backup_rsync_user}@${host}:${remotePath}` + + const user = config.backup_rsync_user; + if (user) { + validateRsyncParam(user, 'user'); + if (!/^[a-zA-Z_][a-zA-Z0-9_-]*$/.test(user)) { + throw new Error('Invalid rsync username format'); + } + } + + const destination = user + ? `${user}@${host}:${remotePath}` : `${host}:${remotePath}`; args.push(source, destination); diff --git a/backend/src/services/emailProcessor.js b/backend/src/services/emailProcessor.js index e8ea628b..2ea88d0a 100644 --- a/backend/src/services/emailProcessor.js +++ b/backend/src/services/emailProcessor.js @@ -345,15 +345,16 @@ async function processTemplate(template, variables, language = 'en') { processedVariables.welcome_message = formatWelcomeMessage(processedVariables.welcome_message); } - // Compile templates with Handlebars - const subjectTemplate = Handlebars.compile(subject); - const htmlTemplate = Handlebars.compile(htmlBody); - const textTemplate = Handlebars.compile(textBody); + // Safe template replacement (no code execution, only simple variable substitution) + function safeTemplateReplace(template, variables) { + return template.replace(/\{\{(\w+)\}\}/g, (match, key) => + variables.hasOwnProperty(key) ? String(variables[key]) : match + ); + } - // Process templates with processedVariables (includes formatted dates and security messages) - subject = subjectTemplate(processedVariables); - htmlBody = htmlTemplate(processedVariables); - textBody = textTemplate(processedVariables); + subject = safeTemplateReplace(subject, processedVariables); + htmlBody = safeTemplateReplace(htmlBody, processedVariables); + textBody = safeTemplateReplace(textBody, processedVariables); // Inject client access section if client_link is provided (#172) if (processedVariables.client_link) { diff --git a/backend/src/services/rateLimitService.js b/backend/src/services/rateLimitService.js index 2739207f..028fb8a6 100644 --- a/backend/src/services/rateLimitService.js +++ b/backend/src/services/rateLimitService.js @@ -163,22 +163,13 @@ async function createRateLimiter() { const isAuthEndpoint = req.path.match(/\/(auth|login|gallery\/[^/]+\/verify)$/); return isAuthEndpoint ? currentConfig.authMaxRequests : currentConfig.maxRequests; }, - keyGenerator: (req) => { - // Use correct client IP when behind proxy - return req.headers['x-forwarded-for']?.split(',')[0]?.trim() || - req.headers['x-real-ip'] || - req.connection.remoteAddress || - req.ip; - }, + keyGenerator: (req) => req.ip, skip: async (req) => { const currentConfig = await getRateLimitSettings(); return shouldSkipRateLimit(req, currentConfig); }, handler: (req, res) => { - const clientIp = req.headers['x-forwarded-for']?.split(',')[0]?.trim() || - req.headers['x-real-ip'] || - req.connection.remoteAddress || - req.ip; + const clientIp = req.ip; // Enhanced logging for production analysis logger.warn('Rate limit exceeded', { @@ -223,22 +214,13 @@ async function createAuthRateLimiter() { return rateLimit({ windowMs: config.windowMinutes * 60 * 1000, max: config.authMaxRequests, - keyGenerator: (req) => { - // Use correct client IP when behind proxy - return req.headers['x-forwarded-for']?.split(',')[0]?.trim() || - req.headers['x-real-ip'] || - req.connection.remoteAddress || - req.ip; - }, + keyGenerator: (req) => req.ip, skip: async () => { const currentConfig = await getRateLimitSettings(); return !currentConfig.enabled; }, handler: (req, res) => { - const clientIp = req.headers['x-forwarded-for']?.split(',')[0]?.trim() || - req.headers['x-real-ip'] || - req.connection.remoteAddress || - req.ip; + const clientIp = req.ip; // Enhanced logging for auth failures logger.warn('Auth rate limit exceeded', { diff --git a/backend/src/services/recaptcha.js b/backend/src/services/recaptcha.js index 05d49f58..b7891fec 100644 --- a/backend/src/services/recaptcha.js +++ b/backend/src/services/recaptcha.js @@ -30,10 +30,10 @@ async function verifyRecaptcha(token) { return false; } - // If no secret key configured, log warning but pass + // If no secret key configured, fail closed if (!secretKey) { - console.warn('reCAPTCHA enabled but no secret key configured'); - return true; + console.warn('reCAPTCHA enabled but no secret key configured — blocking request'); + return false; } try { diff --git a/backend/src/services/storage/s3Storage.js b/backend/src/services/storage/s3Storage.js index 9a481093..e8eb4b13 100644 --- a/backend/src/services/storage/s3Storage.js +++ b/backend/src/services/storage/s3Storage.js @@ -85,6 +85,16 @@ class S3StorageAdapter extends stream.EventEmitter { endpoint = this.config.sslEnabled ? `https://${endpoint}` : `http://${endpoint}`; } + // SSRF protection: block private/internal S3 endpoints in production + // Local endpoints (e.g. MinIO on localhost) are allowed in development + if (process.env.NODE_ENV === 'production') { + const { validateExternalUrl } = require('../../utils/networkValidation'); + const urlCheck = validateExternalUrl(endpoint); + if (!urlCheck.valid) { + throw new Error(`Invalid S3 endpoint: ${urlCheck.error}`); + } + } + s3Config.endpoint = endpoint; // For S3-compatible services with custom endpoints, force path style diff --git a/backend/src/services/userManagementService.js b/backend/src/services/userManagementService.js index e212dba1..4221a1ff 100644 --- a/backend/src/services/userManagementService.js +++ b/backend/src/services/userManagementService.js @@ -18,7 +18,7 @@ const { ConflictError, NotFoundError, ValidationError } = require('../utils/erro * @param {object} params - { email, roleId, invitedById } * @returns {Promise} Created invitation details */ -async function createInvitation({ email, roleId, invitedById }) { +async function createInvitation({ email, roleId, invitedById, inviterRoleName }) { // Check if email already exists const existingUser = await db('admin_users').where('email', email).first(); if (existingUser) { @@ -42,6 +42,11 @@ async function createInvitation({ email, roleId, invitedById }) { throw new NotFoundError('Role', roleId); } + // Role hierarchy: only super_admin can invite super_admin + if (role.name === 'super_admin' && inviterRoleName !== 'super_admin') { + throw new ValidationError('Only Super Admins can invite new Super Admins'); + } + // Generate secure invitation token (64 characters hex = 32 bytes) const token = crypto.randomBytes(32).toString('hex'); const expiresAt = new Date(Date.now() + 7 * 24 * 60 * 60 * 1000); // 7 days @@ -213,7 +218,7 @@ async function getAdminUserById(id) { * @param {number} updatedById - ID of user making the update * @returns {Promise} Updated user */ -async function updateAdminUser(id, updates, updatedById) { +async function updateAdminUser(id, updates, updatedById, requestingAdmin = {}) { const user = await db('admin_users').where('id', id).first(); if (!user) { throw new NotFoundError('Admin user', id); @@ -248,6 +253,34 @@ async function updateAdminUser(id, updates, updatedById) { if (!role) { throw new NotFoundError('Role', updates.role_id); } + + // Role hierarchy enforcement + const superAdminRole = await db('roles').where('name', 'super_admin').first(); + const isSuperAdmin = requestingAdmin.roleName === 'super_admin'; + + // Only super_admin can assign super_admin role + if (superAdminRole && role.id === superAdminRole.id && !isSuperAdmin) { + throw new ValidationError('Only Super Admins can assign the Super Admin role'); + } + + // Prevent self-role-update + if (id === updatedById) { + throw new ValidationError('Cannot change your own role'); + } + + // Prevent downgrading the last super_admin + if (superAdminRole && user.role_id === superAdminRole.id && role.id !== superAdminRole.id) { + const superAdminCount = await db('admin_users') + .where('role_id', superAdminRole.id) + .where('is_active', formatBoolean(true)) + .count('id as count') + .first(); + + if (Number(superAdminCount?.count) <= 1) { + throw new ValidationError('Cannot demote the last Super Admin'); + } + } + allowedUpdates.role_id = updates.role_id; } diff --git a/backend/src/utils/authSecurity.js b/backend/src/utils/authSecurity.js index 82b61bc4..41204172 100644 --- a/backend/src/utils/authSecurity.js +++ b/backend/src/utils/authSecurity.js @@ -261,7 +261,7 @@ async function checkAccountLockout(identifier, ipAddress) { return { isLocked: false }; } catch (error) { logger.error('Error checking account lockout:', error); - return { isLocked: false }; // Fail open to avoid locking users out due to errors + return { isLocked: true, remainingTime: 300 }; // Fail closed on DB error } } diff --git a/backend/src/utils/cssSanitizer.js b/backend/src/utils/cssSanitizer.js index 97abbc2b..884b78dd 100644 --- a/backend/src/utils/cssSanitizer.js +++ b/backend/src/utils/cssSanitizer.js @@ -29,8 +29,8 @@ const FORBIDDEN_PATTERNS = [ /on\w+\s*=/gi, // onclick=, onload=, etc. ]; -// Pattern for external URLs (block external, allow data: for images) -const EXTERNAL_URL_PATTERN = /url\s*\(\s*["']?(?!data:image)/gi; +// Pattern for external URLs (block external, allow only safe raster data: images) +const EXTERNAL_URL_PATTERN = /url\s*\(\s*["']?(?!data:image\/(?:jpeg|jpg|png|gif|webp))/gi; // Maximum CSS size in bytes (100KB) const MAX_CSS_SIZE = 100 * 1024; @@ -50,7 +50,8 @@ function sanitizeCss(css) { /@charset[^;]+;?/gi, /expression\s*\([^)]*\)/gi, /url\s*\(\s*(['"])\s*javascript:[^)]*\)/gi, - /url\s*\(\s*(['"])\s*data:text\/javascript[^)]*\)/gi + /url\s*\(\s*(['"])\s*data:text\/javascript[^)]*\)/gi, + /url\s*\(\s*(['"]?)\s*data:image\/svg\+xml[^)]*\)/gi ]; disallowedPatterns.forEach((pattern) => { diff --git a/backend/src/utils/networkValidation.js b/backend/src/utils/networkValidation.js new file mode 100644 index 00000000..e5a33984 --- /dev/null +++ b/backend/src/utils/networkValidation.js @@ -0,0 +1,97 @@ +const { URL } = require('url'); +const net = require('net'); + +/** + * Check if a hostname or IP resolves to a private/internal network address. + * Blocks SSRF attempts targeting internal infrastructure. + */ +function isPrivateIP(hostname) { + if (!hostname || typeof hostname !== 'string') return true; + + const lower = hostname.toLowerCase().trim(); + + // Block known metadata / loopback hostnames + const blockedHostnames = [ + 'localhost', + 'metadata.google.internal', + 'metadata.google', + '169.254.169.254', + '0.0.0.0', + '::1', + '[::1]', + ]; + if (blockedHostnames.includes(lower)) return true; + + // If it's an IP address, check ranges directly + if (net.isIPv4(lower)) { + return isPrivateIPv4(lower); + } + + // IPv6 checks + if (net.isIPv6(lower) || lower.startsWith('[')) { + const cleanIp = lower.replace(/^\[|\]$/g, ''); + return isPrivateIPv6(cleanIp); + } + + // Hostname patterns that resolve to internal services + if (lower.endsWith('.internal') || lower.endsWith('.local') || lower.endsWith('.localhost')) { + return true; + } + + return false; +} + +function isPrivateIPv4(ip) { + const parts = ip.split('.').map(Number); + if (parts.length !== 4 || parts.some(p => isNaN(p))) return true; + + const [a, b] = parts; + + // 127.0.0.0/8 — loopback + if (a === 127) return true; + // 10.0.0.0/8 — private + if (a === 10) return true; + // 172.16.0.0/12 — private + if (a === 172 && b >= 16 && b <= 31) return true; + // 192.168.0.0/16 — private + if (a === 192 && b === 168) return true; + // 169.254.0.0/16 — link-local + if (a === 169 && b === 254) return true; + // 0.0.0.0/8 + if (a === 0) return true; + + return false; +} + +function isPrivateIPv6(ip) { + const lower = ip.toLowerCase(); + // ::1 loopback + if (lower === '::1' || lower === '0000:0000:0000:0000:0000:0000:0000:0001') return true; + // fc00::/7 — unique local + if (lower.startsWith('fc') || lower.startsWith('fd')) return true; + // fe80::/10 — link-local + if (lower.startsWith('fe80')) return true; + // :: unspecified + if (lower === '::') return true; + + return false; +} + +/** + * Validate a URL string, rejecting private/internal targets. + * @param {string} urlString - URL to validate + * @returns {{ valid: boolean, error?: string }} + */ +function validateExternalUrl(urlString) { + try { + const parsed = new URL(urlString); + if (isPrivateIP(parsed.hostname)) { + return { valid: false, error: 'URL points to a private or internal network address' }; + } + return { valid: true }; + } catch { + return { valid: false, error: 'Invalid URL format' }; + } +} + +module.exports = { isPrivateIP, validateExternalUrl }; diff --git a/backend/src/utils/passwordValidation.js b/backend/src/utils/passwordValidation.js index 64b1bab2..ca93bd28 100644 --- a/backend/src/utils/passwordValidation.js +++ b/backend/src/utils/passwordValidation.js @@ -213,17 +213,17 @@ async function validatePasswordInContext(password, context, userData = {}) { // Base validation with gallery-specific options const result = validatePassword(password, galleryOptions); - // Override validation for common date formats - // Allow passwords like "04.07.2025", "04/07/2025", "04-07-2025" - const datePattern = /^\d{1,2}[.\/-]\d{1,2}[.\/-]\d{4}$/; - if (datePattern.test(password)) { - // Date format is valid for gallery passwords - return { - valid: true, - errors: [], - score: 2, - feedback: {} - }; + // Only allow date-format passwords when complexity is 'simple' + if (complexityLevel === 'simple') { + const datePattern = /^\d{1,2}[.\/-]\d{1,2}[.\/-]\d{4}$/; + if (datePattern.test(password)) { + return { + valid: true, + errors: [], + score: 2, + feedback: {} + }; + } } // Additional gallery-specific checks diff --git a/backend/src/utils/requestIp.js b/backend/src/utils/requestIp.js index 12af29b0..0fba5817 100644 --- a/backend/src/utils/requestIp.js +++ b/backend/src/utils/requestIp.js @@ -9,28 +9,8 @@ function getClientIp(req) { if (!req) { return ''; } - - const forwardedFor = req.headers['x-forwarded-for']; - - if (typeof forwardedFor === 'string' && forwardedFor.length > 0) { - const [firstIp] = forwardedFor.split(',').map(part => part.trim()).filter(Boolean); - if (firstIp) { - return firstIp; - } - } else if (Array.isArray(forwardedFor) && forwardedFor.length > 0) { - const [firstIp] = forwardedFor; - if (firstIp) { - return firstIp.trim(); - } - } - - return ( - req.ip || - req.connection?.remoteAddress || - req.socket?.remoteAddress || - req.connection?.socket?.remoteAddress || - '' - ); + // Use req.ip which respects Express 'trust proxy' setting + return req.ip || req.connection?.remoteAddress || ''; } module.exports = { getClientIp }; diff --git a/backend/src/utils/tokenRevocation.js b/backend/src/utils/tokenRevocation.js index 70fb9a71..258e3dee 100644 --- a/backend/src/utils/tokenRevocation.js +++ b/backend/src/utils/tokenRevocation.js @@ -57,11 +57,6 @@ async function isTokenRevoked(decodedToken) { const revoked = await db('revoked_tokens') .where('token_id', tokenId) - .orWhere((builder) => { - builder - .where('user_id', decodedToken.id) - .where('revoked_at', '<=', new Date(decodedToken.iat * 1000).toISOString()); - }) .first(); return !!revoked; diff --git a/backend/src/utils/tokenUtils.js b/backend/src/utils/tokenUtils.js index 4207faab..c70ad92b 100644 --- a/backend/src/utils/tokenUtils.js +++ b/backend/src/utils/tokenUtils.js @@ -8,9 +8,8 @@ const secureCookie = (() => { if (typeof process.env.COOKIE_SECURE === 'string') { return process.env.COOKIE_SECURE.toLowerCase() === 'true'; } - // Default to false so native HTTP installs stay functional. Operators can - // opt-in via COOKIE_SECURE=true when serving behind HTTPS. - return false; + // Default to true in production (HTTPS expected), false in development + return process.env.NODE_ENV === 'production'; })(); const sameSiteDefault = process.env.COOKIE_SAMESITE || 'Lax'; const cookieDomain = process.env.COOKIE_DOMAIN; diff --git a/frontend/nginx.conf b/frontend/nginx.conf index 7fe4c5df..8b15f7c3 100644 --- a/frontend/nginx.conf +++ b/frontend/nginx.conf @@ -37,6 +37,11 @@ server { location ~* \.(js|css|png|jpg|jpeg|gif|ico|svg|woff|woff2|ttf|eot)$ { expires 1y; add_header Cache-Control "public, immutable"; + # Re-apply security headers (add_header in location block overrides server-level) + add_header X-Frame-Options "SAMEORIGIN" always; + add_header X-Content-Type-Options "nosniff" always; + add_header Referrer-Policy "strict-origin-when-cross-origin" always; + add_header Content-Security-Policy "default-src 'self'; script-src 'self' https://www.google.com https://www.gstatic.com; style-src 'self' 'unsafe-inline' https:; img-src 'self' data: https: blob:; connect-src 'self' https://www.google.com https://www.gstatic.com; font-src 'self' https: data:; object-src 'none'; media-src 'self'; frame-src 'self' https://www.google.com" always; } # Cache index.html with revalidation @@ -44,6 +49,11 @@ server { add_header Cache-Control "no-cache, no-store, must-revalidate"; add_header Pragma "no-cache"; add_header Expires "0"; + # Re-apply security headers + add_header X-Frame-Options "SAMEORIGIN" always; + add_header X-Content-Type-Options "nosniff" always; + add_header Referrer-Policy "strict-origin-when-cross-origin" always; + add_header Content-Security-Policy "default-src 'self'; script-src 'self' https://www.google.com https://www.gstatic.com; style-src 'self' 'unsafe-inline' https:; img-src 'self' data: https: blob:; connect-src 'self' https://www.google.com https://www.gstatic.com; font-src 'self' https: data:; object-src 'none'; media-src 'self'; frame-src 'self' https://www.google.com" always; } # API proxy diff --git a/frontend/src/components/admin/EmailPreviewModal.tsx b/frontend/src/components/admin/EmailPreviewModal.tsx index d45672b3..ae77da12 100644 --- a/frontend/src/components/admin/EmailPreviewModal.tsx +++ b/frontend/src/components/admin/EmailPreviewModal.tsx @@ -76,6 +76,7 @@ export const EmailPreviewModal: React.FC = ({ srcDoc={htmlContent} className="w-full h-[600px] border-0" title="Email Preview" + sandbox="allow-same-origin" /> ) : (