Skip to content

Commit 0d23ab0

Browse files
committed
JS: Add data flow modeling for promisified user-defined functions
1 parent 2c6db00 commit 0d23ab0

File tree

4 files changed

+52
-3
lines changed

4 files changed

+52
-3
lines changed

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: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@
8282
| other.js:28:27:28:29 | cmd | other.js:5:25:5:31 | req.url | other.js:28:27:28:29 | cmd | This command line depends on a $@. | other.js:5:25:5:31 | req.url | user-provided value |
8383
| other.js:30:33:30:35 | cmd | other.js:5:25:5:31 | req.url | other.js:30:33:30:35 | cmd | This command line depends on a $@. | other.js:5:25:5:31 | req.url | user-provided value |
8484
| other.js:34:44:34:46 | cmd | other.js:5:25:5:31 | req.url | other.js:34:44:34:46 | cmd | This command line depends on a $@. | other.js:5:25:5:31 | req.url | user-provided value |
85+
| promisification.js:9:13:9:21 | code.code | promisification.js:15:18:15:25 | req.body | promisification.js:9:13:9:21 | code.code | This command line depends on a $@. | promisification.js:15:18:15:25 | req.body | user-provided value |
8586
| 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 |
8687
| promisification.js:31:24:31:27 | code | promisification.js:30:18:30:25 | req.body | promisification.js:31:24:31:27 | code | This command line depends on a $@. | promisification.js:30:18:30:25 | req.body | user-provided value |
8788
| promisification.js:40:21:40:24 | code | promisification.js:37:18:37:25 | req.body | promisification.js:40:21:40:24 | code | This command line depends on a $@. | promisification.js:37:18:37:25 | req.body | user-provided value |
@@ -94,6 +95,7 @@
9495
| promisification.js:77:24:77:26 | cmd | promisification.js:61:15:61:22 | req.body | promisification.js:77:24:77:26 | cmd | This command line depends on a $@. | promisification.js:61:15:61:22 | req.body | user-provided value |
9596
| promisification.js:78:28:78:30 | cmd | promisification.js:61:15:61:22 | req.body | promisification.js:78:28:78:30 | cmd | This command line depends on a $@. | promisification.js:61:15:61:22 | req.body | user-provided value |
9697
| promisification.js:79:25:79:27 | cmd | promisification.js:61:15:61:22 | req.body | promisification.js:79:25:79:27 | cmd | This command line depends on a $@. | promisification.js:61:15:61:22 | req.body | user-provided value |
98+
| promisification.js:83:36:83:39 | code | promisification.js:61:15:61:22 | req.body | promisification.js:83:36:83:39 | code | This command line depends on a $@. | promisification.js:61:15:61:22 | req.body | user-provided value |
9799
| 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 |
98100
| 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 |
99101
| 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 |
@@ -283,6 +285,11 @@ edges
283285
| other.js:5:9:5:11 | cmd | other.js:34:44:34:46 | cmd | provenance | |
284286
| other.js:5:15:5:38 | url.par ... , true) | other.js:5:9:5:11 | cmd | provenance | |
285287
| other.js:5:25:5:31 | req.url | other.js:5:15:5:38 | url.par ... , true) | provenance | |
288+
| promisification.js:8:21:8:24 | code | promisification.js:9:13:9:16 | code | provenance | |
289+
| promisification.js:9:13:9:16 | code | promisification.js:9:13:9:21 | code.code | provenance | |
290+
| promisification.js:15:11:15:14 | code | promisification.js:16:15:16:18 | code | provenance | |
291+
| promisification.js:15:18:15:25 | req.body | promisification.js:15:11:15:14 | code | provenance | |
292+
| promisification.js:16:15:16:18 | code | promisification.js:8:21:8:24 | code | provenance | |
286293
| promisification.js:21:11:21:14 | code | promisification.js:24:22:24:25 | code | provenance | |
287294
| promisification.js:21:18:21:25 | req.body | promisification.js:21:11:21:14 | code | provenance | |
288295
| promisification.js:30:11:30:14 | code | promisification.js:31:24:31:27 | code | provenance | |
@@ -299,7 +306,10 @@ edges
299306
| promisification.js:61:9:61:11 | cmd | promisification.js:77:24:77:26 | cmd | provenance | |
300307
| promisification.js:61:9:61:11 | cmd | promisification.js:78:28:78:30 | cmd | provenance | |
301308
| promisification.js:61:9:61:11 | cmd | promisification.js:79:25:79:27 | cmd | provenance | |
309+
| promisification.js:61:9:61:11 | cmd | promisification.js:89:12:89:14 | cmd | provenance | |
302310
| promisification.js:61:15:61:22 | req.body | promisification.js:61:9:61:11 | cmd | provenance | |
311+
| promisification.js:81:34:81:37 | code | promisification.js:83:36:83:39 | code | provenance | |
312+
| promisification.js:89:12:89:14 | cmd | promisification.js:81:34:81:37 | code | provenance | |
303313
| promisification.js:99:11:99:14 | code | promisification.js:100:23:100:26 | code | provenance | |
304314
| promisification.js:99:11:99:14 | code | promisification.js:101:27:101:30 | code | provenance | |
305315
| promisification.js:99:11:99:14 | code | promisification.js:102:27:102:30 | code | provenance | |
@@ -502,6 +512,12 @@ nodes
502512
| other.js:28:27:28:29 | cmd | semmle.label | cmd |
503513
| other.js:30:33:30:35 | cmd | semmle.label | cmd |
504514
| other.js:34:44:34:46 | cmd | semmle.label | cmd |
515+
| promisification.js:8:21:8:24 | code | semmle.label | code |
516+
| promisification.js:9:13:9:16 | code | semmle.label | code |
517+
| promisification.js:9:13:9:21 | code.code | semmle.label | code.code |
518+
| promisification.js:15:11:15:14 | code | semmle.label | code |
519+
| promisification.js:15:18:15:25 | req.body | semmle.label | req.body |
520+
| promisification.js:16:15:16:18 | code | semmle.label | code |
505521
| promisification.js:21:11:21:14 | code | semmle.label | code |
506522
| promisification.js:21:18:21:25 | req.body | semmle.label | req.body |
507523
| promisification.js:24:22:24:25 | code | semmle.label | code |
@@ -524,6 +540,9 @@ nodes
524540
| promisification.js:77:24:77:26 | cmd | semmle.label | cmd |
525541
| promisification.js:78:28:78:30 | cmd | semmle.label | cmd |
526542
| promisification.js:79:25:79:27 | cmd | semmle.label | cmd |
543+
| promisification.js:81:34:81:37 | code | semmle.label | code |
544+
| promisification.js:83:36:83:39 | code | semmle.label | code |
545+
| promisification.js:89:12:89:14 | cmd | semmle.label | cmd |
527546
| promisification.js:99:11:99:14 | code | semmle.label | code |
528547
| promisification.js:99:18:99:25 | req.body | semmle.label | req.body |
529548
| promisification.js:100:23:100:26 | code | semmle.label | code |

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,13 @@ const app = express();
66
app.use(bodyParser.json());
77

88
function legacyEval(code) {
9-
cp.exec(code.code); // $ MISSING: Alert
9+
cp.exec(code.code); // $ Alert
1010
}
1111

1212
app.post('/eval', async (req, res) => {
1313
const { promisify } = require('util');
1414
const evalAsync = promisify(legacyEval);
15-
const code = req.body; // $ MISSING: Source
15+
const code = req.body; // $ Source
1616
evalAsync(code);
1717
});
1818

@@ -80,7 +80,7 @@ app.post('/eval', async (req, res) => {
8080

8181
const lambda = es6Promisify((code, callback) => {
8282
try {
83-
const result = cp.exec(code); // $ MISSING: Alert
83+
const result = cp.exec(code); // $ Alert
8484
callback(null, result);
8585
} catch (err) {
8686
callback(err);

0 commit comments

Comments
 (0)