JS: Add result.download to Express as Path Traversal Sink#18749
JS: Add result.download to Express as Path Traversal Sink#18749asgerf merged 3 commits intogithub:mainfrom
Conversation
There was a problem hiding this comment.
PR Overview
This pull request adds new test cases to ensure that Express’s res.download method is correctly analyzed as a path traversal sink. It enhances security tests by covering various file download invocations that combine un-sanitized input with path concatenation.
- Added test cases in tainted-sendFile.js to simulate unsafe usage scenarios of res.download.
- Updated change notes to document the new result.download() support for security analysis.
Changes
| File | Description |
|---|---|
| javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/tainted-sendFile.js | Added several test cases for res.download to highlight path traversal vulnerabilities. |
| javascript/ql/lib/change-notes/2025-02-12-express-download.md | Added a change note entry describing the new addition of result.download support. |
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (2)
javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/tainted-sendFile.js:30
- [nitpick] The code uses 'req.param' to access a parameter, whereas other cases use 'req.params'. For clarity and consistency, consider standardizing on one approach (e.g., 'req.params' or 'req.query') depending on your intended parameter source.
res.download(req.param("gimme"));
javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/tainted-sendFile.js:38
- [nitpick] The use of 'req.param' in this invocation differs from the use of 'req.params' elsewhere in the file. Standardizing the parameter access method will improve code readability and reduce potential confusion about where the parameters originate from.
res.download(req.param("file"), { root: req.param("dir") });
Tip: Copilot code review supports C#, Go, Java, JavaScript, Markdown, Python, Ruby and TypeScript, with more languages coming soon. Learn more
asgerf
left a comment
There was a problem hiding this comment.
Thanks, looks great! I'd just like to formulate the change note in a way that makes more sense to end users.
Co-authored-by: Asger F <asgerf@github.com>
Add result.download to Express as Path Traversal Sink