From 25bbf5d20b8d3386ab15662749fad8391a380ed6 Mon Sep 17 00:00:00 2001
From: jackiettran <41605212+jackiettran@users.noreply.github.com>
Date: Fri, 12 Dec 2025 13:57:44 -0500
Subject: [PATCH] mass assignment vulnerabilites and refactoring of photos

---
 backend/config/imageLimits.js               |  1 +
 backend/routes/rentals.js                   | 45 ++++++++++++++++++++-
 backend/routes/users.js                     | 33 ++++++++++++++-
 backend/services/damageAssessmentService.js |  4 +-
 frontend/src/config/imageLimits.ts          |  1 +
 5 files changed, 79 insertions(+), 5 deletions(-)

diff --git a/backend/config/imageLimits.js b/backend/config/imageLimits.js
index 7feff25..2c8ad70 100644
--- a/backend/config/imageLimits.js
+++ b/backend/config/imageLimits.js
@@ -6,6 +6,7 @@ const IMAGE_LIMITS = {
   items: 10,
   forum: 10,
   conditionChecks: 10,
+  damageReports: 10,
   profile: 1,
   messages: 1,
 };
diff --git a/backend/routes/rentals.js b/backend/routes/rentals.js
index 523399a..ad637c6 100644
--- a/backend/routes/rentals.js
+++ b/backend/routes/rentals.js
@@ -9,6 +9,8 @@ const LateReturnService = require("../services/lateReturnService");
 const DamageAssessmentService = require("../services/damageAssessmentService");
 const emailServices = require("../services/email");
 const logger = require("../utils/logger");
+const { validateS3Keys } = require("../utils/s3KeyValidator");
+const { IMAGE_LIMITS } = require("../config/imageLimits");
 const router = express.Router();
 
 // 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)
 router.post("/:id/report-damage", authenticateToken, async (req, res, next) => {
   try {
     const rentalId = req.params.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(
       rentalId,
diff --git a/backend/routes/users.js b/backend/routes/users.js
index b2609dd..fb3fb1f 100644
--- a/backend/routes/users.js
+++ b/backend/routes/users.js
@@ -23,6 +23,18 @@ const ALLOWED_PROFILE_FIELDS = [
   '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
  */
@@ -36,6 +48,19 @@ function extractAllowedProfileFields(body) {
   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) => {
   try {
     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) => {
   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);
   } catch (error) {
@@ -103,7 +130,9 @@ router.post('/addresses', authenticateToken, async (req, res, next) => {
 
 router.put('/addresses/:id', authenticateToken, async (req, res, next) => {
   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);
   } catch (error) {
diff --git a/backend/services/damageAssessmentService.js b/backend/services/damageAssessmentService.js
index ad0f947..0b09fc8 100644
--- a/backend/services/damageAssessmentService.js
+++ b/backend/services/damageAssessmentService.js
@@ -19,7 +19,7 @@ class DamageAssessmentService {
       replacementCost,
       proofOfOwnership,
       actualReturnDateTime,
-      photos = [],
+      imageFilenames = [],
     } = damageInfo;
 
     const rental = await Rental.findByPk(rentalId, {
@@ -98,7 +98,7 @@ class DamageAssessmentService {
       needsReplacement,
       replacementCost: needsReplacement ? parseFloat(replacementCost) : null,
       proofOfOwnership: proofOfOwnership || [],
-      photos,
+      imageFilenames,
       assessedAt: new Date(),
       assessedBy: userId,
       feeCalculation,
diff --git a/frontend/src/config/imageLimits.ts b/frontend/src/config/imageLimits.ts
index ac0981f..1770ba8 100644
--- a/frontend/src/config/imageLimits.ts
+++ b/frontend/src/config/imageLimits.ts
@@ -6,6 +6,7 @@ export const IMAGE_LIMITS = {
   items: 10,
   forum: 10,
   conditionChecks: 10,
+  damageReports: 10,
   profile: 1,
   messages: 1,
 } as const;