fix: security audit - patch vulnerabilities and regressions#236
Open
Yashagarwal9798 wants to merge 1 commit intoBitWebApp:mainfrom
Open
fix: security audit - patch vulnerabilities and regressions#236Yashagarwal9798 wants to merge 1 commit intoBitWebApp:mainfrom
Yashagarwal9798 wants to merge 1 commit intoBitWebApp:mainfrom
Conversation
|
@Yashagarwal9798 is attempting to deploy a commit to the bitwebapp's projects Team on Vercel. A member of the Team first needs to authorize it. |
Suryansh-Dey
force-pushed
the
main
branch
2 times, most recently
from
ffd2a0e to
a5e184e
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
1.
backend/package.json/backend/package-lock.jsonWhat changed: Added two new security dependencies.
helmetexpress-mongo-sanitize$and.from user input to prevent NoSQL injection attacks2.
backend/src/app.jsWhat changed:
helmetmiddleware withcontentSecurityPolicy: false(disabled CSP to avoid breaking frontend inline scripts/styles; other headers like HSTS, X-Frame-Options are active).express-mongo-sanitizemiddleware after body parsers to sanitize all incoming request data against NoSQL injection ({ "$gt": "" }style attacks).errorHandler,notFoundHandler) was already imported but never mounted — now mounted at the bottom of the middleware chain.What it solves:
3.
backend/src/controllers/admin.controller.jsWhat changed:
{ httpOnly: true, secure: false }to{ httpOnly: true, secure: process.env.NODE_ENV === "production", sameSite: "Lax" }in bothloginAdminandlogoutAdmin.logoutAdminnow passes matching cookie options tores.clearCookie()(browsers ignore clearCookie if options don't match the original set)..select("-password -refeshToken")corrected to.select("-password -refreshToken")inloginAdmin— the typo caused refreshToken to leak in login responses.getCurrendAdminnow selects"-password -refreshToken"instead of just"-password".import { Otp }was placed mid-file (line 254) between function definitions — moved to the top of the file with other imports for proper module structure.What it solves:
4.
backend/src/controllers/awards.controller.jsWhat changed:
deleteAward: Before deleting, verifiesaward.student.toString() === req.user._id.toString(). Users can only delete their own awards.getAwardById: Verifies ownership before returning award data.updateAward: Verifies ownership before allowing updates. Also removedstudentfrom destructuredreq.body— users cannot reassign awards to other students.What it solves:
5.
backend/src/controllers/exam.controller.jsWhat changed:
getExamById: Verifiesexam.name.toString() === req.user._id.toString()before returning data (thenamefield is the User ObjectId reference).What it solves:
6.
backend/src/controllers/higher-education.controller.jsWhat changed:
getHigherEducationById: VerifieshigherEducation.name.toString() === req.user._id.toString()before returning data.What it solves:
7.
backend/src/controllers/internship.controller.jsWhat changed:
getInternshipDataforStudent: Previously acceptedstudent_idfromreq.body(any user could pass any student's ID). Now usesreq.user._iddirectly from the authenticated token.uploadedDoc.urlandInternDocs.urltouploadedDoc.secure_urlandInternDocs.secure_url— ensures HTTPS URLs are stored instead of HTTP."-password -refreshToken"to the.populate("student")call to prevent leaking sensitive fields.What it solves:
8.
backend/src/controllers/interview.controller.jsWhat changed:
sortquery parameter was passed directly to MongoDB.sort(). Now validates against a whitelist of allowed sort fields:["createdAt", "-createdAt", "company", "-company"]. Unrecognized values default to"-createdAt".console.logfor companyId/studentId.What it solves:
9.
backend/src/controllers/profProject.controller.jsWhat changed:
addNewProject: Usesreq.professor._idinstead ofreq.body.profId— prevents professors from creating projects under other professors' accounts.editProject: Verifiesproject.professor.toString() === req.professor._id.toString()before allowing edits.closeProject: Same ownership verification before closing.updateApplicationStatus: PopulatesprojectIdon the application, then verifies the professor owns the referenced project. Added null guard for deleted projects to preventTypeError.getAllApplications: Now queries only applications for the professor's own projects instead of returning all applications system-wide.What it solves:
10.
backend/src/controllers/professor.controller.jsWhat changed:
const url = "http://139.167.188.221:3000/faculty-login"changed toprocess.env.FACULTY_LOGIN_URL || "http://localhost:3000/faculty-login".generateAutoLoginUrlhad a second hardcoded IPhttp://139.167.188.221:3000/faculty-auto-login— changed to useprocess.env.FACULTY_LOGIN_URL.secure: falsetosecure: NODE_ENV === "production", sameSite: "Lax".logoutProfnow passes matching options tores.clearCookie()..select("-password -refeshToken")corrected to-refreshTokeninloginProf.getProfnow selects"-password -refreshToken"— previously only excluded password, exposing all professors' refresh tokens to any caller.Math.random()replaced withcrypto.randomInt(100000, 1000000)inotpForgotPassword— generates cryptographically secure 6-digit OTPs instead of weak 4-digit ones.appliedProfslist before allowing accept/deny operations.Student.updateMany()inmergeGroupschanged toUser.updateMany()—Studentmodel doesn't exist and would crash at runtime.throw new ApiError("Invalid code...")inchangePasswordwas missing the statusCode parameter — changed tonew ApiError(400, "Invalid code...").What it solves:
11.
backend/src/controllers/user.controller.jsWhat changed:
cryptoimport added forcrypto.randomInt().Math.floor(100000 + Math.random() * 900000)replaced withcrypto.randomInt(100000, 1000000)— cryptographically secure 6-digit OTP.Math.floor(Math.random() * 9000 + 1000)(4-digit, insecure) replaced withcrypto.randomInt(100000, 1000000)(6-digit, secure).loginUser, theisVerifiedcheck now happens BEFOREgenerateAcessAndRefreshToken()— previously, tokens were generated and saved to DB even for unverified users.loginUserandlogoutUsernow usesecure: NODE_ENV === "production", sameSite: "Lax".logoutUserpasses matching options tores.clearCookie().getCurrentUsernow selects"-password -refreshToken -marks"instead of just"-marks".User.create()inregisterUserwrapped in try-catch to handle MongoDB E11000 duplicate key errors gracefully (returns 409 instead of crashing with unhandled error).getUserbyRoll: RemovedisAdminfromreq.body— admin/user role now determined byverifyAnyAuthmiddleware. Admin callers get full populated data; regular users get limited fields.What it solves:
12.
backend/src/middlewares/auth.middleware.jsWhat changed:
verifyAdminDB check uncommented: Lines checkingadmin.isAdminwere commented out — now active. Prevents users with admin tokens butisAdmin: falsefrom accessing admin routes.verifyAnyAuthmiddleware added: New middleware that accepts User, Admin, or Professor JWT tokens. Tries Admin first (if token hasisAdminflag), then User, then Professor. Setsreq.admin,req.user, orreq.professoraccordingly. Used for shared endpoints like/getbyroll,/getProf,/get-companies.What it solves:
13.
backend/src/middlewares/multer.middleware.jsWhat changed: Complete rewrite of file upload configuration.
limits: { fileSize: 5 * 1024 * 1024 }).crypto.randomBytes(16).toString("hex")+ original extension. Prevents path traversal and filename injection.What it solves:
.exe,.sh,.php, etc.)14.
backend/src/models/otp.model.jsWhat changed:
createdAtfield withexpires: 600(10 minutes). MongoDB automatically deletes OTP documents after 10 minutes via TTL index.ref:emailfield hadref: "User"which is invalid on String fields (ref is for ObjectId references).import { User }was imported but never used.What it solves:
15.
backend/src/models/professor.model.jsWhat changed:
What it solves:
16.
backend/src/routes/admin.routes.jsWhat changed:
adminAuthLimiter(10 requests per 15 minutes) withrequestIpMiddlewareto the/loginroute./get-companiesopened to all auth types: Changed fromverifyAdmintoverifyAnyAuth— students need company list for the interview experiences page.verifyAnyAuth,createRateLimiter,requestIpMiddleware.What it solves:
17.
backend/src/routes/backlog.routes.jsWhat changed:
verifyAdminto/add-subj: This route for adding backlog subjects had no authentication — anyone could add subjects.What it solves:
18.
backend/src/routes/classroom.routes.jsWhat changed:
verifyJWTto/bookedSlots: This GET route had no authentication — anyone could query all classroom booking information.What it solves:
19.
backend/src/routes/professor.routes.jsWhat changed:
profAuthLimiter(10 req/15 min) to/login.profOtpLimiter(5 req/15 min) to/forgot-pass./getProfauth changed: From no auth →verifyAnyAuth(accessible by students, admins, and professors — all need the professor list).verifyJWTimport.What it solves:
20.
backend/src/routes/user.routes.jsWhat changed:
authLimiter(10 req/15 min) to/login.authLimiterto/register.otpLimiter(5 req/15 min) to/verifyMailand/get-pass-otp./getbyrollauth changed: FromverifyJWTtoverifyAnyAuth— admins need access to this endpoint for the dashboard student search.verifyAnyAuth,createRateLimiter,requestIpMiddleware.What it solves:
21.
backend/src/utils/Socket.jsWhat changed: Complete rewrite of Socket.IO server implementation.
io.use()middleware verifies JWT fromsocket.handshake.auth.tokenor Authorization header. Rejects unauthenticated connections. Resolves user'sfullNamefrom User or Professor collection during auth.Group.findById(roomId)(expects MongoDB ObjectId) toGroup.findOne({ groupId })(matches the nanoid string the frontend sends).m.user.toString()(wrong — members are plain ObjectIds, not objects) tom.toString(), matching the existingchat.controller.jspattern.leader,members,summerAppliedProfs,summerAllocatedProf,minorAllocatedProf,majorAllocatedProf— professors can now access chat rooms.socket.userFullName(resolved server-side during auth) — clients cannot impersonate other users. The sender remains a name string (compatible with existing ChatBox.jsx display logic).What it solves:
memberswas checked)22.
frontend/src/socket.jsWhat changed:
getAccessToken()helper that extracts the JWT fromdocument.cookie(primary method) or falls back tolocalStorage.getItem("accessToken")(used by FacultyAutoLogin). Token is passed viaauth: { token }in the Socket.IO connection options.reconnectionAttemptsoption.What it solves:
localStorage.getItem("token")key was never set by any login flow23.
backend/src/models/professor.model.js(additional fix)What changed:
returnbeforenext()in pre-save hook: TheisModified("password")guard callednext()but didn'treturn, so execution fell through tobcrypt.hash(). Every.save()on a professor document (even for non-password fields) would re-hash the already-hashed password, corrupting it.What it solves:
24.
frontend/src/components/Login.jsxWhat changed:
accessTokenstored to localStorage after login: AddedlocalStorage.setItem("accessToken", res.data.data.accessToken)after a successful login response.What it solves:
socket.js) reads the token from localStorage as a fallback when httpOnly cookies are unreadable by JavaScript. Without this, WebSocket auth was always rejected for student logins.25.
frontend/src/components/Loginadmin.jsxWhat changed:
accessTokenstored to localStorage after login: Same fix as Login.jsx —localStorage.setItem("accessToken", res.data.data.accessToken).What it solves:
26.
frontend/src/components/LoginFaculty.jsxWhat changed:
accessTokenstored to localStorage after login: Same fix —localStorage.setItem("accessToken", res.data.data.accessToken).What it solves:
27.
frontend/src/components/Header.jsx,HeaderFaculty.jsx,HeaderAdmin.jsx,Sidebar.jsx,SideBarFaculty.jsx,SideBarAdmin.jsxWhat changed:
accessTokencleared from localStorage on logout: AddedlocalStorage.removeItem("accessToken")in all 6 logout handlers, immediately after the existinglocalStorage.removeItem("user")/localStorage.removeItem("faculty")calls.What it solves:
28.
backend/src/middlewares/auth.middleware.js(additional fix)What changed:
optionalAuthmiddleware added: Similar toverifyAnyAuthbut non-blocking — if a valid token is present, it resolvesreq.admin,req.user, orreq.professor; if no token or an invalid token is present, it silently callsnext()with no user attached (instead of returning 401).What it solves:
/getbyrollendpoint (student dashboard) needed to work for both authenticated admins (full data) AND unauthenticated/public visitors (limited data).verifyAnyAuthrejected unauthenticated requests with 401, breaking the public dashboard.29.
backend/src/routes/user.routes.js+backend/src/routes/admin.routes.js(additional fix)What changed:
/getbyrollswitched fromverifyAnyAuth→optionalAuth: Allows unauthenticated access with limited data, while admins still get full populated data./get-companiesmade fully public: RemovedverifyAnyAuthmiddleware entirely — company names are non-sensitive and needed by the interview experiences page filter dropdown without requiring login.verifyAnyAuthimport from admin.routes.js after the change.What it solves:
verifyAnyAuthchange)verifyAnyAuthchange)30.
backend/src/controllers/professor.controller.js+backend/src/cron-jobs/notifyProf.js,notifyProfMinor.js,notifyMajorProf.js(additional fix)What changed:
FACULTY_LOGIN_URLandFACULTY_AUTO_LOGIN_URL: The original fix (Login and Signup Added #10) used a singleFACULTY_LOGIN_URLenv var for both the manual login link and the auto-login token URL. These are two different pages (/faculty-loginvs/faculty-auto-login) — a single variable caused one to always be wrong. Now usesFACULTY_AUTO_LOGIN_URL(with fallbackhttp://localhost:3000/faculty-auto-login) for auto-login links."12h"to"30m"— a 12-hour auto-login token is excessive; 30 minutes matches the intended design (as noted in the original code comment).notifyProf.js,notifyProfMinor.js,notifyMajorProf.js) had 2 hardcoded139.167.188.221URLs each (6 total). Replaced withprocess.env.FACULTY_AUTO_LOGIN_URLandprocess.env.FACULTY_LOGIN_URLrespectively.What it solves: