Skip to content

Fix SSJS/NoSQL code injection vulnerabilities#2

Draft
Copilot wants to merge 3 commits into
mainfrom
copilot/apply-autofixes-149
Draft

Fix SSJS/NoSQL code injection vulnerabilities#2
Copilot wants to merge 3 commits into
mainfrom
copilot/apply-autofixes-149

Conversation

Copy link
Copy Markdown

Copilot AI commented Apr 7, 2026

Replaces insecure eval() usage in contribution parsing and eliminates NoSQL $where injection in allocation threshold queries by removing the JavaScript-in-query approach entirely.

Changes

app/routes/contributions.js

  • Replace eval(req.body.*) with strict regex validation (/^\d+$/) to reject any non-numeric input upfront, then convert with Number() for preTax, afterTax, and roth
  • Add early input guard returning a user-facing error for invalid input before session destructuring
  • Remove dead /*jslint evil: true */ directive and stale commented-out parseInt block

app/data/allocations-dao.js

  • Parse threshold via parseInt(threshold, 10) and validate it falls within [0, 99]
  • Replace the $where JavaScript-in-query clause entirely with native MongoDB query operators ({ userId: parsedUserId, stocks: { $gt: parsedThreshold } }), eliminating the JavaScript execution attack surface at the query level
  • Throw a descriptive Error on out-of-range values instead of passing raw user input to the query
// Before — arbitrary JS execution
const preTax = eval(req.body.preTax);

// After — strict regex guard + Number() conversion
const isValidContribution = (val) => /^\d+$/.test(String(val));
if (!isValidContribution(req.body.preTax) || ...) {
    return res.render("contributions", { updateError: "Invalid contribution percentages", ... });
}
const preTax = Number(req.body.preTax);
// Before — $where with string interpolation (JS injection surface)
return { $where: `this.userId == ${parsedUserId} && this.stocks > '${threshold}'` };

// After — native MongoDB operators, no JS execution
return { userId: parsedUserId, stocks: { $gt: parsedThreshold } };

Potential fix for alerts

…d fix NoSQL injection

Agent-Logs-Url: https://github.com/Good-Stuff-Inc/codeql-actions-test1/sessions/e159d99d-4081-4501-b92e-172f2e39504d

Co-authored-by: marcosbergami <56553101+marcosbergami@users.noreply.github.com>
Copilot AI changed the title [WIP] Autofix Code Scanning Alert Fix SSJS/NoSQL code injection vulnerabilities Apr 7, 2026
Copilot AI requested a review from marcosbergami April 7, 2026 14:54
@marcosbergami
Copy link
Copy Markdown
Contributor

@copilot modify this resolution and propose another possible alternative to fix this vulnerability.

…r for allocations

Agent-Logs-Url: https://github.com/Good-Stuff-Inc/codeql-actions-test1/sessions/08fc4d3c-8280-45df-a40d-814e84ccfc10

Co-authored-by: marcosbergami <56553101+marcosbergami@users.noreply.github.com>
Copy link
Copy Markdown
Author

Copilot AI commented Apr 7, 2026

@copilot modify this resolution and propose another possible alternative to fix this vulnerability.

Here's the alternative approach implemented in commit fb2cb4b:

contributions.js — instead of parseInt(), now uses strict regex validation (/^\d+$/) to reject any non-numeric input upfront, then converts with Number(). This is stricter than parseInt which silently truncates strings like "5abc" to 5.

allocations-dao.js — instead of sanitizing input for the $where JavaScript-in-query operator, the $where clause is eliminated entirely and replaced with native MongoDB query operators ({ userId: parsedUserId, stocks: { $gt: parsedThreshold } }). This removes the JavaScript execution attack surface at the query level rather than just sanitizing the input.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants