From 1868a8e6213a5eafe87588a1ec2b32c7cd88ad7b Mon Sep 17 00:00:00 2001 From: seoonju Date: Tue, 15 Jul 2025 13:54:07 +0900 Subject: [PATCH 1/2] [Autofic] Create package.json and CI workflow --- .github/workflows/pr_notify.yml | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 .github/workflows/pr_notify.yml diff --git a/.github/workflows/pr_notify.yml b/.github/workflows/pr_notify.yml new file mode 100644 index 000000000..2b34036d0 --- /dev/null +++ b/.github/workflows/pr_notify.yml @@ -0,0 +1,20 @@ +name: PR Notifier + +on: + pull_request: + types: [opened, reopened, closed] + +jobs: + notify: + runs-on: ubuntu-latest + steps: + - name: Notify Discord + env: + DISCORD_WEBHOOK_URL: ${{ secrets.DISCORD_WEBHOOK_URL }} + run: | + curl -H "Content-Type: application/json" -d '{"content": "🔔 Pull Request [${{ github.event.pull_request.title }}](${{ github.event.pull_request.html_url }}) by ${{ github.event.pull_request.user.login }} - ${{ github.event.action }}"}' $DISCORD_WEBHOOK_URL + - name: Notify Slack + env: + SLACK_WEBHOOK_URL: ${{ secrets.SLACK_WEBHOOK_URL }} + run: | + curl -H "Content-Type: application/json" -d '{"text": ":bell: Pull Request <${{ github.event.pull_request.html_url }}|${{ github.event.pull_request.title }}> by ${{ github.event.pull_request.user.login }} - ${{ github.event.action }}"}' $SLACK_WEBHOOK_URL From e625df244712591cb1112a902f76aaf51416d64f Mon Sep 17 00:00:00 2001 From: seoonju Date: Tue, 15 Jul 2025 13:54:09 +0900 Subject: [PATCH 2/2] [Autofic] 9 malicious code detected!! --- app/data/allocations-dao.js | 12 +----------- app/data/user-dao.js | 9 ++++++++- app/routes/contributions.js | 6 +++--- app/routes/index.js | 20 ++++++++++++++++---- app/routes/profile.js | 2 +- app/routes/research.js | 10 +++++++++- app/routes/session.js | 2 +- server.js | 20 +++----------------- 8 files changed, 42 insertions(+), 39 deletions(-) diff --git a/app/data/allocations-dao.js b/app/data/allocations-dao.js index 24d4718c4..15d1336d4 100644 --- a/app/data/allocations-dao.js +++ b/app/data/allocations-dao.js @@ -60,23 +60,13 @@ const AllocationsDAO = function(db){ const searchCriteria = () => { if (threshold) { - /* // Fix for A1 - 2 NoSQL Injection - escape the threshold parameter properly - // Fix this NoSQL Injection which doesn't sanitze the input parameter 'threshold' and allows attackers - // to inject arbitrary javascript code into the NoSQL query: - // 1. 0';while(true){}' - // 2. 1'; return 1 == '1 - // Also implement fix in allocations.html for UX. const parsedThreshold = parseInt(threshold, 10); if (parsedThreshold >= 0 && parsedThreshold <= 99) { return {$where: `this.userId == ${parsedUserId} && this.stocks > ${parsedThreshold}`}; } throw `The user supplied threshold: ${parsedThreshold} was not valid.`; - */ - return { - $where: `this.userId == ${parsedUserId} && this.stocks > '${threshold}'` - }; } return { userId: parsedUserId @@ -111,4 +101,4 @@ const AllocationsDAO = function(db){ }; -module.exports.AllocationsDAO = AllocationsDAO; +module.exports.AllocationsDAO = AllocationsDAO; \ No newline at end of file diff --git a/app/data/user-dao.js b/app/data/user-dao.js index a674363ef..62d57cca7 100644 --- a/app/data/user-dao.js +++ b/app/data/user-dao.js @@ -88,8 +88,15 @@ function UserDAO(db) { } }; + // Validate and sanitize userName input + if (typeof userName !== 'string' || userName.trim() === '') { + const invalidUserNameError = new Error("Invalid userName"); + invalidUserNameError.invalidUserName = true; + return callback(invalidUserNameError, null); + } + usersCol.findOne({ - userName: userName + userName: userName.trim() }, validateUserDoc); }; diff --git a/app/routes/contributions.js b/app/routes/contributions.js index 7f68170b9..5238c4545 100644 --- a/app/routes/contributions.js +++ b/app/routes/contributions.js @@ -29,9 +29,9 @@ function ContributionsHandler(db) { /*jslint evil: true */ // Insecure use of eval() to parse inputs - const preTax = eval(req.body.preTax); - const afterTax = eval(req.body.afterTax); - const roth = eval(req.body.roth); + const preTax = parseInt(req.body.preTax); + const afterTax = parseInt(req.body.afterTax); + const roth = parseInt(req.body.roth); /* //Fix for A1 -1 SSJS Injection attacks - uses alternate method to eval diff --git a/app/routes/index.js b/app/routes/index.js index a9e55426b..337ab4e10 100644 --- a/app/routes/index.js +++ b/app/routes/index.js @@ -7,6 +7,7 @@ const MemosHandler = require("./memos"); const ResearchHandler = require("./research"); const tutorialRouter = require("./tutorial"); const ErrorHandler = require("./error").errorHandler; +const rateLimit = require("express-rate-limit"); const index = (app, db) => { @@ -26,16 +27,22 @@ const index = (app, db) => { //Middleware to check if user has admin rights const isAdmin = sessionHandler.isAdminUserMiddleware; + // Rate limiting middleware + const limiter = rateLimit({ + windowMs: 15 * 60 * 1000, // 15 minutes + max: 100 // limit each IP to 100 requests per windowMs + }); + // The main page of the app app.get("/", sessionHandler.displayWelcomePage); // Login form app.get("/login", sessionHandler.displayLoginPage); - app.post("/login", sessionHandler.handleLoginRequest); + app.post("/login", limiter, sessionHandler.handleLoginRequest); // Signup form app.get("/signup", sessionHandler.displaySignupPage); - app.post("/signup", sessionHandler.handleSignup); + app.post("/signup", limiter, sessionHandler.handleSignup); // Logout page app.get("/logout", sessionHandler.displayLogoutPage); @@ -68,8 +75,13 @@ const index = (app, db) => { // Handle redirect for learning resources link app.get("/learn", isLoggedIn, (req, res) => { - // Insecure way to handle redirects by taking redirect url from query string - return res.redirect(req.query.url); + const allowedDomains = ['example.com', 'anotherexample.com']; + const url = new URL(req.query.url, `http://${req.headers.host}`); + if (allowedDomains.includes(url.hostname)) { + return res.redirect(req.query.url); + } else { + return res.status(400).send('Invalid redirect URL'); + } }); // Research Page diff --git a/app/routes/profile.js b/app/routes/profile.js index 0b5b34f2d..0bed7b911 100644 --- a/app/routes/profile.js +++ b/app/routes/profile.js @@ -56,7 +56,7 @@ function ProfileHandler(db) { // -- // The Fix: Instead of using greedy quantifiers the same regex will work if we omit the second quantifier + // const regexPattern = /([0-9]+)\#/; - const regexPattern = /([0-9]+)+\#/; + const regexPattern = /[0-9]+\#/; // Allow only numbers with a suffix of the letter #, for example: 'XXXXXX#' const testComplyWithRequirements = regexPattern.test(bankRouting); // if the regex test fails we do not allow saving diff --git a/app/routes/research.js b/app/routes/research.js index c3ae59df6..0e48b2413 100644 --- a/app/routes/research.js +++ b/app/routes/research.js @@ -12,7 +12,15 @@ function ResearchHandler(db) { this.displayResearch = (req, res) => { if (req.query.symbol) { - const url = req.query.url + req.query.symbol; + const allowedDomains = ['https://trustedsource.com', 'https://anothertrustedsource.com']; + const userUrl = req.query.url; + const isValidDomain = allowedDomains.some(domain => userUrl.startsWith(domain)); + + if (!isValidDomain) { + return res.status(400).send("Invalid URL domain."); + } + + const url = userUrl + req.query.symbol; return needle.get(url, (error, newResponse, body) => { if (!error && newResponse.statusCode === 200) { res.writeHead(200, { diff --git a/app/routes/session.js b/app/routes/session.js index 3810fb980..4d4d6cf93 100644 --- a/app/routes/session.js +++ b/app/routes/session.js @@ -140,7 +140,7 @@ function SessionHandler(db) { const USER_RE = /^.{1,20}$/; const FNAME_RE = /^.{1,100}$/; const LNAME_RE = /^.{1,100}$/; - const EMAIL_RE = /^[\S]+@[\S]+\.[\S]+$/; + const EMAIL_RE = /^[^\s@]+@[^\s@]+\.[^\s@]+$/; // Updated regex to avoid ReDoS const PASS_RE = /^.{1,20}$/; /* //Fix for A2-2 - Broken Authentication - requires stronger password diff --git a/server.js b/server.js index d6bb500a2..3f38dd8e0 100644 --- a/server.js +++ b/server.js @@ -82,27 +82,14 @@ MongoClient.connect(db, (err, db) => { secret: cookieSecret, // Both mandatory in Express v4 saveUninitialized: true, - resave: true - /* - // Fix for A5 - Security MisConfig - // Use generic cookie name - key: "sessionId", - */ - - /* - // Fix for A3 - XSS - // TODO: Add "maxAge" + resave: true, cookie: { - httpOnly: true - // Remember to start an HTTPS server to get this working - // secure: true + httpOnly: true, + secure: true // Ensure cookies are sent only over HTTPS } - */ })); - /* - // Fix for A8 - CSRF // Enable Express csrf protection app.use(csrf()); // Make csrf token available in templates @@ -110,7 +97,6 @@ MongoClient.connect(db, (err, db) => { res.locals.csrftoken = req.csrfToken(); next(); }); - */ // Register templating engine app.engine(".html", consolidate.swig);