Skip to content

Commit 4572376

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

File tree

8 files changed

+252
-4
lines changed

8 files changed

+252
-4
lines changed

‎javascript/ql/lib/change-notes/2025-03-28-fs-extra.md

+4
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

+36-4
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

+76
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 |

‎javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/more-fs-extra.js

+33
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)