Skip to content

Commit d37425a

Browse files
committed
JS: Treat promisify(obj).member as obj.member
1 parent 22b6185 commit d37425a

File tree

3 files changed

+35
-6
lines changed

3 files changed

+35
-6
lines changed

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+
DataFlow::SourceNode promisifiedObj, DataFlow::SourceNode originalObj, string member
1121+
|
1122+
promisifiedObj instanceof Promisify::PromisifyAllCall and
1123+
originalObj.flowsTo(promisifiedObj.(Promisify::PromisifyAllCall).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/test/query-tests/Security/CWE-078/CommandInjection/CommandInjection.expected

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,11 @@
8585
| promisification.js:24:22:24:25 | code | promisification.js:21:18:21:25 | req.body | promisification.js:24:22:24:25 | code | This command line depends on a $@. | promisification.js:21:18:21:25 | req.body | user-provided value |
8686
| promisification.js:52:21:52:24 | code | promisification.js:49:18:49:25 | req.body | promisification.js:52:21:52:24 | code | This command line depends on a $@. | promisification.js:49:18:49:25 | req.body | user-provided value |
8787
| promisification.js:55:15:55:18 | code | promisification.js:49:18:49:25 | req.body | promisification.js:55:15:55:18 | code | This command line depends on a $@. | promisification.js:49:18:49:25 | req.body | user-provided value |
88+
| promisification.js:100:23:100:26 | code | promisification.js:99:18:99:25 | req.body | promisification.js:100:23:100:26 | code | This command line depends on a $@. | promisification.js:99:18:99:25 | req.body | user-provided value |
89+
| promisification.js:101:27:101:30 | code | promisification.js:99:18:99:25 | req.body | promisification.js:101:27:101:30 | code | This command line depends on a $@. | promisification.js:99:18:99:25 | req.body | user-provided value |
90+
| promisification.js:102:27:102:30 | code | promisification.js:99:18:99:25 | req.body | promisification.js:102:27:102:30 | code | This command line depends on a $@. | promisification.js:99:18:99:25 | req.body | user-provided value |
91+
| promisification.js:106:24:106:27 | code | promisification.js:99:18:99:25 | req.body | promisification.js:106:24:106:27 | code | This command line depends on a $@. | promisification.js:99:18:99:25 | req.body | user-provided value |
92+
| promisification.js:109:24:109:27 | code | promisification.js:99:18:99:25 | req.body | promisification.js:109:24:109:27 | code | This command line depends on a $@. | promisification.js:99:18:99:25 | req.body | user-provided value |
8893
| third-party-command-injection.js:6:21:6:27 | command | third-party-command-injection.js:5:20:5:26 | command | third-party-command-injection.js:6:21:6:27 | command | This command line depends on a $@. | third-party-command-injection.js:5:20:5:26 | command | user-provided value |
8994
edges
9095
| actions.js:8:9:8:13 | title | actions.js:9:16:9:20 | title | provenance | |
@@ -267,6 +272,12 @@ edges
267272
| promisification.js:49:11:49:14 | code | promisification.js:52:21:52:24 | code | provenance | |
268273
| promisification.js:49:11:49:14 | code | promisification.js:55:15:55:18 | code | provenance | |
269274
| promisification.js:49:18:49:25 | req.body | promisification.js:49:11:49:14 | code | provenance | |
275+
| promisification.js:99:11:99:14 | code | promisification.js:100:23:100:26 | code | provenance | |
276+
| promisification.js:99:11:99:14 | code | promisification.js:101:27:101:30 | code | provenance | |
277+
| promisification.js:99:11:99:14 | code | promisification.js:102:27:102:30 | code | provenance | |
278+
| promisification.js:99:11:99:14 | code | promisification.js:106:24:106:27 | code | provenance | |
279+
| promisification.js:99:11:99:14 | code | promisification.js:109:24:109:27 | code | provenance | |
280+
| promisification.js:99:18:99:25 | req.body | promisification.js:99:11:99:14 | code | provenance | |
270281
| third-party-command-injection.js:5:20:5:26 | command | third-party-command-injection.js:6:21:6:27 | command | provenance | |
271282
nodes
272283
| actions.js:8:9:8:13 | title | semmle.label | title |
@@ -461,6 +472,13 @@ nodes
461472
| promisification.js:49:18:49:25 | req.body | semmle.label | req.body |
462473
| promisification.js:52:21:52:24 | code | semmle.label | code |
463474
| promisification.js:55:15:55:18 | code | semmle.label | code |
475+
| promisification.js:99:11:99:14 | code | semmle.label | code |
476+
| promisification.js:99:18:99:25 | req.body | semmle.label | req.body |
477+
| promisification.js:100:23:100:26 | code | semmle.label | code |
478+
| promisification.js:101:27:101:30 | code | semmle.label | code |
479+
| promisification.js:102:27:102:30 | code | semmle.label | code |
480+
| promisification.js:106:24:106:27 | code | semmle.label | code |
481+
| promisification.js:109:24:109:27 | code | semmle.label | code |
464482
| third-party-command-injection.js:5:20:5:26 | command | semmle.label | command |
465483
| third-party-command-injection.js:6:21:6:27 | command | semmle.label | command |
466484
subpaths

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -96,17 +96,17 @@ app.post('/eval', async (req, res) => {
9696
'exec',
9797
'execSync',
9898
]);
99-
const code = req.body; // $ MISSING: Source
100-
cpThenifyAll.exec(code); // $ MISSING: Alert
101-
cpThenifyAll.execSync(code); // $ MISSING: Alert
102-
cpThenifyAll.execFile(code); // $ MISSING: Alert - not promisified, as it is not listed in `thenifyAll`
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
103103

104104

105105
var cpThenifyAll1 = thenifyAll.withCallback(require('child_process'), {}, ['exec']);
106-
cpThenifyAll1.exec(code, function (err, string) {}); // $ MISSING: Alert
106+
cpThenifyAll1.exec(code, function (err, string) {}); // $ Alert
107107

108108
var cpThenifyAll2 = thenifyAll(require('child_process'));
109-
cpThenifyAll2.exec(code); // $ MISSING: Alert
109+
cpThenifyAll2.exec(code); // $ Alert
110110
});
111111

112112
app.post('/eval', async (req, res) => {

0 commit comments

Comments
 (0)