fix(security): implement file upload security enhancements
- Add path traversal protection with secureStatic middleware - Implement proper MIME type validation for all file uploads - Add content-based file validation (magic numbers) - Create comprehensive fileSecurityUtils for secure file operations - Update adminPhotos.js with enhanced validation - Update adminSettings.js for secure logo/favicon uploads - Addresses file upload vulnerabilities from security scan 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
+6
-3
@@ -105,14 +105,17 @@ const setCorsHeaders = (req, res, next) => {
|
|||||||
next();
|
next();
|
||||||
};
|
};
|
||||||
|
|
||||||
|
// Import secure static middleware
|
||||||
|
const secureStatic = require('./src/middleware/secureStatic');
|
||||||
|
|
||||||
// Static file serving for photos (protected)
|
// Static file serving for photos (protected)
|
||||||
app.use('/photos', require('./src/middleware/photoAuth'), setCorsHeaders, express.static(path.join(__dirname, 'storage/events/active')));
|
app.use('/photos', require('./src/middleware/photoAuth'), setCorsHeaders, secureStatic(path.join(__dirname, 'storage/events/active')));
|
||||||
|
|
||||||
// Static file serving for thumbnails (protected)
|
// Static file serving for thumbnails (protected)
|
||||||
app.use('/thumbnails', require('./src/middleware/photoAuth'), setCorsHeaders, express.static(path.join(__dirname, 'storage/thumbnails')));
|
app.use('/thumbnails', require('./src/middleware/photoAuth'), setCorsHeaders, secureStatic(path.join(__dirname, 'storage/thumbnails')));
|
||||||
|
|
||||||
// Static file serving for uploads (public - logos, favicons)
|
// Static file serving for uploads (public - logos, favicons)
|
||||||
app.use('/uploads', setCorsHeaders, express.static(path.join(__dirname, 'storage/uploads')));
|
app.use('/uploads', setCorsHeaders, secureStatic(path.join(__dirname, 'storage/uploads')));
|
||||||
|
|
||||||
// Health check endpoint
|
// Health check endpoint
|
||||||
app.get('/api/health', (req, res) => {
|
app.get('/api/health', (req, res) => {
|
||||||
|
|||||||
@@ -0,0 +1,46 @@
|
|||||||
|
const path = require('path');
|
||||||
|
const express = require('express');
|
||||||
|
const { safePathJoin, isPathSafe } = require('../utils/fileSecurityUtils');
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Create a secure static file serving middleware that prevents path traversal attacks
|
||||||
|
* @param {string} basePath - The base directory to serve files from
|
||||||
|
* @param {Object} options - Express static options
|
||||||
|
* @returns {Function} - Express middleware
|
||||||
|
*/
|
||||||
|
function secureStatic(basePath, options = {}) {
|
||||||
|
const normalizedBase = path.resolve(basePath);
|
||||||
|
|
||||||
|
return (req, res, next) => {
|
||||||
|
// Get the requested file path
|
||||||
|
const requestedPath = req.path;
|
||||||
|
|
||||||
|
// Validate the path doesn't contain dangerous patterns
|
||||||
|
if (!isPathSafe(requestedPath)) {
|
||||||
|
console.warn(`Potential path traversal attempt blocked: ${requestedPath}`);
|
||||||
|
return res.status(403).json({ error: 'Access denied' });
|
||||||
|
}
|
||||||
|
|
||||||
|
try {
|
||||||
|
// Validate the full path is within the base directory
|
||||||
|
const fullPath = safePathJoin(normalizedBase, requestedPath);
|
||||||
|
|
||||||
|
// If validation passes, use express.static
|
||||||
|
const staticMiddleware = express.static(normalizedBase, {
|
||||||
|
...options,
|
||||||
|
// Disable directory listing for security
|
||||||
|
index: false,
|
||||||
|
// Don't allow dotfiles
|
||||||
|
dotfiles: 'deny'
|
||||||
|
});
|
||||||
|
|
||||||
|
return staticMiddleware(req, res, next);
|
||||||
|
} catch (error) {
|
||||||
|
// Path traversal detected
|
||||||
|
console.error(`Path traversal blocked: ${requestedPath}`, error.message);
|
||||||
|
return res.status(403).json({ error: 'Access denied' });
|
||||||
|
}
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
module.exports = secureStatic;
|
||||||
@@ -56,18 +56,18 @@ const storage = multer.diskStorage({
|
|||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
|
const { validateFileType } = require('../utils/fileSecurityUtils');
|
||||||
|
|
||||||
const upload = multer({
|
const upload = multer({
|
||||||
storage: storage,
|
storage: storage,
|
||||||
limits: {
|
limits: {
|
||||||
fileSize: 50 * 1024 * 1024, // 50MB limit
|
fileSize: 50 * 1024 * 1024, // 50MB limit
|
||||||
},
|
},
|
||||||
fileFilter: (req, file, cb) => {
|
fileFilter: (req, file, cb) => {
|
||||||
// Accept images only
|
// Accept images only with proper validation
|
||||||
const allowedTypes = /jpeg|jpg|png|webp/;
|
const allowedMimeTypes = ['image/jpeg', 'image/png', 'image/webp'];
|
||||||
const extname = allowedTypes.test(path.extname(file.originalname).toLowerCase());
|
|
||||||
const mimetype = allowedTypes.test(file.mimetype);
|
|
||||||
|
|
||||||
if (mimetype && extname) {
|
if (validateFileType(file.originalname, file.mimetype, allowedMimeTypes)) {
|
||||||
return cb(null, true);
|
return cb(null, true);
|
||||||
} else {
|
} else {
|
||||||
cb(new Error('Only JPEG, PNG and WebP images are allowed'));
|
cb(new Error('Only JPEG, PNG and WebP images are allowed'));
|
||||||
@@ -75,6 +75,15 @@ const upload = multer({
|
|||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
|
const { createFileUploadValidator } = require('../utils/fileSecurityUtils');
|
||||||
|
|
||||||
|
// Create content validator middleware
|
||||||
|
const validateUploadContent = createFileUploadValidator({
|
||||||
|
allowedTypes: ['image/jpeg', 'image/png', 'image/webp'],
|
||||||
|
maxFileSize: 50 * 1024 * 1024,
|
||||||
|
validateContent: true
|
||||||
|
});
|
||||||
|
|
||||||
// Upload photos for an event
|
// Upload photos for an event
|
||||||
router.post('/:eventId/upload', adminAuth, (req, res, next) => {
|
router.post('/:eventId/upload', adminAuth, (req, res, next) => {
|
||||||
upload.array('photos', 20)(req, res, (err) => {
|
upload.array('photos', 20)(req, res, (err) => {
|
||||||
@@ -90,7 +99,7 @@ router.post('/:eventId/upload', adminAuth, (req, res, next) => {
|
|||||||
}
|
}
|
||||||
next();
|
next();
|
||||||
});
|
});
|
||||||
}, async (req, res) => {
|
}, validateUploadContent, async (req, res) => {
|
||||||
try {
|
try {
|
||||||
const { eventId } = req.params;
|
const { eventId } = req.params;
|
||||||
const { category_id } = req.body;
|
const { category_id } = req.body;
|
||||||
|
|||||||
@@ -21,18 +21,19 @@ const storage = multer.diskStorage({
|
|||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
|
const { validateFileType } = require('../utils/fileSecurityUtils');
|
||||||
|
|
||||||
const upload = multer({
|
const upload = multer({
|
||||||
storage,
|
storage,
|
||||||
limits: { fileSize: 5 * 1024 * 1024 }, // 5MB
|
limits: { fileSize: 5 * 1024 * 1024 }, // 5MB
|
||||||
fileFilter: (req, file, cb) => {
|
fileFilter: (req, file, cb) => {
|
||||||
const allowedTypes = /jpeg|jpg|png|gif|svg/;
|
// Note: SVG files are excluded from magic number validation for logos
|
||||||
const extname = allowedTypes.test(path.extname(file.originalname).toLowerCase());
|
const allowedMimeTypes = ['image/jpeg', 'image/png', 'image/gif', 'image/svg+xml'];
|
||||||
const mimetype = allowedTypes.test(file.mimetype);
|
|
||||||
|
|
||||||
if (mimetype && extname) {
|
if (validateFileType(file.originalname, file.mimetype, allowedMimeTypes)) {
|
||||||
return cb(null, true);
|
return cb(null, true);
|
||||||
} else {
|
} else {
|
||||||
cb(new Error('Only image files are allowed'));
|
cb(new Error('Only JPEG, PNG, GIF and SVG image files are allowed'));
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
@@ -54,8 +55,18 @@ const faviconUpload = multer({
|
|||||||
storage: faviconStorage,
|
storage: faviconStorage,
|
||||||
limits: { fileSize: 1 * 1024 * 1024 }, // 1MB
|
limits: { fileSize: 1 * 1024 * 1024 }, // 1MB
|
||||||
fileFilter: (req, file, cb) => {
|
fileFilter: (req, file, cb) => {
|
||||||
const allowedTypes = ['image/png', 'image/x-icon', 'image/vnd.microsoft.icon'];
|
const allowedMimeTypes = ['image/png', 'image/x-icon', 'image/vnd.microsoft.icon'];
|
||||||
if (allowedTypes.includes(file.mimetype)) {
|
|
||||||
|
// For ICO files, we can't use the standard validateFileType
|
||||||
|
if (file.mimetype === 'image/png') {
|
||||||
|
if (validateFileType(file.originalname, file.mimetype, ['image/png'])) {
|
||||||
|
cb(null, true);
|
||||||
|
} else {
|
||||||
|
cb(new Error('Invalid PNG file'));
|
||||||
|
}
|
||||||
|
} else if (allowedMimeTypes.includes(file.mimetype) &&
|
||||||
|
(file.originalname.toLowerCase().endsWith('.ico') ||
|
||||||
|
file.originalname.toLowerCase().endsWith('.png'))) {
|
||||||
cb(null, true);
|
cb(null, true);
|
||||||
} else {
|
} else {
|
||||||
cb(new Error('Favicon must be PNG or ICO format'));
|
cb(new Error('Favicon must be PNG or ICO format'));
|
||||||
|
|||||||
@@ -0,0 +1,234 @@
|
|||||||
|
const path = require('path');
|
||||||
|
const fs = require('fs').promises;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Secure file security utilities to prevent path traversal and validate file types
|
||||||
|
*/
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Safely join paths and prevent directory traversal attacks
|
||||||
|
* @param {string} basePath - The base directory path
|
||||||
|
* @param {string} userPath - The user-provided path to join
|
||||||
|
* @returns {string} - Safe joined path
|
||||||
|
* @throws {Error} - If path traversal is detected
|
||||||
|
*/
|
||||||
|
function safePathJoin(basePath, userPath) {
|
||||||
|
// Normalize the base path
|
||||||
|
const normalizedBase = path.resolve(basePath);
|
||||||
|
|
||||||
|
// Join and resolve the full path
|
||||||
|
const joinedPath = path.join(normalizedBase, userPath);
|
||||||
|
const resolvedPath = path.resolve(joinedPath);
|
||||||
|
|
||||||
|
// Ensure the resolved path starts with the base path
|
||||||
|
if (!resolvedPath.startsWith(normalizedBase + path.sep) && resolvedPath !== normalizedBase) {
|
||||||
|
throw new Error('Path traversal attempt detected');
|
||||||
|
}
|
||||||
|
|
||||||
|
return resolvedPath;
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Validate file path to prevent directory traversal
|
||||||
|
* @param {string} filePath - The file path to validate
|
||||||
|
* @returns {boolean} - True if path is safe
|
||||||
|
*/
|
||||||
|
function isPathSafe(filePath) {
|
||||||
|
// Check for common path traversal patterns
|
||||||
|
const dangerousPatterns = [
|
||||||
|
/\.\.[\/\\]/, // ../ or ..\
|
||||||
|
/^[\/\\]/, // Absolute paths
|
||||||
|
/^[A-Za-z]:/, // Windows drive letters
|
||||||
|
/[\x00-\x1f]/ // Control characters
|
||||||
|
];
|
||||||
|
|
||||||
|
return !dangerousPatterns.some(pattern => pattern.test(filePath));
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Enhanced MIME type validation
|
||||||
|
*/
|
||||||
|
const ALLOWED_IMAGE_TYPES = {
|
||||||
|
'image/jpeg': {
|
||||||
|
extensions: ['.jpg', '.jpeg'],
|
||||||
|
magicNumbers: [
|
||||||
|
{ offset: 0, bytes: [0xFF, 0xD8, 0xFF] } // JPEG
|
||||||
|
]
|
||||||
|
},
|
||||||
|
'image/png': {
|
||||||
|
extensions: ['.png'],
|
||||||
|
magicNumbers: [
|
||||||
|
{ offset: 0, bytes: [0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A] } // PNG
|
||||||
|
]
|
||||||
|
},
|
||||||
|
'image/webp': {
|
||||||
|
extensions: ['.webp'],
|
||||||
|
magicNumbers: [
|
||||||
|
{ offset: 0, bytes: [0x52, 0x49, 0x46, 0x46] }, // RIFF
|
||||||
|
{ offset: 8, bytes: [0x57, 0x45, 0x42, 0x50] } // WEBP
|
||||||
|
]
|
||||||
|
},
|
||||||
|
'image/gif': {
|
||||||
|
extensions: ['.gif'],
|
||||||
|
magicNumbers: [
|
||||||
|
{ offset: 0, bytes: [0x47, 0x49, 0x46, 0x38, 0x37, 0x61] }, // GIF87a
|
||||||
|
{ offset: 0, bytes: [0x47, 0x49, 0x46, 0x38, 0x39, 0x61] } // GIF89a
|
||||||
|
]
|
||||||
|
},
|
||||||
|
'image/svg+xml': {
|
||||||
|
extensions: ['.svg'],
|
||||||
|
// SVG files are XML-based text files, so we skip magic number validation
|
||||||
|
magicNumbers: null
|
||||||
|
}
|
||||||
|
};
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Validate file type by MIME type and extension
|
||||||
|
* @param {string} filename - The filename
|
||||||
|
* @param {string} mimetype - The MIME type
|
||||||
|
* @param {string[]} allowedTypes - Array of allowed MIME types
|
||||||
|
* @returns {boolean} - True if file type is valid
|
||||||
|
*/
|
||||||
|
function validateFileType(filename, mimetype, allowedTypes) {
|
||||||
|
// Check if MIME type is allowed
|
||||||
|
if (!allowedTypes.includes(mimetype)) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Get file extension
|
||||||
|
const ext = path.extname(filename).toLowerCase();
|
||||||
|
|
||||||
|
// Check if extension matches the MIME type
|
||||||
|
const typeConfig = ALLOWED_IMAGE_TYPES[mimetype];
|
||||||
|
if (!typeConfig || !typeConfig.extensions.includes(ext)) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Validate file content by checking magic numbers (file signatures)
|
||||||
|
* @param {string} filePath - Path to the file
|
||||||
|
* @param {string} expectedMimeType - Expected MIME type
|
||||||
|
* @returns {Promise<boolean>} - True if file content matches expected type
|
||||||
|
*/
|
||||||
|
async function validateFileContent(filePath, expectedMimeType) {
|
||||||
|
try {
|
||||||
|
const typeConfig = ALLOWED_IMAGE_TYPES[expectedMimeType];
|
||||||
|
if (!typeConfig) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Skip validation for file types without magic numbers (like SVG)
|
||||||
|
if (!typeConfig.magicNumbers) {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Read the first 20 bytes of the file (enough for most magic numbers)
|
||||||
|
const buffer = Buffer.alloc(20);
|
||||||
|
const fileHandle = await fs.open(filePath, 'r');
|
||||||
|
await fileHandle.read(buffer, 0, 20, 0);
|
||||||
|
await fileHandle.close();
|
||||||
|
|
||||||
|
// Check magic numbers
|
||||||
|
return typeConfig.magicNumbers.every(magic => {
|
||||||
|
for (let i = 0; i < magic.bytes.length; i++) {
|
||||||
|
if (buffer[magic.offset + i] !== magic.bytes[i]) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return true;
|
||||||
|
});
|
||||||
|
} catch (error) {
|
||||||
|
console.error('Error validating file content:', error);
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Get safe filename for storage
|
||||||
|
* @param {string} originalFilename - Original filename
|
||||||
|
* @returns {string} - Safe filename
|
||||||
|
*/
|
||||||
|
function getSafeFilename(originalFilename) {
|
||||||
|
const timestamp = Date.now();
|
||||||
|
const randomString = Math.random().toString(36).substring(2, 15);
|
||||||
|
const ext = path.extname(originalFilename).toLowerCase();
|
||||||
|
|
||||||
|
// Validate extension
|
||||||
|
const validExtensions = ['.jpg', '.jpeg', '.png', '.webp', '.gif', '.svg', '.ico'];
|
||||||
|
if (!validExtensions.includes(ext)) {
|
||||||
|
throw new Error('Invalid file extension');
|
||||||
|
}
|
||||||
|
|
||||||
|
return `upload_${timestamp}_${randomString}${ext}`;
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Create a file upload validator middleware
|
||||||
|
* @param {Object} options - Validation options
|
||||||
|
* @returns {Function} - Express middleware function
|
||||||
|
*/
|
||||||
|
function createFileUploadValidator(options = {}) {
|
||||||
|
const {
|
||||||
|
allowedTypes = ['image/jpeg', 'image/png', 'image/webp'],
|
||||||
|
maxFileSize = 50 * 1024 * 1024, // 50MB default
|
||||||
|
validateContent = true
|
||||||
|
} = options;
|
||||||
|
|
||||||
|
return async (req, res, next) => {
|
||||||
|
try {
|
||||||
|
if (!req.files || req.files.length === 0) {
|
||||||
|
return next();
|
||||||
|
}
|
||||||
|
|
||||||
|
for (const file of req.files) {
|
||||||
|
// Validate file type
|
||||||
|
if (!validateFileType(file.originalname, file.mimetype, allowedTypes)) {
|
||||||
|
return res.status(400).json({
|
||||||
|
error: `Invalid file type: ${file.originalname}. Allowed types: ${allowedTypes.join(', ')}`
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
// Validate file size
|
||||||
|
if (file.size > maxFileSize) {
|
||||||
|
return res.status(400).json({
|
||||||
|
error: `File too large: ${file.originalname}. Maximum size: ${maxFileSize / 1024 / 1024}MB`
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
// Validate file content if enabled
|
||||||
|
if (validateContent && file.path) {
|
||||||
|
const isValidContent = await validateFileContent(file.path, file.mimetype);
|
||||||
|
if (!isValidContent) {
|
||||||
|
// Remove the file if content doesn't match
|
||||||
|
try {
|
||||||
|
await fs.unlink(file.path);
|
||||||
|
} catch (err) {
|
||||||
|
console.error('Error removing invalid file:', err);
|
||||||
|
}
|
||||||
|
return res.status(400).json({
|
||||||
|
error: `File content does not match declared type: ${file.originalname}`
|
||||||
|
});
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
next();
|
||||||
|
} catch (error) {
|
||||||
|
console.error('File validation error:', error);
|
||||||
|
res.status(500).json({ error: 'File validation failed' });
|
||||||
|
}
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
module.exports = {
|
||||||
|
safePathJoin,
|
||||||
|
isPathSafe,
|
||||||
|
validateFileType,
|
||||||
|
validateFileContent,
|
||||||
|
getSafeFilename,
|
||||||
|
createFileUploadValidator,
|
||||||
|
ALLOWED_IMAGE_TYPES
|
||||||
|
};
|
||||||
Reference in New Issue
Block a user