fix: blacklist transaction sort fields from query parameters - #1135
Conversation
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 50 minutes. View limit detailsLimit details: You’ve used the included review currently available. Review configuration: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Repeated orderBy query keys are arrays, and string checks such as includes() do not mean the same thing for arrays as they do for strings. Sort fields are now matched explicitly, and unknown values fall back to timestamp. Co-authored-by: David <Klakurka@users.noreply.github.com>
93a36d6 to
fc9b16d
Compare
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The static sort mappings address the type-confusion risk while preserving supported sorting behavior.
Review effort: Balanced
Findings: None
What changed in this PR
Replaces dynamic Prisma transaction sorting with safe, explicit field mappings.
Changes:
- Adds transaction and payment sort-field allowlists with timestamp fallback.
- Handles repeated query parameters safely.
- Adds tests for valid, unknown, and array sort values.
| File | Description |
|---|---|
services/transactionService.ts |
Safely maps transaction and payment sorting fields. |
tests/unittests/transactionService.test.ts |
Tests supported fields and secure fallbacks. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Related to https://github.com/PayButton/paybutton-server/security/code-scanning/3
Related to https://github.com/PayButton/paybutton-server/security/code-scanning/6
Related to https://github.com/PayButton/paybutton-server/security/code-scanning/7
Related to https://github.com/PayButton/paybutton-server/security/code-scanning/8
Description
CodeQL
js/type-confusion-through-parameter-tamperingflaggedorderByon the paybutton and address transaction lists. A repeated query key is an array, andincludes()then checks membership instead of a substring before the value is used as a Prisma sort key.Sort fields are now an explicit blacklist. Unknown values, including arrays, sort by timestamp. Payment sorting uses the same approach for
values,networkId, and the other payment columns.The GitHub token for this run cannot read code-scanning alert bodies, so this is one of the fixes from a local CodeQL code-scanning run of the same default query suite.
Test plan
npx ts-node -O '{"module":"commonjs"}' node_modules/jest/bin/jest.js tests/unittests/transactionService.test.ts --forceExitaddress.networkId,values,networkId) and that__proto__,constructor, and arrayorderByfall back totimestamp.