Skip to content

Commit

Permalink
Merge pull request #5 from bewake24/security
Browse files Browse the repository at this point in the history 
Fix code scanning alert no. 2: Uncontrolled data used in path expression

Solve security issue in security branch and merge to main. 
Solve issues like rate limit, csrf attack, uncontrolled data path expression and more
  • Loading branch information
@bewake24
bewake24 authored Nov 28, 2024
2 parents 5e18d52 + 3ef0e26 commit 99b8ae7
Show file tree
Hide file tree
Showing 28 changed files with 835 additions and 259 deletions.
40 changes: 32 additions & 8 deletions controllers/address.controller.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,12 @@ const addAnAddress = asyncHandler(async (req, res) => {

console.log(`Address added successfully to the user ${userId}`);

ApiResponse.success(res, "Address added successfully", newAddress, 201);
ApiResponse.success(
res,
"Address added successfully",
{ address: newAddress, csrfToken: req.csrfToken() },
201
);
} catch (err) {
if (err.name === MONGOOSE_VALIDATION_ERROR) {
return ApiResponse.validationError(
Expand Down Expand Up @@ -93,13 +98,13 @@ const updateAnAddress = asyncHandler(async (req, res) => {

const addressUser = await Address.findById(addressId).select("userId");
if (!addressUser) {
ApiResponse.notFound(
return ApiResponse.notFound(
res,
"Invalid address ID provided, Address not found"
);
}
if (addressUser.userId.toString() !== req.user._id.toString()) {
ApiResponse.forbidden(
return ApiResponse.forbidden(
res,
"Address doesn't belongs to loggedin user and hence can't update the address"
);
Expand All @@ -115,13 +120,27 @@ const updateAnAddress = asyncHandler(async (req, res) => {
await defaultAddress.save();
}
}
const address = await Address.findByIdAndUpdate(addressId, req.body, {

const validFields = Object.keys(Address.schema.paths);
const updateData = {};
for (const key of Object.keys(req.body)) {
if (validFields.includes(key)) {
updateData[key] = req.body[key];
}
}
const address = await Address.findByIdAndUpdate(addressId, updateData, {
//Don't update data from req.body directly it may lead to NoSQL Injection Attack
new: true,
runValidators: true,
});
console.log("Address updated successfully");

ApiResponse.success(res, "Address updated successfully", address, 200);
ApiResponse.success(
res,
"Address updated successfully",
{ address, csrfToken: req.csrfToken() },
200
);
} catch (err) {
if (err.name === MONGOOSE_CAST_ERROR && err.kind === MONGOOSE_OBJECT_ID) {
return ApiResponse.validationError(
Expand Down Expand Up @@ -149,22 +168,27 @@ const deleteAnAddress = asyncHandler(async (req, res) => {
const addressUser = await Address.findById(addressId).select("userId");

if (!addressUser) {
ApiResponse.notFound(
return ApiResponse.notFound(
res,
"Invalid address ID provided, Address not found"
);
}

if (addressUser.userId.toString() !== req.user._id.toString()) {
ApiResponse.forbidden(
return ApiResponse.forbidden(
res,
"Address doesn't belongs to loggedin user and hence can't delete the address"
);
}

const address = await Address.findByIdAndDelete(addressId);
console.log("Address deleted successfully");
ApiResponse.success(res, "Address deleted successfully", address, 200);
ApiResponse.success(
res,
"Address deleted successfully",
{ csrfToken: req.csrfToken() },
200
);
} catch (err) {
if (err.name === MONGOOSE_CAST_ERROR && err.kind === MONGOOSE_OBJECT_ID) {
return ApiResponse.validationError(
Expand Down
35 changes: 29 additions & 6 deletions controllers/attribute.controller.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,17 @@ const {
MONGOOSE_CAST_ERROR,
MONGOOSE_OBJECT_ID,
} = require("../constants/models.constants");
const { get } = require("mongoose");

const createAttribute = asyncHandler(async (req, res) => {
try {
let { values } = req.body;
values = values.split(",");
const attribute = await Attribute.create({ ...req.body, values });

ApiResponse.success(res, "Attribute created successfully", attribute);
ApiResponse.success(res, "Attribute created successfully", {
attribute,
csrfToken: req.csrfToken(),
});
} catch (error) {
if (error.name === MONGOOSE_VALIDATION_ERROR) {
return ApiResponse.validationError(
Expand Down Expand Up @@ -74,6 +76,19 @@ const updateAnAttribute = asyncHandler(async (req, res) => {
try {
const { name, values } = req.body;

// Protection against NoSQL injection attack
if (name) {
if (typeof name !== "string") {
return ApiResponse.validationError(res, "Invalid input format 1");
}
}
const newValues = values.split(",");
if (newValues) {
if (!Array.isArray(newValues)) {
return ApiResponse.validationError(res, "Invalid input format 2");
}
}

const attribute = await Attribute.findById(req.params.id);

if (!attribute) {
Expand All @@ -84,10 +99,12 @@ const updateAnAttribute = asyncHandler(async (req, res) => {
return ApiResponse.conflict(res, "Attribute already exists", 400);
}
const existingValues = attribute.values;
const newValues = values.split(",");
const updatedAttribute = await Attribute.findByIdAndUpdate(
attribute._id,
{ name, values: [...new Set([...existingValues, ...newValues])] },
{
name: typeof name === "string" ? name : attribute.name, // name should only be string to protect against NoSQL Injection attack
values: [...new Set([...existingValues, ...newValues])],
},
{
new: true,
runValidators: true,
Expand All @@ -97,7 +114,11 @@ const updateAnAttribute = asyncHandler(async (req, res) => {
ApiResponse.success(
res,
"Attribute updated successfully",
updatedAttribute
{
attribute: updatedAttribute,
csrfToken: req.csrfToken(),
},
200
);
} catch (error) {
if (error.name === MONGOOSE_VALIDATION_ERROR) {
Expand Down Expand Up @@ -134,7 +155,9 @@ const deleteAnAttribute = asyncHandler(async (req, res) => {
return ApiResponse.notFound(res, "Attribute not found", 404);
}
await Attribute.findByIdAndDelete(id);
ApiResponse.success(res, "Attribute deleted successfully");
ApiResponse.success(res, "Attribute deleted successfully", {
csrfToken: req.csrfToken(),
});
} catch (error) {
if (
error.name === MONGOOSE_CAST_ERROR &&
Expand Down
14 changes: 12 additions & 2 deletions controllers/cart.controller.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,12 @@ const createACart = asyncHandler(async (req, res) => {
}
const cart = await Cart.create({ customerId: userId, cartItems });

ApiResponse.success(res, "Cart created successfully", cart, 200);
ApiResponse.success(
res,
"Cart created successfully",
{ cart, csrfTolken: req.csrfToken() },
200
);
} catch (error) {
if (
error.name === MONGOOSE_CAST_ERROR &&
Expand Down Expand Up @@ -144,7 +149,12 @@ const addItemsToCart = asyncHandler(async (req, res) => {
{ $set: { cartItems: mergeCartItems(cart.cartItems, cartItems) } },
{ new: true, runValidators: true }
);
ApiResponse.success(res, "Cart updated successfully", updatedCart, 200);
ApiResponse.success(
res,
"Cart updated successfully",
{ cart: updatedCart, csrfTolken: req.csrfToken() },
200
);
} catch (error) {
ApiResponse.error(res, "Error while updating cart", 500, error);
}
Expand Down
50 changes: 38 additions & 12 deletions controllers/category.controller.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,21 @@ const Category = require("../model/category.model");
const Product = require("../model/product.model");
const ApiResponse = require("../utils/ApiResponse");
const asyncHandler = require("../utils/asyncHandler");
const invalidFieldMessage = require("../utils/invalidFieldMessage");

const createCategory = asyncHandler(async (req, res) => {
try {
let thumbnail;
console.log(req.file);
if (req.file) {
thumbnail = req.file.filename;
}
const category = await Category.create({ ...req.body, thumbnail });
ApiResponse.success(res, "Category created successfully", category, 201);
ApiResponse.success(
res,
"Category created successfully",
{ category, csrfToken: req.csrfToken() },
201
);
} catch (error) {
if (error.name === MONGOOSE_VALIDATION_ERROR) {
return ApiResponse.error(res, invalidFieldMessage(error), 400);
Expand Down Expand Up @@ -46,17 +51,14 @@ const getCategories = asyncHandler(async (req, res) => {

const deleteCategory = asyncHandler(async (req, res) => {
try {
console.log(req.params);
const category = await Category.findOne(req.params);

if (!category) {
return ApiResponse.notFound(res, "Category not found", 404);
}

// Use async/await with Promise.all to handle product updates
const products = await Product.find({ categories: category._id });

// Remove the category reference from each product
await Promise.all(
products.map(async (product) => {
product.category = product.category.filter((c) => c !== category._id);
Expand All @@ -67,7 +69,12 @@ const deleteCategory = asyncHandler(async (req, res) => {
// Delete the category
await category.deleteOne();

ApiResponse.success(res, "Category deleted successfully", {});
ApiResponse.success(
res,
"Category deleted successfully",
{ csrfToken: req.csrfToken() },
200
);
} catch (error) {
if (
error.name === MONGOOSE_CAST_ERROR &&
Expand All @@ -82,13 +89,27 @@ const deleteCategory = asyncHandler(async (req, res) => {
const updateCategory = asyncHandler(async (req, res) => {
try {
const { name, slug, isActive } = req.body;

const isProductActive =
isActive === "true" ? true : isActive === "false" ? false : isActive;
if (
typeof name !== "string" ||
typeof slug !== "string" ||
typeof isProductActive !== "boolean"
) {
return ApiResponse.validationError(res, "Invalid input data", {}, 400);
}
let thumbnail;
if (req.file) {
thumbnail = req.file.filename;
}
const category = await Category.find(req.params, {
new: true,
});
const category = await Category.findOne({ slug: req.params.slug }).select(
"_id"
);

if (!category) {
return ApiResponse.notFound(res, "Category not found", 404);
}

if (name) {
// Can we use pre save hooks to save update products category name instead of doing this here.
Expand All @@ -102,8 +123,8 @@ const updateCategory = asyncHandler(async (req, res) => {
});
}

const updatedCategory = await Category.findOneAndUpdate(
req.params,
const updatedCategory = await Category.findByIdAndUpdate(
category._id,
{
name,
slug,
Expand All @@ -113,7 +134,12 @@ const updateCategory = asyncHandler(async (req, res) => {
{ new: true, runValidators: true }
);

ApiResponse.success(res, "Category updated successfully", updatedCategory);
ApiResponse.success(
res,
"Category updated successfully",
{ category: updatedCategory, csrfToken: req.csrfToken() },
200
);
} catch (error) {
ApiResponse.error(res, "Error while updating category", 500);
}
Expand Down
Loading

0 comments on commit 99b8ae7

Please sign in to comment.