mass assignment vulnerabilites and refactoring of photos

This commit is contained in:
jackiettran
2025-12-12 13:57:44 -05:00
parent 1dee5232a0
commit 25bbf5d20b
5 changed files with 79 additions and 5 deletions

View File

@@ -6,6 +6,7 @@ const IMAGE_LIMITS = {
items: 10, items: 10,
forum: 10, forum: 10,
conditionChecks: 10, conditionChecks: 10,
damageReports: 10,
profile: 1, profile: 1,
messages: 1, messages: 1,
}; };

View File

@@ -9,6 +9,8 @@ const LateReturnService = require("../services/lateReturnService");
const DamageAssessmentService = require("../services/damageAssessmentService"); const DamageAssessmentService = require("../services/damageAssessmentService");
const emailServices = require("../services/email"); const emailServices = require("../services/email");
const logger = require("../utils/logger"); const logger = require("../utils/logger");
const { validateS3Keys } = require("../utils/s3KeyValidator");
const { IMAGE_LIMITS } = require("../config/imageLimits");
const router = express.Router(); const router = express.Router();
// Helper function to check and update review visibility // Helper function to check and update review visibility
@@ -1257,12 +1259,53 @@ router.post("/:id/mark-return", authenticateToken, async (req, res, next) => {
} }
}); });
// Allowed fields for damage report (prevents mass assignment)
const ALLOWED_DAMAGE_REPORT_FIELDS = [
'description',
'canBeFixed',
'repairCost',
'needsReplacement',
'replacementCost',
'proofOfOwnership',
'actualReturnDateTime',
'imageFilenames',
];
function extractAllowedDamageFields(body) {
const result = {};
for (const field of ALLOWED_DAMAGE_REPORT_FIELDS) {
if (body[field] !== undefined) {
result[field] = body[field];
}
}
return result;
}
// Report item as damaged (owner only) // Report item as damaged (owner only)
router.post("/:id/report-damage", authenticateToken, async (req, res, next) => { router.post("/:id/report-damage", authenticateToken, async (req, res, next) => {
try { try {
const rentalId = req.params.id; const rentalId = req.params.id;
const userId = req.user.id; const userId = req.user.id;
const damageInfo = req.body; // Extract only allowed fields (prevents mass assignment)
const damageInfo = extractAllowedDamageFields(req.body);
// Validate imageFilenames if provided
if (damageInfo.imageFilenames !== undefined) {
const imageFilenamesArray = Array.isArray(damageInfo.imageFilenames)
? damageInfo.imageFilenames
: [];
const keyValidation = validateS3Keys(imageFilenamesArray, 'damage-reports', {
maxKeys: IMAGE_LIMITS.damageReports,
});
if (!keyValidation.valid) {
return res.status(400).json({
error: keyValidation.error,
details: keyValidation.invalidKeys,
});
}
damageInfo.imageFilenames = imageFilenamesArray;
}
const result = await DamageAssessmentService.processDamageAssessment( const result = await DamageAssessmentService.processDamageAssessment(
rentalId, rentalId,

View File

@@ -23,6 +23,18 @@ const ALLOWED_PROFILE_FIELDS = [
'itemRequestNotificationRadius', 'itemRequestNotificationRadius',
]; ];
// Allowed fields for user address create/update (prevents mass assignment)
const ALLOWED_ADDRESS_FIELDS = [
'address1',
'address2',
'city',
'state',
'zipCode',
'country',
'latitude',
'longitude',
];
/** /**
* Extract only allowed fields from request body * Extract only allowed fields from request body
*/ */
@@ -36,6 +48,19 @@ function extractAllowedProfileFields(body) {
return result; return result;
} }
/**
* Extract only allowed address fields from request body
*/
function extractAllowedAddressFields(body) {
const result = {};
for (const field of ALLOWED_ADDRESS_FIELDS) {
if (body[field] !== undefined) {
result[field] = body[field];
}
}
return result;
}
router.get('/profile', authenticateToken, async (req, res, next) => { router.get('/profile', authenticateToken, async (req, res, next) => {
try { try {
const user = await User.findByPk(req.user.id, { const user = await User.findByPk(req.user.id, {
@@ -86,7 +111,9 @@ router.get('/addresses', authenticateToken, async (req, res, next) => {
router.post('/addresses', authenticateToken, async (req, res, next) => { router.post('/addresses', authenticateToken, async (req, res, next) => {
try { try {
const address = await userService.createUserAddress(req.user.id, req.body); // Extract only allowed fields (prevents mass assignment)
const allowedData = extractAllowedAddressFields(req.body);
const address = await userService.createUserAddress(req.user.id, allowedData);
res.status(201).json(address); res.status(201).json(address);
} catch (error) { } catch (error) {
@@ -103,7 +130,9 @@ router.post('/addresses', authenticateToken, async (req, res, next) => {
router.put('/addresses/:id', authenticateToken, async (req, res, next) => { router.put('/addresses/:id', authenticateToken, async (req, res, next) => {
try { try {
const address = await userService.updateUserAddress(req.user.id, req.params.id, req.body); // Extract only allowed fields (prevents mass assignment)
const allowedData = extractAllowedAddressFields(req.body);
const address = await userService.updateUserAddress(req.user.id, req.params.id, allowedData);
res.json(address); res.json(address);
} catch (error) { } catch (error) {

View File

@@ -19,7 +19,7 @@ class DamageAssessmentService {
replacementCost, replacementCost,
proofOfOwnership, proofOfOwnership,
actualReturnDateTime, actualReturnDateTime,
photos = [], imageFilenames = [],
} = damageInfo; } = damageInfo;
const rental = await Rental.findByPk(rentalId, { const rental = await Rental.findByPk(rentalId, {
@@ -98,7 +98,7 @@ class DamageAssessmentService {
needsReplacement, needsReplacement,
replacementCost: needsReplacement ? parseFloat(replacementCost) : null, replacementCost: needsReplacement ? parseFloat(replacementCost) : null,
proofOfOwnership: proofOfOwnership || [], proofOfOwnership: proofOfOwnership || [],
photos, imageFilenames,
assessedAt: new Date(), assessedAt: new Date(),
assessedBy: userId, assessedBy: userId,
feeCalculation, feeCalculation,

View File

@@ -6,6 +6,7 @@ export const IMAGE_LIMITS = {
items: 10, items: 10,
forum: 10, forum: 10,
conditionChecks: 10, conditionChecks: 10,
damageReports: 10,
profile: 1, profile: 1,
messages: 1, messages: 1,
} as const; } as const;