From a4138bd4eea16bfa733a443ba4051ebc938dfb3e Mon Sep 17 00:00:00 2001 From: Ramjat19 Date: Thu, 30 Oct 2025 18:48:40 +0000 Subject: [PATCH] security features --- backend/src/middleware/validation.ts | 31 +++++++ backend/src/routes/auth.ts | 21 +++++ backend/src/routes/pullRequest.ts | 23 +++-- backend/src/utils/security.ts | 134 +++++++++++++++++++++++++++ 4 files changed, 199 insertions(+), 10 deletions(-) create mode 100644 backend/src/utils/security.ts diff --git a/backend/src/middleware/validation.ts b/backend/src/middleware/validation.ts index 519e765..c80343f 100644 --- a/backend/src/middleware/validation.ts +++ b/backend/src/middleware/validation.ts @@ -1,6 +1,7 @@ import { body, param, query, validationResult } from 'express-validator'; import { Request, Response, NextFunction } from 'express'; import mongoose from 'mongoose'; +import { sanitizeRegexInput as securitySanitizeRegex } from '../utils/security'; // Custom validation functions const isValidObjectId = (value: string) => { @@ -11,6 +12,9 @@ const isValidProjectId = (value: string) => { return value === 'global' || value === 'default' || isValidObjectId(value); }; +// Utility function to sanitize regex input to prevent ReDoS attacks +export const sanitizeRegexInput = securitySanitizeRegex; + // Validation result handler export const handleValidationErrors = (req: Request, res: Response, next: NextFunction) => { const errors = validationResult(req); @@ -147,4 +151,31 @@ export const validateProjectQuery = [ .custom(isValidProjectId) .withMessage('Invalid project ID in query'), handleValidationErrors +]; + +// Pull Request Query Validation +export const validatePRQuery = [ + query('search') + .optional() + .isLength({ max: 100 }) + .withMessage('Search query cannot exceed 100 characters') + .matches(/^[a-zA-Z0-9\s\-_.,!?'"()]+$/) + .withMessage('Search query contains invalid characters'), + query('status') + .optional() + .isIn(['open', 'closed', 'merged', 'all']) + .withMessage('Status must be open, closed, merged, or all'), + query('assignedTo') + .optional() + .isIn(['me', 'unassigned']) + .withMessage('AssignedTo must be me or unassigned'), + query('page') + .optional() + .isInt({ min: 1, max: 1000 }) + .withMessage('Page must be between 1 and 1000'), + query('limit') + .optional() + .isInt({ min: 1, max: 100 }) + .withMessage('Limit must be between 1 and 100'), + handleValidationErrors ]; \ No newline at end of file diff --git a/backend/src/routes/auth.ts b/backend/src/routes/auth.ts index 5007062..e6896fb 100644 --- a/backend/src/routes/auth.ts +++ b/backend/src/routes/auth.ts @@ -5,6 +5,7 @@ import jwt from "jsonwebtoken"; import authMiddleware, { AuthRequest } from "../middleware/auth"; import { validateLogin, validateSignup } from "../middleware/validation"; import { authLimiter } from "../middleware/security"; +import { validateSecureStrings, logSecurityEvent } from "../utils/security"; const router = Router(); @@ -13,6 +14,16 @@ router.post("/signup", authLimiter, validateSignup, async (req: AuthRequest, res try { const { username, email, password } = req.body; + // Ensure all inputs are strings to prevent NoSQL injection + const validation = validateSecureStrings({ email, username, password }); + if (!validation.isValid) { + logSecurityEvent('INVALID_INPUT_TYPE', `Signup attempt with invalid input types: ${validation.invalidFields.join(', ')}`, req); + return res.status(400).json({ + error: "Invalid request", + message: "All fields must be strings" + }); + } + // Check if user exists const existingUser = await User.findOne({ email }); if (existingUser) { @@ -54,6 +65,16 @@ router.post("/login", authLimiter, validateLogin, async (req: AuthRequest, res: try { const { email, password } = req.body; + // Ensure email and password are strings to prevent NoSQL injection + const validation = validateSecureStrings({ email, password }); + if (!validation.isValid) { + logSecurityEvent('NOSQL_INJECTION_ATTEMPT', `Login attempt with invalid input types: ${validation.invalidFields.join(', ')}`, req); + return res.status(400).json({ + error: "Invalid request", + message: "Email and password must be strings" + }); + } + // Find user and include password for comparison const user = await User.findOne({ email }).select('+password'); if (!user) { diff --git a/backend/src/routes/pullRequest.ts b/backend/src/routes/pullRequest.ts index 8fd4d7a..40665ff 100644 --- a/backend/src/routes/pullRequest.ts +++ b/backend/src/routes/pullRequest.ts @@ -3,7 +3,9 @@ import authMiddleware, { AuthRequest } from '../middleware/auth'; import { validateCreatePR, validatePRReview, - validateObjectId + validateObjectId, + validatePRQuery, + sanitizeRegexInput } from '../middleware/validation'; import { strictLimiter } from '../middleware/security'; import PullRequest from '../models/PullRequest'; @@ -64,7 +66,7 @@ router.post('/', strictLimiter, authMiddleware, validateCreatePR, async (req: Au }); // Get pull requests for a repository/project -router.get('/repository/:projectId', authMiddleware, async (req: AuthRequest, res: Response): Promise => { +router.get('/repository/:projectId', authMiddleware, validatePRQuery, async (req: AuthRequest, res: Response): Promise => { try { const { projectId } = req.params; const { status, search, assignedTo, page = 1, limit = 10, simple } = req.query; @@ -83,21 +85,22 @@ router.get('/repository/:projectId', authMiddleware, async (req: AuthRequest, re const filter: any = { repository: projectId }; - // Status filter - if (status && status !== 'all') { + // Status filter - ensure it's a string to prevent NoSQL injection + if (status && typeof status === 'string' && status !== 'all') { filter.status = status; } - // Search filter (title and description) - if (search) { + // Search filter (title and description) - sanitized to prevent ReDoS + if (search && typeof search === 'string') { + const sanitizedSearch = sanitizeRegexInput(search); filter.$or = [ - { title: { $regex: search, $options: 'i' } }, - { description: { $regex: search, $options: 'i' } } + { title: { $regex: sanitizedSearch, $options: 'i' } }, + { description: { $regex: sanitizedSearch, $options: 'i' } } ]; } - // Assigned to filter - if (assignedTo) { + // Assigned to filter - ensure it's a string to prevent NoSQL injection + if (assignedTo && typeof assignedTo === 'string') { if (assignedTo === 'me') { filter.assignedReviewers = req.user.id; } else if (assignedTo === 'unassigned') { diff --git a/backend/src/utils/security.ts b/backend/src/utils/security.ts new file mode 100644 index 0000000..d51d23a --- /dev/null +++ b/backend/src/utils/security.ts @@ -0,0 +1,134 @@ +/** + * Security utilities for preventing common web application vulnerabilities + */ + +import mongoose from 'mongoose'; + +/** + * Validates that an input is a primitive string (prevents NoSQL injection) + * @param value - The value to validate + * @param fieldName - Name of the field for error messages + * @returns boolean indicating if value is a safe string + */ +export const isSecureString = (value: any, fieldName?: string): value is string => { + if (typeof value !== 'string') { + console.log(`Security: Rejected non-string input for ${fieldName || 'field'}: ${typeof value}`); + return false; + } + return true; +}; + +/** + * Validates multiple fields are secure strings + * @param fields - Object with field names and values + * @returns Object with validation results + */ +export const validateSecureStrings = (fields: Record) => { + const invalidFields: string[] = []; + + for (const [fieldName, value] of Object.entries(fields)) { + if (!isSecureString(value, fieldName)) { + invalidFields.push(fieldName); + } + } + + return { + isValid: invalidFields.length === 0, + invalidFields + }; +}; + +/** + * Sanitizes regex input to prevent ReDoS attacks + * @param input - The input string to sanitize + * @param maxLength - Maximum allowed length (default: 50) + * @returns Sanitized string safe for regex usage + */ +export const sanitizeRegexInput = (input: string, maxLength: number = 50): string => { + if (!isSecureString(input)) { + return ''; + } + + // Escape special regex characters and limit length + return input + .replace(/[.*+?^${}()|[\]\\]/g, '\\$&') // Escape regex special chars + .substring(0, maxLength); // Limit length to prevent ReDoS +}; + +/** + * Validates that a value is a valid MongoDB ObjectId + * @param value - The value to validate + * @returns boolean indicating if value is valid ObjectId + */ +export const isValidObjectId = (value: any): value is string => { + return typeof value === 'string' && mongoose.Types.ObjectId.isValid(value); +}; + +/** + * Validates that a value is a valid project ID (ObjectId, 'global', or 'default') + * @param value - The value to validate + * @returns boolean indicating if value is valid project ID + */ +export const isValidProjectId = (value: any): value is string => { + return typeof value === 'string' && + (value === 'global' || value === 'default' || mongoose.Types.ObjectId.isValid(value)); +}; + +/** + * Sanitizes pagination parameters + * @param page - Page number + * @param limit - Items per page + * @returns Sanitized pagination object + */ +export const sanitizePagination = (page: any, limit: any) => { + const pageNum = Math.max(1, Math.min(1000, parseInt(page as string) || 1)); + const limitNum = Math.max(1, Math.min(100, parseInt(limit as string) || 10)); + + return { + page: pageNum, + limit: limitNum, + skip: (pageNum - 1) * limitNum + }; +}; + +/** + * Creates a secure filter object for database queries + * @param baseFilter - Base filter object + * @param allowedFields - Array of allowed field names + * @param inputFilter - User-provided filter object + * @returns Sanitized filter object + */ +export const createSecureFilter = ( + baseFilter: Record, + allowedFields: string[], + inputFilter: Record +): Record => { + const secureFilter = { ...baseFilter }; + + for (const [key, value] of Object.entries(inputFilter)) { + if (allowedFields.includes(key) && isSecureString(value)) { + secureFilter[key] = value; + } + } + + return secureFilter; +}; + +/** + * Logs security events for monitoring + * @param event - Type of security event + * @param details - Additional details about the event + * @param request - Express request object (optional) + */ +export const logSecurityEvent = ( + event: string, + details: string, + request?: any +) => { + const timestamp = new Date().toISOString(); + const ip = request?.ip || 'unknown'; + const userId = request?.user?.id || 'anonymous'; + const username = request?.user?.username || 'anonymous'; + + console.log(`[SECURITY] ${timestamp} - ${event}: ${details} | IP: ${ip} | User: ${username}(${userId})`); +}; \ No newline at end of file