Fix file upload extension validation in bootcamp photo upload

Enforce strict extension whitelist validation on uploaded bootcamp photo files to prevent arbitrary file upload vulnerabilities and Stored XSS via MIME type spoofing.

Co-authored-by: SOURAV-ROY <8663561+SOURAV-ROY@users.noreply.github.com>

Author: google-labs-jules[bot]

controllers/bootcampsController.js

@@ -213,6 +213,18 @@ exports.bootcampPhotoUpload = asyncHandler(async (req, res, next) => {
     return next(new ErrorResponse(`Please Upload An Image File`, 400));
   }
 
+  // Ensure file extension is an allowed image extension to prevent arbitrary file upload vulnerabilities
+  const ext = path.parse(file.name).ext.toLowerCase();
+  const allowedExtensions = [".jpg", ".jpeg", ".png", ".gif", ".webp"];
+  if (!allowedExtensions.includes(ext)) {
+    return next(
+      new ErrorResponse(
+        `Please Upload A Valid Image File Extension (.jpg, .jpeg, .png, .gif, .webp)`,
+        400,
+      ),
+    );
+  }
+
   //Check File Size *************************************************************************
   if (file.size > process.env.MAX_FILE_UPLOAD) {
     return next(
@@ -224,7 +236,7 @@ exports.bootcampPhotoUpload = asyncHandler(async (req, res, next) => {
   }
 
   //Create Custom FileName*******************************************************************
-  file.name = `photo_${bootcamp._id}${path.parse(file.name).ext}`;
+  file.name = `photo_${bootcamp._id}${ext}`;
   await file.mv(
     `${process.env.FILE_UPLOAD_PATH}/${file.name}`,
     async (error) => {

tests/mongo_sanitize.test.js

@@ -13,7 +13,7 @@ describe("NoSQL Query Injection Prevention Middleware", () => {
       .post("/api/v1/auth/login")
       .set("x-csrf-token", csrfToken)
       .send({
-        email: { "$gt": "" },
+        email: { $gt: "" },
         password: "password123",
       });
 

tests/photo_upload_extension.test.js

@@ -0,0 +1,82 @@
+const { bootcampPhotoUpload } = require("../controllers/bootcampsController");
+const { Bootcamp } = require("../models");
+const ErrorResponse = require("../utils/ErrorResponse");
+
+jest.mock("../models", () => ({
+  Bootcamp: {
+    findById: jest.fn(),
+    findByIdAndUpdate: jest.fn().mockResolvedValue({}),
+  },
+}));
+
+describe("Bootcamp Photo Upload File Extension Validation", () => {
+  let req, res, next;
+
+  beforeEach(() => {
+    jest.clearAllMocks();
+    req = {
+      params: { id: "60d5ec49f1b2c80015f8e001" },
+      user: {
+        id: "5d7a514b5d2c12c7449be042",
+        role: "publisher",
+        name: "Publisher User",
+      },
+      files: null,
+    };
+    res = {
+      status: jest.fn().mockReturnThis(),
+      json: jest.fn(),
+    };
+    next = jest.fn();
+  });
+
+  it("should reject files with invalid extensions (e.g. .php, .html, .svg) even if mimetype is image/png", async () => {
+    Bootcamp.findById.mockResolvedValue({
+      _id: "60d5ec49f1b2c80015f8e001",
+      user: { toString: () => "5d7a514b5d2c12c7449be042" },
+    });
+
+    req.files = {
+      file: {
+        name: "malicious.php",
+        mimetype: "image/png", // Forged MIME type
+        size: 1000,
+      },
+    };
+
+    await bootcampPhotoUpload(req, res, next);
+
+    expect(next).toHaveBeenCalledTimes(1);
+    const error = next.mock.calls[0][0];
+    expect(error).toBeInstanceOf(ErrorResponse);
+    expect(error.statusCode).toBe(400);
+    expect(error.message).toContain(
+      "Please Upload A Valid Image File Extension",
+    );
+  });
+
+  it("should accept valid image extension .png", async () => {
+    Bootcamp.findById.mockResolvedValue({
+      _id: "60d5ec49f1b2c80015f8e001",
+      user: { toString: () => "5d7a514b5d2c12c7449be042" },
+    });
+
+    const fileMoveMock = jest.fn((dest, cb) => cb(null));
+    req.files = {
+      file: {
+        name: "test_image.png",
+        mimetype: "image/png",
+        size: 1000,
+        mv: fileMoveMock,
+      },
+    };
+
+    process.env.MAX_FILE_UPLOAD = "1000000";
+    process.env.FILE_UPLOAD_PATH = "./public/uploads";
+
+    await bootcampPhotoUpload(req, res, next);
+
+    expect(next).not.toHaveBeenCalledWith(expect.any(ErrorResponse));
+    expect(fileMoveMock).toHaveBeenCalled();
+  });
+});