-
Notifications
You must be signed in to change notification settings - Fork 0
fix: patch critical security vulnerabilities across the backend #3
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: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,17 @@ | ||||||||||||
| # Server | ||||||||||||
| PORT=5000 | ||||||||||||
| NODE_ENV=development | ||||||||||||
|
|
||||||||||||
| # Database | ||||||||||||
| DB_HOST=localhost | ||||||||||||
| DB_PORT=3306 | ||||||||||||
| DB_NAME=smartqueue | ||||||||||||
| DB_USER=root | ||||||||||||
| DB_PASSWORD= | ||||||||||||
|
|
||||||||||||
| # Auth | ||||||||||||
| JWT_SECRET=change-me-to-a-strong-random-value | ||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Strengthen the JWT_SECRET placeholder to convey minimum security requirements. The generic placeholder doesn't communicate that JWT HS256 secrets should be at least 256 bits (32+ random bytes). A weak secret undermines the entire authentication system. 🔐 Proposed fix-JWT_SECRET=change-me-to-a-strong-random-value
+JWT_SECRET=use-at-least-32-random-bytes-base64-encoded-minimum-256-bitsOr add a comment above: +# JWT_SECRET must be at least 256 bits (32 bytes). Generate with: node -e "console.log(require('crypto').randomBytes(32).toString('base64'))"
JWT_SECRET=change-me-to-a-strong-random-value📝 Committable suggestion
Suggested change
Suggested change
🤖 Prompt for AI Agents |
||||||||||||
| JWT_EXPIRES_IN=24h | ||||||||||||
|
|
||||||||||||
| # CORS — comma-separated allowed origins | ||||||||||||
| CORS_ORIGIN=http://localhost:5173 | ||||||||||||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,27 @@ | ||||||||||
| const jwt = require('jsonwebtoken'); | ||||||||||
|
|
||||||||||
| const authenticate = (req, res, next) => { | ||||||||||
| const header = req.headers.authorization; | ||||||||||
|
|
||||||||||
| if (!header || !header.startsWith('Bearer ')) { | ||||||||||
| return res.status(401).json({ | ||||||||||
| success: false, | ||||||||||
| message: 'Authentication required.', | ||||||||||
| }); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| const token = header.split(' ')[1]; | ||||||||||
|
|
||||||||||
| try { | ||||||||||
| const decoded = jwt.verify(token, process.env.JWT_SECRET); | ||||||||||
|
Comment on lines
+15
to
+16
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🌐 Web query:
💡 Result: In jsonwebtoken version 9.x, the jwt.verify function does not accept unsigned tokens ("alg": "none") by default [1]. If you need to verify an unsigned token, you must explicitly include 'none' in the algorithms option [1]. When the algorithms option is not specified, jwt.verify defaults to a list of allowed algorithms based on the type of key provided [2][3]: - Secret (HMAC): ['HS256', 'HS384', 'HS512'] [2][3] - RSA: ['RS256', 'RS384', 'RS512'] [2][3] - EC (Elliptic Curve): ['ES256', 'ES384', 'ES512'] [2][3] - Default (if key type cannot be inferred): ['RS256', 'RS384', 'RS512'] [2][3] To verify an unsigned token, you must pass { algorithms: ['none'] } in the options argument [1]. It is strongly recommended to always explicitly specify the expected algorithms to prevent security vulnerabilities, such as algorithm confusion attacks [2][3]. Citations:
🏁 Script executed: #!/bin/bash
set -euo pipefail
# Locate the auth controller and the middleware file mentioned in the review
rg -n "jwt\\.sign\\(" backend/src --hidden || true
rg -n "JWT_SECRET|jwt\\.verify\\(" backend/src/middleware/auth.middleware.js backend/src || true
# Print the relevant sections with line numbers (keep output small)
echo "---- auth.controller.js jwt.sign context ----"
for f in $(fd -t f "auth.controller.js" backend/src -a 2>/dev/null || true); do
echo "FILE: $f"
sed -n '1,220p' "$f" | nl -ba | sed -n '1,220p'
done
echo "---- auth.middleware.js jwt.verify context ----"
if [ -f "backend/src/middleware/auth.middleware.js" ]; then
sed -n '1,120p' backend/src/middleware/auth.middleware.js | nl -ba | sed -n '1,120p'
fiRepository: 0xpritam/SmartQueue Length of output: 605 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "---- backend/src/controllers/auth.controller.js around jwt.sign ----"
sed -n '70,130p' backend/src/controllers/auth.controller.js | cat -n
echo "---- backend/src/middleware/auth.middleware.js around jwt.verify ----"
sed -n '1,80p' backend/src/middleware/auth.middleware.js | cat -nRepository: 0xpritam/SmartQueue Length of output: 2772 Pin the expected algorithm in
🔒 Proposed change- const decoded = jwt.verify(token, process.env.JWT_SECRET);
+ const decoded = jwt.verify(token, process.env.JWT_SECRET, { algorithms: ['HS256'] });📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||
| req.user = decoded; | ||||||||||
| return next(); | ||||||||||
| } catch (err) { | ||||||||||
| return res.status(401).json({ | ||||||||||
| success: false, | ||||||||||
| message: 'Invalid or expired token.', | ||||||||||
| }); | ||||||||||
| } | ||||||||||
| }; | ||||||||||
|
|
||||||||||
| module.exports = { authenticate }; | ||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,14 +1,22 @@ | ||
| const express = require('express'); | ||
| const rateLimit = require('express-rate-limit'); | ||
| const router = express.Router(); | ||
|
|
||
| // 1. Add 'login' to the destructured import | ||
| const { register, login } = require('../controllers/auth.controller'); | ||
|
|
||
| // POST /api/auth/register | ||
| router.post('/register', register); | ||
| // Rate-limit auth endpoints to mitigate brute-force attacks | ||
| const authLimiter = rateLimit({ | ||
| windowMs: 15 * 60 * 1000, // 15 minutes | ||
| max: 20, // 20 attempts per window per IP | ||
| standardHeaders: true, | ||
| legacyHeaders: false, | ||
| message: { | ||
| success: false, | ||
| message: 'Too many requests, please try again later.', | ||
| }, | ||
| }); | ||
|
Comment on lines
+8
to
+17
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🌐 Web query:
💡 Result: In express-rate-limit, the max option is not deprecated in the sense of being removed; rather, it has been superseded by the limit option [1][2]. The option was renamed from max to limit starting in version 7.x to better align with IETF standard draft terminology [2][3]. The package continues to support max for backwards compatibility, meaning existing code using max will still function as expected [1][2]. However, the limit option is the current, preferred configuration property [4][5]. It is recommended to update your configuration to use limit to ensure future compatibility and avoid linter warnings [5]. Additionally, note that the behavior of setting this value to 0 changed in version 7.0.0: whereas it previously acted as a "disable" flag, it now blocks all requests to the endpoint [1][2]. To disable the rate limiter, you should use the skip function instead [6][2]. Citations:
Switch
♻️ Proposed change windowMs: 15 * 60 * 1000, // 15 minutes
- max: 20, // 20 attempts per window per IP
+ limit: 20, // 20 attempts per window per IPAlso, if this service is behind a reverse proxy (Nginx/ELB), configure 🤖 Prompt for AI Agents |
||
|
|
||
| // 2. Add the login route definition | ||
| // This maps to: POST /api/auth/login | ||
| router.post('/login', login); | ||
| router.post('/register', authLimiter, register); | ||
| router.post('/login', authLimiter, login); | ||
|
|
||
| module.exports = router; | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a placeholder password to avoid encouraging empty passwords.
The empty
DB_PASSWORDvalue, even in an example file, may encourage developers to run local environments without passwords, creating a security gap.🔒 Proposed fix
📝 Committable suggestion
🧰 Tools
🪛 dotenv-linter (4.0.0)
[warning] 10-10: [UnorderedKey] The DB_PASSWORD key should go before the DB_PORT key
(UnorderedKey)
🤖 Prompt for AI Agents