-
Notifications
You must be signed in to change notification settings - Fork 1.8k
JS: Promisification library modeling and enhance flow #20435
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
d6a14e6
22b6185
d37425a
312471e
35c75c0
e002f20
2c6db00
0d23ab0
3a75500
278a1ef
49ccb8c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| category: minorAnalysis | ||
| --- | ||
| * Added modeling for promisification libraries `@gar/promisify`, `es6-promisify`, `util.promisify`, `thenify-all`, `call-me-maybe`, `@google-cloud/promisify`, and `util-promisify`. | ||
| * Data flow is now tracked through promisified user-defined functions. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| extensions: | ||
| - addsTo: | ||
| pack: codeql/javascript-all | ||
| extensible: summaryModel | ||
| data: | ||
| - ["call-me-maybe", "", "Argument[1]", "ReturnValue", "value"] |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| /** | ||
| * Provides data flow steps for promisified user-defined function calls. | ||
| * This ensures that when you call a promisified user-defined function, | ||
| * arguments flow to the original function's parameters. | ||
| */ | ||
|
|
||
| private import javascript | ||
| private import semmle.javascript.dataflow.AdditionalFlowSteps | ||
|
|
||
| /** | ||
| * A data flow step from arguments of promisified user-defined function calls to | ||
| * the parameters of the original function. | ||
| */ | ||
| class PromisifiedUserFunctionArgumentFlow extends AdditionalFlowStep { | ||
| override predicate step(DataFlow::Node pred, DataFlow::Node succ) { | ||
| exists( | ||
| DataFlow::CallNode promisifiedCall, Promisify::PromisifyCall promisify, | ||
| DataFlow::FunctionNode originalFunc, int i | ||
| | | ||
| // The promisified call flows from a promisify result | ||
| promisify.flowsTo(promisifiedCall.getCalleeNode()) and | ||
| // The original function was promisified | ||
| originalFunc.flowsTo(promisify.getArgument(0)) and | ||
| // Argument i of the promisified call flows to parameter i of the original function | ||
| pred = promisifiedCall.getArgument(i) and | ||
| succ = originalFunc.getParameter(i) | ||
| ) | ||
| } | ||
| } |
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,153 @@ | ||||||
| const express = require('express'); | ||||||
| const bodyParser = require('body-parser'); | ||||||
| const cp = require('child_process'); | ||||||
|
|
||||||
| const app = express(); | ||||||
| app.use(bodyParser.json()); | ||||||
|
|
||||||
| function legacyEval(code) { | ||||||
| cp.exec(code.code); // $ Alert | ||||||
| } | ||||||
|
|
||||||
| app.post('/eval', async (req, res) => { | ||||||
| const { promisify } = require('util'); | ||||||
| const evalAsync = promisify(legacyEval); | ||||||
| const code = req.body; // $ Source | ||||||
| evalAsync(code); | ||||||
| }); | ||||||
|
|
||||||
| app.post('/eval', async (req, res) => { | ||||||
| const directPromisify = require('util.promisify'); | ||||||
| const code = req.body; // $ Source | ||||||
|
|
||||||
| const promisifiedExec3 = directPromisify(cp.exec); | ||||||
| promisifiedExec3(code); // $ Alert | ||||||
| }); | ||||||
|
|
||||||
| app.post('/eval', async (req, res) => { | ||||||
| const promisify2 = require('util.promisify-all'); | ||||||
| const promisifiedCp = promisify2(cp); | ||||||
| const code = req.body; // $ Source | ||||||
| promisifiedCp.exec(code); // $ Alert | ||||||
| }); | ||||||
|
|
||||||
|
|
||||||
| app.post('/eval', async (req, res) => { | ||||||
| var garPromisify = require("@gar/promisify"); | ||||||
| const code = req.body; // $ Source | ||||||
|
|
||||||
| const promisifiedExec = garPromisify(cp.exec); | ||||||
| promisifiedExec(code); // $ Alert | ||||||
|
|
||||||
| const promisifiedCp = garPromisify(cp); | ||||||
| promisifiedCp.exec(code); // $ Alert | ||||||
| }); | ||||||
|
|
||||||
| app.post('/eval', async (req, res) => { | ||||||
| require('util.promisify/shim')(); | ||||||
| const util = require('util'); | ||||||
| const code = req.body; // $ Source | ||||||
|
|
||||||
| const promisifiedExec = util.promisify(cp.exec); | ||||||
| promisifiedExec(code); // $ Alert | ||||||
|
|
||||||
| const execAsync = util.promisify(cp.exec.bind(cp)); | ||||||
| execAsync(code); // $ Alert | ||||||
| }); | ||||||
|
|
||||||
|
|
||||||
| app.post('/eval', async (req, res) => { | ||||||
| const es6Promisify = require("es6-promisify"); | ||||||
| let cmd = req.body; // $ Source | ||||||
|
|
||||||
| // Test basic promisification | ||||||
| const promisifiedExec = es6Promisify(cp.exec); | ||||||
| promisifiedExec(cmd); // $ Alert | ||||||
|
|
||||||
| // Test with method binding | ||||||
| const execBoundAsync = es6Promisify(cp.exec.bind(cp)); | ||||||
| execBoundAsync(cmd); // $ Alert | ||||||
|
|
||||||
| const promisifiedExecMulti = es6Promisify(cp.exec, { | ||||||
| multiArgs: true | ||||||
| }); | ||||||
| promisifiedExecMulti(cmd); // $ Alert | ||||||
|
|
||||||
| const promisifiedCp = es6Promisify.promisifyAll(cp); | ||||||
| promisifiedCp.exec(cmd); // $ Alert | ||||||
| promisifiedCp.execFile(cmd); // $ Alert | ||||||
| promisifiedCp.spawn(cmd); // $ Alert | ||||||
|
|
||||||
| const lambda = es6Promisify((code, callback) => { | ||||||
| try { | ||||||
| const result = cp.exec(code); // $ Alert | ||||||
| callback(null, result); | ||||||
| } catch (err) { | ||||||
| callback(err); | ||||||
| } | ||||||
| }); | ||||||
| lambda(cmd); | ||||||
| }); | ||||||
|
|
||||||
|
|
||||||
| app.post('/eval', async (req, res) => { | ||||||
| var thenifyAll = require('thenify-all'); | ||||||
| var cpThenifyAll = thenifyAll(require('child_process'), {}, [ | ||||||
| 'exec', | ||||||
| 'execSync', | ||||||
| ]); | ||||||
| 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 | ||||||
|
||||||
| 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 |
Copilot
AI
Sep 16, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is unnecessary trailing whitespace on line 115. This should be removed for consistency with the rest of the codebase.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[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.