Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 4572376

Browse filesBrowse files
authored
Merge pull request #19143 from Napalys/js/fs-extra-missing
JS: Modeling of `fs-extra` functions
2 parents de8a328 + 75b4d1b commit 4572376
Copy full SHA for 4572376

File tree

8 files changed

+252
-4
lines changed
Filter options

8 files changed

+252
-4
lines changed
+4Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* Added support for additional `fs-extra` methods as sinks in path-injection queries.

‎javascript/ql/lib/semmle/javascript/frameworks/NodeJSLib.qll

Copy file name to clipboardExpand all lines: javascript/ql/lib/semmle/javascript/frameworks/NodeJSLib.qll
+36-4Lines changed: 36 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -434,7 +434,7 @@ module NodeJSLib {
434434
* method might represent a file path.
435435
*/
436436
private predicate fsExtraExtensionFileParam(string methodName, int i) {
437-
methodName = ["copy", "copySync", "copyFile"] and i = [0, 1]
437+
methodName = ["copy", "copySync", "copyFile", "cp", "copyFileSync", "cpSync"] and i = [0, 1]
438438
or
439439
methodName = ["move", "moveSync"] and i = [0, 1]
440440
or
@@ -450,10 +450,13 @@ module NodeJSLib {
450450
or
451451
methodName = ["readJson", "readJSON", "readJsonSync", "readJSONSync"] and i = 0
452452
or
453-
methodName = ["remove", "removeSync"] and i = 0
453+
methodName = ["remove", "removeSync", "rmSync", "rm", "rmdir", "rmdirSync"] and i = 0
454454
or
455455
methodName =
456-
["outputJSON", "outputJson", "writeJSON", "writeJson", "writeJSONSync", "writeJsonSync"] and
456+
[
457+
"outputJSON", "outputJson", "writeJSON", "writeJson", "writeJSONSync", "writeJsonSync",
458+
"outputJSONSync", "outputJsonSync"
459+
] and
457460
i = 0
458461
or
459462
methodName = ["ensureFile", "ensureFileSync"] and i = 0
@@ -462,9 +465,15 @@ module NodeJSLib {
462465
or
463466
methodName = ["ensureSymlink", "ensureSymlinkSync"] and i = [0, 1]
464467
or
465-
methodName = ["emptyDir", "emptyDirSync"] and i = 0
468+
methodName = ["emptyDir", "emptyDirSync", "emptydir", "emptydirSync"] and i = 0
466469
or
467470
methodName = ["pathExists", "pathExistsSync"] and i = 0
471+
or
472+
methodName = ["lutimes", "lutimesSync"] and i = 0
473+
or
474+
methodName =
475+
["opendir", "opendirSync", "openAsBlob", "statfs", "statfsSync", "open", "openSync"] and
476+
i = 0
468477
}
469478

470479
/**
@@ -592,6 +601,13 @@ module NodeJSLib {
592601
}
593602
}
594603

604+
/** A vectored write to the file system using `writev` or `writevSync` methods. */
605+
private class NodeJSFileSystemVectorWrite extends FileSystemWriteAccess, NodeJSFileSystemAccess {
606+
NodeJSFileSystemVectorWrite() { methodName = ["writev", "writevSync"] }
607+
608+
override DataFlow::Node getADataNode() { result = this.getArgument(1) }
609+
}
610+
595611
/** A file system read. */
596612
private class NodeJSFileSystemAccessRead extends FileSystemReadAccess, NodeJSFileSystemAccess {
597613
NodeJSFileSystemAccessRead() { methodName = ["read", "readSync", "readFile", "readFileSync"] }
@@ -619,6 +635,22 @@ module NodeJSLib {
619635
}
620636
}
621637

638+
/** A vectored read to the file system. */
639+
private class NodeJSFileSystemAccessVectorRead extends FileSystemReadAccess,
640+
NodeJSFileSystemAccess
641+
{
642+
NodeJSFileSystemAccessVectorRead() { methodName = ["readv", "readvSync"] }
643+
644+
override DataFlow::Node getADataNode() {
645+
result = this.getArgument(1)
646+
or
647+
exists(DataFlow::ArrayCreationNode array |
648+
array.flowsTo(this.getArgument(1)) and
649+
result = array.getAnElement()
650+
)
651+
}
652+
}
653+
622654
/**
623655
* A write to the file system, using a stream.
624656
*/

‎javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/TaintedPath.expected

Copy file name to clipboardExpand all lines: javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/TaintedPath.expected
+76Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,29 @@
5252
| handlebars.js:11:32:11:39 | filePath | handlebars.js:29:46:29:60 | req.params.path | handlebars.js:11:32:11:39 | filePath | This path depends on a $@. | handlebars.js:29:46:29:60 | req.params.path | user-provided value |
5353
| handlebars.js:15:25:15:32 | filePath | handlebars.js:43:15:43:29 | req.params.path | handlebars.js:15:25:15:32 | filePath | This path depends on a $@. | handlebars.js:43:15:43:29 | req.params.path | user-provided value |
5454
| hapi.js:15:44:15:51 | filepath | hapi.js:14:30:14:51 | request ... ilepath | hapi.js:15:44:15:51 | filepath | This path depends on a $@. | hapi.js:14:30:14:51 | request ... ilepath | user-provided value |
55+
| more-fs-extra.js:10:15:10:22 | filename | more-fs-extra.js:8:26:8:33 | req.body | more-fs-extra.js:10:15:10:22 | filename | This path depends on a $@. | more-fs-extra.js:8:26:8:33 | req.body | user-provided value |
56+
| more-fs-extra.js:11:11:11:18 | filename | more-fs-extra.js:8:26:8:33 | req.body | more-fs-extra.js:11:11:11:18 | filename | This path depends on a $@. | more-fs-extra.js:8:26:8:33 | req.body | user-provided value |
57+
| more-fs-extra.js:12:14:12:21 | filename | more-fs-extra.js:8:26:8:33 | req.body | more-fs-extra.js:12:14:12:21 | filename | This path depends on a $@. | more-fs-extra.js:8:26:8:33 | req.body | user-provided value |
58+
| more-fs-extra.js:13:18:13:25 | filename | more-fs-extra.js:8:26:8:33 | req.body | more-fs-extra.js:13:18:13:25 | filename | This path depends on a $@. | more-fs-extra.js:8:26:8:33 | req.body | user-provided value |
59+
| more-fs-extra.js:14:11:14:18 | filename | more-fs-extra.js:8:26:8:33 | req.body | more-fs-extra.js:14:11:14:18 | filename | This path depends on a $@. | more-fs-extra.js:8:26:8:33 | req.body | user-provided value |
60+
| more-fs-extra.js:15:21:15:28 | filename | more-fs-extra.js:8:26:8:33 | req.body | more-fs-extra.js:15:21:15:28 | filename | This path depends on a $@. | more-fs-extra.js:8:26:8:33 | req.body | user-provided value |
61+
| more-fs-extra.js:16:21:16:28 | filename | more-fs-extra.js:8:26:8:33 | req.body | more-fs-extra.js:16:21:16:28 | filename | This path depends on a $@. | more-fs-extra.js:8:26:8:33 | req.body | user-provided value |
62+
| more-fs-extra.js:17:31:17:38 | filename | more-fs-extra.js:8:26:8:33 | req.body | more-fs-extra.js:17:31:17:38 | filename | This path depends on a $@. | more-fs-extra.js:8:26:8:33 | req.body | user-provided value |
63+
| more-fs-extra.js:18:15:18:22 | filename | more-fs-extra.js:8:26:8:33 | req.body | more-fs-extra.js:18:15:18:22 | filename | This path depends on a $@. | more-fs-extra.js:8:26:8:33 | req.body | user-provided value |
64+
| more-fs-extra.js:19:25:19:32 | filename | more-fs-extra.js:8:26:8:33 | req.body | more-fs-extra.js:19:25:19:32 | filename | This path depends on a $@. | more-fs-extra.js:8:26:8:33 | req.body | user-provided value |
65+
| more-fs-extra.js:20:21:20:28 | filename | more-fs-extra.js:8:26:8:33 | req.body | more-fs-extra.js:20:21:20:28 | filename | This path depends on a $@. | more-fs-extra.js:8:26:8:33 | req.body | user-provided value |
66+
| more-fs-extra.js:21:17:21:24 | filename | more-fs-extra.js:8:26:8:33 | req.body | more-fs-extra.js:21:17:21:24 | filename | This path depends on a $@. | more-fs-extra.js:8:26:8:33 | req.body | user-provided value |
67+
| more-fs-extra.js:22:16:22:23 | filename | more-fs-extra.js:8:26:8:33 | req.body | more-fs-extra.js:22:16:22:23 | filename | This path depends on a $@. | more-fs-extra.js:8:26:8:33 | req.body | user-provided value |
68+
| more-fs-extra.js:23:20:23:27 | filename | more-fs-extra.js:8:26:8:33 | req.body | more-fs-extra.js:23:20:23:27 | filename | This path depends on a $@. | more-fs-extra.js:8:26:8:33 | req.body | user-provided value |
69+
| more-fs-extra.js:24:19:24:26 | filename | more-fs-extra.js:8:26:8:33 | req.body | more-fs-extra.js:24:19:24:26 | filename | This path depends on a $@. | more-fs-extra.js:8:26:8:33 | req.body | user-provided value |
70+
| more-fs-extra.js:25:15:25:22 | filename | more-fs-extra.js:8:26:8:33 | req.body | more-fs-extra.js:25:15:25:22 | filename | This path depends on a $@. | more-fs-extra.js:8:26:8:33 | req.body | user-provided value |
71+
| more-fs-extra.js:26:19:26:26 | filename | more-fs-extra.js:8:26:8:33 | req.body | more-fs-extra.js:26:19:26:26 | filename | This path depends on a $@. | more-fs-extra.js:8:26:8:33 | req.body | user-provided value |
72+
| more-fs-extra.js:27:13:27:20 | filename | more-fs-extra.js:8:26:8:33 | req.body | more-fs-extra.js:27:13:27:20 | filename | This path depends on a $@. | more-fs-extra.js:8:26:8:33 | req.body | user-provided value |
73+
| more-fs-extra.js:28:17:28:24 | filename | more-fs-extra.js:8:26:8:33 | req.body | more-fs-extra.js:28:17:28:24 | filename | This path depends on a $@. | more-fs-extra.js:8:26:8:33 | req.body | user-provided value |
74+
| more-fs-extra.js:29:23:29:30 | filename | more-fs-extra.js:8:26:8:33 | req.body | more-fs-extra.js:29:23:29:30 | filename | This path depends on a $@. | more-fs-extra.js:8:26:8:33 | req.body | user-provided value |
75+
| more-fs-extra.js:30:16:30:23 | filename | more-fs-extra.js:8:26:8:33 | req.body | more-fs-extra.js:30:16:30:23 | filename | This path depends on a $@. | more-fs-extra.js:8:26:8:33 | req.body | user-provided value |
76+
| more-fs-extra.js:31:20:31:27 | filename | more-fs-extra.js:8:26:8:33 | req.body | more-fs-extra.js:31:20:31:27 | filename | This path depends on a $@. | more-fs-extra.js:8:26:8:33 | req.body | user-provided value |
77+
| more-fs-extra.js:32:23:32:30 | filename | more-fs-extra.js:8:26:8:33 | req.body | more-fs-extra.js:32:23:32:30 | filename | This path depends on a $@. | more-fs-extra.js:8:26:8:33 | req.body | user-provided value |
5578
| normalizedPaths.js:13:19:13:22 | path | normalizedPaths.js:11:14:11:27 | req.query.path | normalizedPaths.js:13:19:13:22 | path | This path depends on a $@. | normalizedPaths.js:11:14:11:27 | req.query.path | user-provided value |
5679
| normalizedPaths.js:14:19:14:29 | './' + path | normalizedPaths.js:11:14:11:27 | req.query.path | normalizedPaths.js:14:19:14:29 | './' + path | This path depends on a $@. | normalizedPaths.js:11:14:11:27 | req.query.path | user-provided value |
5780
| normalizedPaths.js:15:19:15:38 | path + '/index.html' | normalizedPaths.js:11:14:11:27 | req.query.path | normalizedPaths.js:15:19:15:38 | path + '/index.html' | This path depends on a $@. | normalizedPaths.js:11:14:11:27 | req.query.path | user-provided value |
@@ -347,6 +370,32 @@ edges
347370
| handlebars.js:43:15:43:29 | req.params.path | handlebars.js:13:73:13:80 | filePath | provenance | |
348371
| hapi.js:14:19:14:51 | filepath | hapi.js:15:44:15:51 | filepath | provenance | |
349372
| hapi.js:14:30:14:51 | request ... ilepath | hapi.js:14:19:14:51 | filepath | provenance | |
373+
| more-fs-extra.js:8:11:8:22 | { filename } | more-fs-extra.js:8:13:8:20 | filename | provenance | Config |
374+
| more-fs-extra.js:8:11:8:33 | filename | more-fs-extra.js:10:15:10:22 | filename | provenance | |
375+
| more-fs-extra.js:8:11:8:33 | filename | more-fs-extra.js:11:11:11:18 | filename | provenance | |
376+
| more-fs-extra.js:8:11:8:33 | filename | more-fs-extra.js:12:14:12:21 | filename | provenance | |
377+
| more-fs-extra.js:8:11:8:33 | filename | more-fs-extra.js:13:18:13:25 | filename | provenance | |
378+
| more-fs-extra.js:8:11:8:33 | filename | more-fs-extra.js:14:11:14:18 | filename | provenance | |
379+
| more-fs-extra.js:8:11:8:33 | filename | more-fs-extra.js:15:21:15:28 | filename | provenance | |
380+
| more-fs-extra.js:8:11:8:33 | filename | more-fs-extra.js:16:21:16:28 | filename | provenance | |
381+
| more-fs-extra.js:8:11:8:33 | filename | more-fs-extra.js:17:31:17:38 | filename | provenance | |
382+
| more-fs-extra.js:8:11:8:33 | filename | more-fs-extra.js:18:15:18:22 | filename | provenance | |
383+
| more-fs-extra.js:8:11:8:33 | filename | more-fs-extra.js:19:25:19:32 | filename | provenance | |
384+
| more-fs-extra.js:8:11:8:33 | filename | more-fs-extra.js:20:21:20:28 | filename | provenance | |
385+
| more-fs-extra.js:8:11:8:33 | filename | more-fs-extra.js:21:17:21:24 | filename | provenance | |
386+
| more-fs-extra.js:8:11:8:33 | filename | more-fs-extra.js:22:16:22:23 | filename | provenance | |
387+
| more-fs-extra.js:8:11:8:33 | filename | more-fs-extra.js:23:20:23:27 | filename | provenance | |
388+
| more-fs-extra.js:8:11:8:33 | filename | more-fs-extra.js:24:19:24:26 | filename | provenance | |
389+
| more-fs-extra.js:8:11:8:33 | filename | more-fs-extra.js:25:15:25:22 | filename | provenance | |
390+
| more-fs-extra.js:8:11:8:33 | filename | more-fs-extra.js:26:19:26:26 | filename | provenance | |
391+
| more-fs-extra.js:8:11:8:33 | filename | more-fs-extra.js:27:13:27:20 | filename | provenance | |
392+
| more-fs-extra.js:8:11:8:33 | filename | more-fs-extra.js:28:17:28:24 | filename | provenance | |
393+
| more-fs-extra.js:8:11:8:33 | filename | more-fs-extra.js:29:23:29:30 | filename | provenance | |
394+
| more-fs-extra.js:8:11:8:33 | filename | more-fs-extra.js:30:16:30:23 | filename | provenance | |
395+
| more-fs-extra.js:8:11:8:33 | filename | more-fs-extra.js:31:20:31:27 | filename | provenance | |
396+
| more-fs-extra.js:8:11:8:33 | filename | more-fs-extra.js:32:23:32:30 | filename | provenance | |
397+
| more-fs-extra.js:8:13:8:20 | filename | more-fs-extra.js:8:11:8:33 | filename | provenance | |
398+
| more-fs-extra.js:8:26:8:33 | req.body | more-fs-extra.js:8:11:8:22 | { filename } | provenance | |
350399
| normalizedPaths.js:11:7:11:27 | path | normalizedPaths.js:13:19:13:22 | path | provenance | |
351400
| normalizedPaths.js:11:7:11:27 | path | normalizedPaths.js:14:26:14:29 | path | provenance | |
352401
| normalizedPaths.js:11:7:11:27 | path | normalizedPaths.js:15:19:15:22 | path | provenance | |
@@ -827,6 +876,33 @@ nodes
827876
| hapi.js:14:19:14:51 | filepath | semmle.label | filepath |
828877
| hapi.js:14:30:14:51 | request ... ilepath | semmle.label | request ... ilepath |
829878
| hapi.js:15:44:15:51 | filepath | semmle.label | filepath |
879+
| more-fs-extra.js:8:11:8:22 | { filename } | semmle.label | { filename } |
880+
| more-fs-extra.js:8:11:8:33 | filename | semmle.label | filename |
881+
| more-fs-extra.js:8:13:8:20 | filename | semmle.label | filename |
882+
| more-fs-extra.js:8:26:8:33 | req.body | semmle.label | req.body |
883+
| more-fs-extra.js:10:15:10:22 | filename | semmle.label | filename |
884+
| more-fs-extra.js:11:11:11:18 | filename | semmle.label | filename |
885+
| more-fs-extra.js:12:14:12:21 | filename | semmle.label | filename |
886+
| more-fs-extra.js:13:18:13:25 | filename | semmle.label | filename |
887+
| more-fs-extra.js:14:11:14:18 | filename | semmle.label | filename |
888+
| more-fs-extra.js:15:21:15:28 | filename | semmle.label | filename |
889+
| more-fs-extra.js:16:21:16:28 | filename | semmle.label | filename |
890+
| more-fs-extra.js:17:31:17:38 | filename | semmle.label | filename |
891+
| more-fs-extra.js:18:15:18:22 | filename | semmle.label | filename |
892+
| more-fs-extra.js:19:25:19:32 | filename | semmle.label | filename |
893+
| more-fs-extra.js:20:21:20:28 | filename | semmle.label | filename |
894+
| more-fs-extra.js:21:17:21:24 | filename | semmle.label | filename |
895+
| more-fs-extra.js:22:16:22:23 | filename | semmle.label | filename |
896+
| more-fs-extra.js:23:20:23:27 | filename | semmle.label | filename |
897+
| more-fs-extra.js:24:19:24:26 | filename | semmle.label | filename |
898+
| more-fs-extra.js:25:15:25:22 | filename | semmle.label | filename |
899+
| more-fs-extra.js:26:19:26:26 | filename | semmle.label | filename |
900+
| more-fs-extra.js:27:13:27:20 | filename | semmle.label | filename |
901+
| more-fs-extra.js:28:17:28:24 | filename | semmle.label | filename |
902+
| more-fs-extra.js:29:23:29:30 | filename | semmle.label | filename |
903+
| more-fs-extra.js:30:16:30:23 | filename | semmle.label | filename |
904+
| more-fs-extra.js:31:20:31:27 | filename | semmle.label | filename |
905+
| more-fs-extra.js:32:23:32:30 | filename | semmle.label | filename |
830906
| normalizedPaths.js:11:7:11:27 | path | semmle.label | path |
831907
| normalizedPaths.js:11:14:11:27 | req.query.path | semmle.label | req.query.path |
832908
| normalizedPaths.js:13:19:13:22 | path | semmle.label | path |
+33Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
const express = require('express');
2+
const fs = require('fs-extra');
3+
const app = express();
4+
5+
app.use(express.json());
6+
7+
app.post('/rmsync', (req, res) => {
8+
const { filename } = req.body; // $ Source
9+
10+
fs.rmSync(filename); // $ Alert
11+
fs.rm(filename); // $ Alert
12+
fs.rmdir(filename); // $ Alert
13+
fs.rmdirSync(filename); // $ Alert
14+
fs.cp(filename, "destination"); // $ Alert
15+
fs.cp("source", filename); // $ Alert
16+
fs.copyFileSync(filename, "destination"); // $ Alert
17+
fs.copyFileSync("source", filename); // $ Alert
18+
fs.cpSync(filename, "destination"); // $ Alert
19+
fs.cpSync("source", filename); // $ Alert
20+
fs.emptydirSync(filename); // $ Alert
21+
fs.emptydir(filename); // $ Alert
22+
fs.opendir(filename); // $ Alert
23+
fs.opendirSync(filename); // $ Alert
24+
fs.openAsBlob(filename); // $ Alert
25+
fs.statfs(filename); // $ Alert
26+
fs.statfsSync(filename); // $ Alert
27+
fs.open(filename, 'r'); // $ Alert
28+
fs.openSync(filename, 'r'); // $ Alert
29+
fs.outputJSONSync(filename, req.body.data, { spaces: 2 }); // $ Alert
30+
fs.lutimes(filename, new Date(req.body.atime), new Date(req.body.mtime)); // $ Alert
31+
fs.lutimesSync(filename, new Date(req.body.atime), new Date(req.body.mtime)); // $ Alert
32+
fs.outputJsonSync(filename, { timestamp: new Date().toISOString(), action: req.body.action, user: req.body.user}, { spaces: 2 }); // $ Alert
33+
});

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.