Skip to content

Commit 97a11de

Browse files
authored
Merge pull request #20435 from Napalys/js/promisification_modeling
JS: Promisification library modeling and enhance flow
2 parents 6d9e489 + 49ccb8c commit 97a11de

File tree

8 files changed

+360
-4
lines changed

8 files changed

+360
-4
lines changed
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* Added modeling for promisification libraries `@gar/promisify`, `es6-promisify`, `util.promisify`, `thenify-all`, `call-me-maybe`, `@google-cloud/promisify`, and `util-promisify`.
5+
* Data flow is now tracked through promisified user-defined functions.
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
extensions:
2+
- addsTo:
3+
pack: codeql/javascript-all
4+
extensible: summaryModel
5+
data:
6+
- ["call-me-maybe", "", "Argument[1]", "ReturnValue", "value"]

javascript/ql/lib/semmle/javascript/ApiGraphs.qll

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1115,6 +1115,17 @@ module API {
11151115
ref = awaited(call)
11161116
)
11171117
or
1118+
// Handle promisified object member access: promisify(obj).member should be treated as obj.member (promisified)
1119+
exists(
1120+
Promisify::PromisifyAllCall promisifiedObj, DataFlow::SourceNode originalObj,
1121+
string member
1122+
|
1123+
originalObj.flowsTo(promisifiedObj.getArgument(0)) and
1124+
use(base, originalObj) and
1125+
lbl = Label::member(member) and
1126+
ref = promisifiedObj.getAPropertyRead(member)
1127+
)
1128+
or
11181129
decoratorDualEdge(base, lbl, ref)
11191130
or
11201131
decoratorUseEdge(base, lbl, ref)

javascript/ql/lib/semmle/javascript/Promises.qll

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -727,8 +727,12 @@ module Promisify {
727727
PromisifyAllCall() {
728728
this =
729729
[
730-
DataFlow::moduleMember("bluebird", "promisifyAll"),
731-
DataFlow::moduleImport(["util-promisifyall", "pify"])
730+
DataFlow::moduleMember(["bluebird", "@google-cloud/promisify", "es6-promisify"],
731+
"promisifyAll"),
732+
DataFlow::moduleMember("thenify-all", "withCallback"),
733+
DataFlow::moduleImport([
734+
"util-promisifyall", "pify", "thenify-all", "@gar/promisify", "util.promisify-all"
735+
])
732736
].getACall()
733737
}
734738
}
@@ -741,11 +745,13 @@ module Promisify {
741745
PromisifyCall() {
742746
this = DataFlow::moduleImport(["util", "bluebird"]).getAMemberCall("promisify")
743747
or
744-
this = DataFlow::moduleImport(["pify", "util.promisify"]).getACall()
748+
this = DataFlow::moduleImport(["pify", "util.promisify", "util-promisify"]).getACall()
745749
or
746-
this = DataFlow::moduleImport("thenify").getACall()
750+
this = DataFlow::moduleImport(["thenify", "@gar/promisify", "es6-promisify"]).getACall()
747751
or
748752
this = DataFlow::moduleMember("thenify", "withCallback").getACall()
753+
or
754+
this = DataFlow::moduleMember("@google-cloud/promisify", "promisify").getACall()
749755
}
750756
}
751757
}

javascript/ql/lib/semmle/javascript/dataflow/DataFlow.qll

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1912,6 +1912,7 @@ module DataFlow {
19121912
deprecated import Configuration
19131913
import TypeTracking
19141914
import AdditionalFlowSteps
1915+
import PromisifyFlow
19151916
import internal.FunctionWrapperSteps
19161917
import internal.sharedlib.DataFlow
19171918
import internal.BarrierGuards
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
/**
2+
* Provides data flow steps for promisified user-defined function calls.
3+
* This ensures that when you call a promisified user-defined function,
4+
* arguments flow to the original function's parameters.
5+
*/
6+
7+
private import javascript
8+
private import semmle.javascript.dataflow.AdditionalFlowSteps
9+
10+
/**
11+
* A data flow step from arguments of promisified user-defined function calls to
12+
* the parameters of the original function.
13+
*/
14+
class PromisifiedUserFunctionArgumentFlow extends AdditionalFlowStep {
15+
override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
16+
exists(
17+
DataFlow::CallNode promisifiedCall, Promisify::PromisifyCall promisify,
18+
DataFlow::FunctionNode originalFunc, int i
19+
|
20+
// The promisified call flows from a promisify result
21+
promisify.flowsTo(promisifiedCall.getCalleeNode()) and
22+
// The original function was promisified
23+
originalFunc.flowsTo(promisify.getArgument(0)) and
24+
// Argument i of the promisified call flows to parameter i of the original function
25+
pred = promisifiedCall.getArgument(i) and
26+
succ = originalFunc.getParameter(i)
27+
)
28+
}
29+
}

javascript/ql/test/query-tests/Security/CWE-078/CommandInjection/CommandInjection.expected

Lines changed: 145 additions & 0 deletions
Large diffs are not rendered by default.
Lines changed: 153 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,153 @@
1+
const express = require('express');
2+
const bodyParser = require('body-parser');
3+
const cp = require('child_process');
4+
5+
const app = express();
6+
app.use(bodyParser.json());
7+
8+
function legacyEval(code) {
9+
cp.exec(code.code); // $ Alert
10+
}
11+
12+
app.post('/eval', async (req, res) => {
13+
const { promisify } = require('util');
14+
const evalAsync = promisify(legacyEval);
15+
const code = req.body; // $ Source
16+
evalAsync(code);
17+
});
18+
19+
app.post('/eval', async (req, res) => {
20+
const directPromisify = require('util.promisify');
21+
const code = req.body; // $ Source
22+
23+
const promisifiedExec3 = directPromisify(cp.exec);
24+
promisifiedExec3(code); // $ Alert
25+
});
26+
27+
app.post('/eval', async (req, res) => {
28+
const promisify2 = require('util.promisify-all');
29+
const promisifiedCp = promisify2(cp);
30+
const code = req.body; // $ Source
31+
promisifiedCp.exec(code); // $ Alert
32+
});
33+
34+
35+
app.post('/eval', async (req, res) => {
36+
var garPromisify = require("@gar/promisify");
37+
const code = req.body; // $ Source
38+
39+
const promisifiedExec = garPromisify(cp.exec);
40+
promisifiedExec(code); // $ Alert
41+
42+
const promisifiedCp = garPromisify(cp);
43+
promisifiedCp.exec(code); // $ Alert
44+
});
45+
46+
app.post('/eval', async (req, res) => {
47+
require('util.promisify/shim')();
48+
const util = require('util');
49+
const code = req.body; // $ Source
50+
51+
const promisifiedExec = util.promisify(cp.exec);
52+
promisifiedExec(code); // $ Alert
53+
54+
const execAsync = util.promisify(cp.exec.bind(cp));
55+
execAsync(code); // $ Alert
56+
});
57+
58+
59+
app.post('/eval', async (req, res) => {
60+
const es6Promisify = require("es6-promisify");
61+
let cmd = req.body; // $ Source
62+
63+
// Test basic promisification
64+
const promisifiedExec = es6Promisify(cp.exec);
65+
promisifiedExec(cmd); // $ Alert
66+
67+
// Test with method binding
68+
const execBoundAsync = es6Promisify(cp.exec.bind(cp));
69+
execBoundAsync(cmd); // $ Alert
70+
71+
const promisifiedExecMulti = es6Promisify(cp.exec, {
72+
multiArgs: true
73+
});
74+
promisifiedExecMulti(cmd); // $ Alert
75+
76+
const promisifiedCp = es6Promisify.promisifyAll(cp);
77+
promisifiedCp.exec(cmd); // $ Alert
78+
promisifiedCp.execFile(cmd); // $ Alert
79+
promisifiedCp.spawn(cmd); // $ Alert
80+
81+
const lambda = es6Promisify((code, callback) => {
82+
try {
83+
const result = cp.exec(code); // $ Alert
84+
callback(null, result);
85+
} catch (err) {
86+
callback(err);
87+
}
88+
});
89+
lambda(cmd);
90+
});
91+
92+
93+
app.post('/eval', async (req, res) => {
94+
var thenifyAll = require('thenify-all');
95+
var cpThenifyAll = thenifyAll(require('child_process'), {}, [
96+
'exec',
97+
'execSync',
98+
]);
99+
const code = req.body; // $ Source
100+
cpThenifyAll.exec(code); // $ Alert
101+
cpThenifyAll.execSync(code); // $ Alert
102+
cpThenifyAll.execFile(code); // $ SPURIOUS: Alert - not promisified, as it is not listed in `thenifyAll`, but it should fine to flag it
103+
104+
105+
var cpThenifyAll1 = thenifyAll.withCallback(require('child_process'), {}, ['exec']);
106+
cpThenifyAll1.exec(code, function (err, string) {}); // $ Alert
107+
108+
var cpThenifyAll2 = thenifyAll(require('child_process'));
109+
cpThenifyAll2.exec(code); // $ Alert
110+
});
111+
112+
app.post('/eval', async (req, res) => {
113+
const maybe = require('call-me-maybe');
114+
const code = req.body; // $ Source
115+
116+
function createExecPromise(cmd) {
117+
return new Promise((resolve) => {
118+
resolve(cmd);
119+
});
120+
}
121+
122+
const cmdPromise = createExecPromise(code);
123+
maybe(null, cmdPromise).then(cmd => {
124+
cp.exec(cmd); // $ Alert
125+
});
126+
});
127+
128+
app.post('/eval', async (req, res) => {
129+
const utilPromisify = require('util-promisify');
130+
const code = req.body; // $ Source
131+
132+
const promisifiedExec = utilPromisify(cp.exec);
133+
promisifiedExec(code); // $ Alert
134+
135+
const execAsync = utilPromisify(cp.exec.bind(cp));
136+
execAsync(code); // $ Alert
137+
});
138+
139+
app.post('/eval', async (req, res) => {
140+
const {promisify, promisifyAll} = require('@google-cloud/promisify');
141+
const code = req.body; // $ Source
142+
143+
const promisifiedExec = promisify(cp.exec);
144+
promisifiedExec(code); // $ Alert
145+
146+
const execAsync = promisify(cp.exec.bind(cp));
147+
execAsync(code); // $ Alert
148+
149+
const promisifiedCp = promisifyAll(cp);
150+
promisifiedCp.exec(code); // $ Alert
151+
promisifiedCp.execFile(code); // $ Alert
152+
promisifiedCp.spawn(code); // $ Alert
153+
});

0 commit comments

Comments
 (0)