-
Notifications
You must be signed in to change notification settings - Fork 1
chore: update express to version 5.1.0 in package.json #43
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: SOURAV
Are you sure you want to change the base?
Conversation
chore: update express to version 5.1.0 in package.json
The latest updates on your projects. Learn more about Vercel for GitHub.
|
//Create Custom FileName******************************************************************* | ||
file.name = `photo_${bootcamp._id}${path.parse(file.name).ext}`; | ||
await file.mv( | ||
`${process.env.FILE_UPLOAD_PATH}/${file.name}`, |
Check failure
Code scanning / CodeQL
Uncontrolled data used in path expression High
user-provided value
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 1 month ago
The best way to fix this issue is to sanitize the file extension before using it to construct the final file path. Specifically:
- Only allow a predefined set of safe image extensions (
.jpg
,.jpeg
,.png
,.gif
, etc.). - Avoid using arbitrary extensions extracted from user input.
- Alternatively, use a sanitization library or match against a whitelist regular expression and, in case of an invalid extension, reject the upload or default to
.jpg
.
Change needed:
- In
controllers/bootcampsController.js
, at line 228, modify how the extension is determined. Instead of blindly usingpath.parse(file.name).ext
, validate or sanitize it. - Add a helper function (in this file) to check the extension, or implement inline validation.
- If not present, declare the allowed extensions array/set.
- The
.mv
function or path construction must use only the sanitized/validated extension.
No external libraries are strictly needed; this can be done with built-in string manipulation and regular expressions, but you may optionally use a small, well-known package (sanitize-filename
) if shown in the accepted usage above.
-
Copy modified lines R228-R232
@@ -225,7 +225,11 @@ | ||
} | ||
|
||
//Create Custom FileName******************************************************************* | ||
file.name = `photo_${bootcamp._id}${path.parse(file.name).ext}`; | ||
// Only allow safe image extensions | ||
const allowedExts = ['.jpg', '.jpeg', '.png', '.gif', '.webp']; | ||
let origExt = path.parse(file.name).ext.toLowerCase(); | ||
let safeExt = allowedExts.includes(origExt) ? origExt : '.jpg'; | ||
file.name = `photo_${bootcamp._id}${safeExt}`; | ||
await file.mv( | ||
`${process.env.FILE_UPLOAD_PATH}/${file.name}`, | ||
async (error) => { |
console.log(error); | ||
return next(new ErrorResponse(`Problem With File Upload`, 500)); | ||
} | ||
await Bootcamp.findByIdAndUpdate(req.params.id, { photo: file.name }); |
Check failure
Code scanning / CodeQL
Database query built from user-controlled sources High
user-provided value
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 1 month ago
To fix this problem, we should sanitize and restrict the user input that is incorporated into the database update. Specifically, the file extension should be explicitly validated to ensure it is a safe, expected image extension (such as .jpg
, .jpeg
, .png
, .gif
, etc.). Rather than using path.parse(file.name).ext
directly, extract the extension, normalize it, and check if it is in an allowed set. If not, reject the upload.
How to fix:
- Before assigning to
file.name
, extract the original file extension, normalize it (lowercase), check against an allowed list (.jpg
,.jpeg
,.png
,.gif
, etc). - If the extension is not in the allowed list, return an error response.
- Only if valid, construct
file.name
as before. - This validation must be performed before the filename is persisted to the DB and before the file is written to disk.
- No external library is required—use Node's built-in
path
module.
Where to change:
- In
controllers/bootcampsController.js
, inside thebootcampPhotoUpload
method, between lines 211 and 228.
Requirements:
- Implement extension extraction and validation.
- Return an error if the extension is not allowed.
-
Copy modified lines R212-R218 -
Copy modified line R235
@@ -209,6 +209,13 @@ | ||
|
||
const file = req.files.file; | ||
|
||
// Restrict allowed extensions to prevent injection or abuse | ||
const allowedExtensions = ['.jpg', '.jpeg', '.png', '.gif']; | ||
const ext = path.parse(file.name).ext.toLowerCase(); | ||
if (!allowedExtensions.includes(ext)) { | ||
return next(new ErrorResponse(`Please upload a file with one of the following extensions: ${allowedExtensions.join(', ')}`, 400)); | ||
} | ||
|
||
//Make Sure thee image is photo *********************************************************** | ||
if (!file.mimetype.startsWith("image")) { | ||
return next(new ErrorResponse(`Please Upload An Image File`, 400)); | ||
@@ -225,7 +232,7 @@ | ||
} | ||
|
||
//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) => { |
fix: improve bootcamp update logic and add optional chaining for user authorization
|
||
bootcamp = await Bootcamp.findByIdAndUpdate( | ||
bootcampId, | ||
{ ...body }, |
Check failure
Code scanning / CodeQL
Database query built from user-controlled sources High
user-provided value
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 1 month ago
To fix this problem, we must ensure that user-supplied input is NOT used directly as the update object for findByIdAndUpdate
.
The best approach is to flatten the user update so that all properties are applied as literal values, and NEVER allow operators like $set
, $where
, etc. The safest and most common pattern for "accepting any field updates" is to wrap the user-supplied fields in a $set
, but that too can be dangerous if the user is allowed to set e.g. array fields or subdocuments inappropriately.
However, for completeness and general security, we should:
- Only allow a whitelist of field names to be updated (e.g., the set of fields you want users to be able to change—commonly title, description, etc.), and block any unexpected or system fields.
- Apply those fields using
$set
. - If you can't or don't want to maintain a whitelist, then ensure that the update document only sets plain values, and does not allow dangerous update operators to be present at the top-level or in nested objects.
In this case, the best quick fix is to wrap updates in a $set
, i.e., { $set: body }
. Optionally, you can filter body
to only the allowed fields.
Modifications to make:
- Change line 97 from
{ ...body }
to{ $set: body }
. - Optionally, you may want to restrict
body
to a set of known properties before assigning it to ensure users can't change protected or non-editable fields. (But as per instructions, we must not change overall functionality.)
No imports or new dependencies are needed.
-
Copy modified line R97
@@ -94,7 +94,7 @@ | ||
|
||
bootcamp = await Bootcamp.findByIdAndUpdate( | ||
bootcampId, | ||
{ ...body }, | ||
{ $set: body }, | ||
{ | ||
new: true, | ||
runValidators: true, |
chore: update express to version 5.1.0 in package.json