[Security Fix] SAST: Authenticated RCE via eval() applied to request body fields preTax/afterTax/r...#411
Open
okaypatrick wants to merge 1 commit into
Open
Conversation
…s preTax/afterTax/roth.
The handler passed `req.body.preTax`, `req.body.afterTax`, and `req.body.roth` directly to `eval()`. Because the request body is fully attacker-controlled, any authenticated user could submit a string of JavaScript (e.g. `require('child_process').execSync('...')`) and have it executed in the Node.js server process, resulting in remote code execution.
The fix replaces each `eval()` call with `parseInt(..., 10)`, which safely coerces the input to an integer. The existing downstream validations (`
|
Hello @okaypatrick just to let you know that your "AI-Powered Fix Generator" does not have any intelligence to figure out this is an intentionally vulnerable app which does not need fixing 🤣 please stop spamming with useless PRs |
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
2 participants
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.
Security Fix
Type: SAST
Generated by: AI-Powered Fix Generator
Finding: Authenticated RCE via eval() applied to request body fields preTax/afterTax/roth.
Rule: claude-injection-eval-contributions (oogway-scanner)
File:
app/routes/contributions.js(line 33)Severity: HIGH
Explanation
The handler passed
req.body.preTax,req.body.afterTax, andreq.body.rothdirectly toeval(). Because the request body is fully attacker-controlled, any authenticated user could submit a string of JavaScript (e.g.require('child_process').execSync('...')) and have it executed in the Node.js server process, resulting in remote code execution.The fix replaces each
eval()call withparseInt(..., 10), which safely coerces the input to an integer. The existing downstream validations (isNaN, negative checks, sum-cap of 30) continue to work unchanged because they already expect numeric values, preserving all functional behavior while eliminating the injection sink.Changes
app/routes/contributions.js: Replace eval() of user-supplied request body fields with parseInt() to prevent server-side JavaScript injection / authenticated RCE.Test Suggestions
Breaking Changes