JS: Promisification library modeling and enhance flow#20435
Merged
Napalys merged 11 commits intogithub:mainfrom Sep 16, 2025
Merged
JS: Promisification library modeling and enhance flow#20435Napalys merged 11 commits intogithub:mainfrom
Napalys merged 11 commits intogithub:mainfrom
Conversation
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the JavaScript analysis by adding comprehensive modeling for promisification libraries and improving data flow through promisified functions.
- Added support for seven new promisification libraries including
@gar/promisify,es6-promisify,util.promisify,thenify-all,call-me-maybe,@google-cloud/promisify, andutil-promisify - Enhanced data flow tracking to handle promisified user-defined functions
- Added API graph support for promisified object member access
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
javascript/ql/test/query-tests/Security/CWE-078/CommandInjection/promisification.js |
Test file demonstrating command injection vulnerabilities through various promisification libraries |
javascript/ql/test/query-tests/Security/CWE-078/CommandInjection/CommandInjection.expected |
Expected test results for the new promisification test cases |
javascript/ql/lib/semmle/javascript/dataflow/PromisifyFlow.qll |
New module providing data flow steps for promisified user-defined function calls |
javascript/ql/lib/semmle/javascript/dataflow/DataFlow.qll |
Imports the new PromisifyFlow module |
javascript/ql/lib/semmle/javascript/Promises.qll |
Extended PromisifyCall and PromisifyAllCall classes to support new libraries |
javascript/ql/lib/semmle/javascript/ApiGraphs.qll |
Added handling for promisified object member access in API graphs |
javascript/ql/lib/ext/call-me-maybe.model.yml |
New model file for the call-me-maybe library |
javascript/ql/lib/change-notes/2025-09-15-promisifications.md |
Release notes documenting the new promisification features |
Comment on lines
+28
to
+29
| const promisify2 = require('util.promisify-all'); | ||
| const promisifiedCp = promisify2(cp); |
There was a problem hiding this comment.
[nitpick] The variable name 'promisify2' is inconsistent with the naming pattern used elsewhere in the file. Consider using 'promisifyAll' to match the library's functionality and improve clarity.
Suggested change
| const promisify2 = require('util.promisify-all'); | |
| const promisifiedCp = promisify2(cp); | |
| const promisifyAll = require('util.promisify-all'); | |
| const promisifiedCp = promisifyAll(cp); |
| const code = req.body; // $ Source | ||
| cpThenifyAll.exec(code); // $ Alert | ||
| cpThenifyAll.execSync(code); // $ Alert | ||
| cpThenifyAll.execFile(code); // $ SPURIOUS: Alert - not promisified, as it is not listed in `thenifyAll`, but it should fine to flag it |
There was a problem hiding this comment.
The comment contains a grammatical error. 'but it should fine to flag it' should be 'but it should be fine to flag it'.
Suggested change
| cpThenifyAll.execFile(code); // $ SPURIOUS: Alert - not promisified, as it is not listed in `thenifyAll`, but it should fine to flag it | |
| cpThenifyAll.execFile(code); // $ SPURIOUS: Alert - not promisified, as it is not listed in `thenifyAll`, but it should be fine to flag it |
| app.post('/eval', async (req, res) => { | ||
| const maybe = require('call-me-maybe'); | ||
| const code = req.body; // $ Source | ||
|
|
There was a problem hiding this comment.
There is unnecessary trailing whitespace on line 115. This should be removed for consistency with the rest of the codebase.
Suggested change
asgerf
reviewed
Sep 16, 2025
Contributor
asgerf
left a comment
asgerf
left a comment
There was a problem hiding this comment.
One minor comments otherwise looks good to merge
| exists( | ||
| DataFlow::SourceNode promisifiedObj, DataFlow::SourceNode originalObj, string member | ||
| | | ||
| promisifiedObj instanceof Promisify::PromisifyAllCall and |
Contributor
There was a problem hiding this comment.
Why not just set the type of promisifiedObj to Promisify::PromifiyAllCall?
…d of `DataFlow::SourceNode`
asgerf
approved these changes
Sep 16, 2025
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.
Added modeling for the following promisification related packages: