From 14999c19dacc263ec37f0d0bad1124a6ffa33dd8 Mon Sep 17 00:00:00 2001 From: Napalys Date: Wed, 2 Apr 2025 10:21:17 +0200 Subject: [PATCH 1/3] Added test cases for `rimraf` library. --- .../Security/CWE-022/TaintedPath/rimraf.js | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/rimraf.js diff --git a/javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/rimraf.js b/javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/rimraf.js new file mode 100644 index 000000000000..a6f3c52068ac --- /dev/null +++ b/javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/rimraf.js @@ -0,0 +1,30 @@ +const express = require('express'); +const rimraf = require('rimraf'); + +const app = express(); +app.use(express.json()); + +app.post('/rmsync', async (req, res) => { + const { path } = req.body; // $ MISSING: Source + + rimraf.sync(path); // $ MISSING: Alert + rimraf.rimrafSync(path); // $ MISSING: Alert + rimraf.native(path); // $ MISSING: Alert + await rimraf.native(path); // $ MISSING: Alert + rimraf.native.sync(path); // $ MISSING: Alert + rimraf.nativeSync(path); // $ MISSING: Alert + await rimraf.manual(path); // $ MISSING: Alert + rimraf.manual(path); // $ MISSING: Alert + rimraf.manual.sync(path); // $ MISSING: Alert + rimraf.manualSync(path); // $ MISSING: Alert + await rimraf.windows(path); // $ MISSING: Alert + rimraf.windows(path); // $ MISSING: Alert + rimraf.windows.sync(path); // $ MISSING: Alert + rimraf.windowsSync(path); // $ MISSING: Alert + rimraf.moveRemove(path); // $ MISSING: Alert + rimraf.moveRemove.sync(path); // $ MISSING: Alert + rimraf.moveRemoveSync(path); // $ MISSING: Alert + rimraf.posixSync(path); // $ MISSING: Alert + rimraf.posix(path); // $ MISSING: Alert + rimraf.posix.sync(path); // $ MISSING: Alert +}); From b16b407f892d4adaf377f84ea099ad14fa3366e5 Mon Sep 17 00:00:00 2001 From: Napalys Date: Wed, 2 Apr 2025 12:49:48 +0200 Subject: [PATCH 2/3] Add `rimraf` model and update tests for path injection vulnerabilities --- javascript/ql/lib/ext/rimraf.model.yml | 8 +++ .../CWE-022/TaintedPath/TaintedPath.expected | 67 +++++++++++++++++++ .../Security/CWE-022/TaintedPath/rimraf.js | 42 ++++++------ 3 files changed, 96 insertions(+), 21 deletions(-) create mode 100644 javascript/ql/lib/ext/rimraf.model.yml diff --git a/javascript/ql/lib/ext/rimraf.model.yml b/javascript/ql/lib/ext/rimraf.model.yml new file mode 100644 index 000000000000..8afd53e47efe --- /dev/null +++ b/javascript/ql/lib/ext/rimraf.model.yml @@ -0,0 +1,8 @@ +extensions: + - addsTo: + pack: codeql/javascript-all + extensible: sinkModel + data: + - ["rimraf", "Member[sync,native,manual,windows,moveRemove,posix].Argument[0]", "path-injection"] + - ["rimraf", "Member[rimrafSync,nativeSync,manualSync,windowsSync,moveRemoveSync,posixSync].Argument[0]", "path-injection"] + - ["rimraf", "Member[native,manual,windows,moveRemove,posix].Member[sync].Argument[0]", "path-injection"] diff --git a/javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/TaintedPath.expected b/javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/TaintedPath.expected index 4147726065ec..99be2545b8e3 100644 --- a/javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/TaintedPath.expected +++ b/javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/TaintedPath.expected @@ -168,6 +168,26 @@ | prettier.js:11:44:11:44 | p | prettier.js:6:13:6:13 | p | prettier.js:11:44:11:44 | p | This path depends on a $@. | prettier.js:6:13:6:13 | p | user-provided value | | pupeteer.js:9:28:9:34 | tainted | pupeteer.js:5:28:5:53 | parseTo ... t).name | pupeteer.js:9:28:9:34 | tainted | This path depends on a $@. | pupeteer.js:5:28:5:53 | parseTo ... t).name | user-provided value | | pupeteer.js:13:37:13:43 | tainted | pupeteer.js:5:28:5:53 | parseTo ... t).name | pupeteer.js:13:37:13:43 | tainted | This path depends on a $@. | pupeteer.js:5:28:5:53 | parseTo ... t).name | user-provided value | +| rimraf.js:10:17:10:20 | path | rimraf.js:8:22:8:29 | req.body | rimraf.js:10:17:10:20 | path | This path depends on a $@. | rimraf.js:8:22:8:29 | req.body | user-provided value | +| rimraf.js:11:23:11:26 | path | rimraf.js:8:22:8:29 | req.body | rimraf.js:11:23:11:26 | path | This path depends on a $@. | rimraf.js:8:22:8:29 | req.body | user-provided value | +| rimraf.js:12:19:12:22 | path | rimraf.js:8:22:8:29 | req.body | rimraf.js:12:19:12:22 | path | This path depends on a $@. | rimraf.js:8:22:8:29 | req.body | user-provided value | +| rimraf.js:13:25:13:28 | path | rimraf.js:8:22:8:29 | req.body | rimraf.js:13:25:13:28 | path | This path depends on a $@. | rimraf.js:8:22:8:29 | req.body | user-provided value | +| rimraf.js:14:24:14:27 | path | rimraf.js:8:22:8:29 | req.body | rimraf.js:14:24:14:27 | path | This path depends on a $@. | rimraf.js:8:22:8:29 | req.body | user-provided value | +| rimraf.js:15:23:15:26 | path | rimraf.js:8:22:8:29 | req.body | rimraf.js:15:23:15:26 | path | This path depends on a $@. | rimraf.js:8:22:8:29 | req.body | user-provided value | +| rimraf.js:16:25:16:28 | path | rimraf.js:8:22:8:29 | req.body | rimraf.js:16:25:16:28 | path | This path depends on a $@. | rimraf.js:8:22:8:29 | req.body | user-provided value | +| rimraf.js:17:19:17:22 | path | rimraf.js:8:22:8:29 | req.body | rimraf.js:17:19:17:22 | path | This path depends on a $@. | rimraf.js:8:22:8:29 | req.body | user-provided value | +| rimraf.js:18:24:18:27 | path | rimraf.js:8:22:8:29 | req.body | rimraf.js:18:24:18:27 | path | This path depends on a $@. | rimraf.js:8:22:8:29 | req.body | user-provided value | +| rimraf.js:19:23:19:26 | path | rimraf.js:8:22:8:29 | req.body | rimraf.js:19:23:19:26 | path | This path depends on a $@. | rimraf.js:8:22:8:29 | req.body | user-provided value | +| rimraf.js:20:26:20:29 | path | rimraf.js:8:22:8:29 | req.body | rimraf.js:20:26:20:29 | path | This path depends on a $@. | rimraf.js:8:22:8:29 | req.body | user-provided value | +| rimraf.js:21:20:21:23 | path | rimraf.js:8:22:8:29 | req.body | rimraf.js:21:20:21:23 | path | This path depends on a $@. | rimraf.js:8:22:8:29 | req.body | user-provided value | +| rimraf.js:22:25:22:28 | path | rimraf.js:8:22:8:29 | req.body | rimraf.js:22:25:22:28 | path | This path depends on a $@. | rimraf.js:8:22:8:29 | req.body | user-provided value | +| rimraf.js:23:24:23:27 | path | rimraf.js:8:22:8:29 | req.body | rimraf.js:23:24:23:27 | path | This path depends on a $@. | rimraf.js:8:22:8:29 | req.body | user-provided value | +| rimraf.js:24:23:24:26 | path | rimraf.js:8:22:8:29 | req.body | rimraf.js:24:23:24:26 | path | This path depends on a $@. | rimraf.js:8:22:8:29 | req.body | user-provided value | +| rimraf.js:25:28:25:31 | path | rimraf.js:8:22:8:29 | req.body | rimraf.js:25:28:25:31 | path | This path depends on a $@. | rimraf.js:8:22:8:29 | req.body | user-provided value | +| rimraf.js:26:27:26:30 | path | rimraf.js:8:22:8:29 | req.body | rimraf.js:26:27:26:30 | path | This path depends on a $@. | rimraf.js:8:22:8:29 | req.body | user-provided value | +| rimraf.js:27:22:27:25 | path | rimraf.js:8:22:8:29 | req.body | rimraf.js:27:22:27:25 | path | This path depends on a $@. | rimraf.js:8:22:8:29 | req.body | user-provided value | +| rimraf.js:28:18:28:21 | path | rimraf.js:8:22:8:29 | req.body | rimraf.js:28:18:28:21 | path | This path depends on a $@. | rimraf.js:8:22:8:29 | req.body | user-provided value | +| rimraf.js:29:23:29:26 | path | rimraf.js:8:22:8:29 | req.body | rimraf.js:29:23:29:26 | path | This path depends on a $@. | rimraf.js:8:22:8:29 | req.body | user-provided value | | sharedlib-repro.js:22:18:22:25 | filepath | sharedlib-repro.js:13:22:13:43 | req.par ... spaceId | sharedlib-repro.js:22:18:22:25 | filepath | This path depends on a $@. | sharedlib-repro.js:13:22:13:43 | req.par ... spaceId | user-provided value | | tainted-access-paths.js:8:19:8:22 | path | tainted-access-paths.js:6:24:6:30 | req.url | tainted-access-paths.js:8:19:8:22 | path | This path depends on a $@. | tainted-access-paths.js:6:24:6:30 | req.url | user-provided value | | tainted-access-paths.js:12:19:12:25 | obj.sub | tainted-access-paths.js:6:24:6:30 | req.url | tainted-access-paths.js:12:19:12:25 | obj.sub | This path depends on a $@. | tainted-access-paths.js:6:24:6:30 | req.url | user-provided value | @@ -594,6 +614,29 @@ edges | pupeteer.js:5:9:5:71 | tainted | pupeteer.js:13:37:13:43 | tainted | provenance | | | pupeteer.js:5:19:5:71 | "dir/" ... t.data" | pupeteer.js:5:9:5:71 | tainted | provenance | | | pupeteer.js:5:28:5:53 | parseTo ... t).name | pupeteer.js:5:19:5:71 | "dir/" ... t.data" | provenance | Config | +| rimraf.js:8:11:8:18 | { path } | rimraf.js:8:13:8:16 | path | provenance | Config | +| rimraf.js:8:11:8:29 | path | rimraf.js:10:17:10:20 | path | provenance | | +| rimraf.js:8:11:8:29 | path | rimraf.js:11:23:11:26 | path | provenance | | +| rimraf.js:8:11:8:29 | path | rimraf.js:12:19:12:22 | path | provenance | | +| rimraf.js:8:11:8:29 | path | rimraf.js:13:25:13:28 | path | provenance | | +| rimraf.js:8:11:8:29 | path | rimraf.js:14:24:14:27 | path | provenance | | +| rimraf.js:8:11:8:29 | path | rimraf.js:15:23:15:26 | path | provenance | | +| rimraf.js:8:11:8:29 | path | rimraf.js:16:25:16:28 | path | provenance | | +| rimraf.js:8:11:8:29 | path | rimraf.js:17:19:17:22 | path | provenance | | +| rimraf.js:8:11:8:29 | path | rimraf.js:18:24:18:27 | path | provenance | | +| rimraf.js:8:11:8:29 | path | rimraf.js:19:23:19:26 | path | provenance | | +| rimraf.js:8:11:8:29 | path | rimraf.js:20:26:20:29 | path | provenance | | +| rimraf.js:8:11:8:29 | path | rimraf.js:21:20:21:23 | path | provenance | | +| rimraf.js:8:11:8:29 | path | rimraf.js:22:25:22:28 | path | provenance | | +| rimraf.js:8:11:8:29 | path | rimraf.js:23:24:23:27 | path | provenance | | +| rimraf.js:8:11:8:29 | path | rimraf.js:24:23:24:26 | path | provenance | | +| rimraf.js:8:11:8:29 | path | rimraf.js:25:28:25:31 | path | provenance | | +| rimraf.js:8:11:8:29 | path | rimraf.js:26:27:26:30 | path | provenance | | +| rimraf.js:8:11:8:29 | path | rimraf.js:27:22:27:25 | path | provenance | | +| rimraf.js:8:11:8:29 | path | rimraf.js:28:18:28:21 | path | provenance | | +| rimraf.js:8:11:8:29 | path | rimraf.js:29:23:29:26 | path | provenance | | +| rimraf.js:8:13:8:16 | path | rimraf.js:8:11:8:29 | path | provenance | | +| rimraf.js:8:22:8:29 | req.body | rimraf.js:8:11:8:18 | { path } | provenance | | | sharedlib-repro.js:13:22:13:43 | req.par ... spaceId | sharedlib-repro.js:21:27:21:34 | filepath | provenance | | | sharedlib-repro.js:21:27:21:34 | filepath | sharedlib-repro.js:22:18:22:25 | filepath | provenance | | | tainted-access-paths.js:6:7:6:48 | path | tainted-access-paths.js:8:19:8:22 | path | provenance | | @@ -1133,6 +1176,30 @@ nodes | pupeteer.js:5:28:5:53 | parseTo ... t).name | semmle.label | parseTo ... t).name | | pupeteer.js:9:28:9:34 | tainted | semmle.label | tainted | | pupeteer.js:13:37:13:43 | tainted | semmle.label | tainted | +| rimraf.js:8:11:8:18 | { path } | semmle.label | { path } | +| rimraf.js:8:11:8:29 | path | semmle.label | path | +| rimraf.js:8:13:8:16 | path | semmle.label | path | +| rimraf.js:8:22:8:29 | req.body | semmle.label | req.body | +| rimraf.js:10:17:10:20 | path | semmle.label | path | +| rimraf.js:11:23:11:26 | path | semmle.label | path | +| rimraf.js:12:19:12:22 | path | semmle.label | path | +| rimraf.js:13:25:13:28 | path | semmle.label | path | +| rimraf.js:14:24:14:27 | path | semmle.label | path | +| rimraf.js:15:23:15:26 | path | semmle.label | path | +| rimraf.js:16:25:16:28 | path | semmle.label | path | +| rimraf.js:17:19:17:22 | path | semmle.label | path | +| rimraf.js:18:24:18:27 | path | semmle.label | path | +| rimraf.js:19:23:19:26 | path | semmle.label | path | +| rimraf.js:20:26:20:29 | path | semmle.label | path | +| rimraf.js:21:20:21:23 | path | semmle.label | path | +| rimraf.js:22:25:22:28 | path | semmle.label | path | +| rimraf.js:23:24:23:27 | path | semmle.label | path | +| rimraf.js:24:23:24:26 | path | semmle.label | path | +| rimraf.js:25:28:25:31 | path | semmle.label | path | +| rimraf.js:26:27:26:30 | path | semmle.label | path | +| rimraf.js:27:22:27:25 | path | semmle.label | path | +| rimraf.js:28:18:28:21 | path | semmle.label | path | +| rimraf.js:29:23:29:26 | path | semmle.label | path | | sharedlib-repro.js:13:22:13:43 | req.par ... spaceId | semmle.label | req.par ... spaceId | | sharedlib-repro.js:21:27:21:34 | filepath | semmle.label | filepath | | sharedlib-repro.js:22:18:22:25 | filepath | semmle.label | filepath | diff --git a/javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/rimraf.js b/javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/rimraf.js index a6f3c52068ac..9595078cf8df 100644 --- a/javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/rimraf.js +++ b/javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/rimraf.js @@ -5,26 +5,26 @@ const app = express(); app.use(express.json()); app.post('/rmsync', async (req, res) => { - const { path } = req.body; // $ MISSING: Source + const { path } = req.body; // $ Source - rimraf.sync(path); // $ MISSING: Alert - rimraf.rimrafSync(path); // $ MISSING: Alert - rimraf.native(path); // $ MISSING: Alert - await rimraf.native(path); // $ MISSING: Alert - rimraf.native.sync(path); // $ MISSING: Alert - rimraf.nativeSync(path); // $ MISSING: Alert - await rimraf.manual(path); // $ MISSING: Alert - rimraf.manual(path); // $ MISSING: Alert - rimraf.manual.sync(path); // $ MISSING: Alert - rimraf.manualSync(path); // $ MISSING: Alert - await rimraf.windows(path); // $ MISSING: Alert - rimraf.windows(path); // $ MISSING: Alert - rimraf.windows.sync(path); // $ MISSING: Alert - rimraf.windowsSync(path); // $ MISSING: Alert - rimraf.moveRemove(path); // $ MISSING: Alert - rimraf.moveRemove.sync(path); // $ MISSING: Alert - rimraf.moveRemoveSync(path); // $ MISSING: Alert - rimraf.posixSync(path); // $ MISSING: Alert - rimraf.posix(path); // $ MISSING: Alert - rimraf.posix.sync(path); // $ MISSING: Alert + rimraf.sync(path); // $ Alert + rimraf.rimrafSync(path); // $ Alert + rimraf.native(path); // $ Alert + await rimraf.native(path); // $ Alert + rimraf.native.sync(path); // $ Alert + rimraf.nativeSync(path); // $ Alert + await rimraf.manual(path); // $ Alert + rimraf.manual(path); // $ Alert + rimraf.manual.sync(path); // $ Alert + rimraf.manualSync(path); // $ Alert + await rimraf.windows(path); // $ Alert + rimraf.windows(path); // $ Alert + rimraf.windows.sync(path); // $ Alert + rimraf.windowsSync(path); // $ Alert + rimraf.moveRemove(path); // $ Alert + rimraf.moveRemove.sync(path); // $ Alert + rimraf.moveRemoveSync(path); // $ Alert + rimraf.posixSync(path); // $ Alert + rimraf.posix(path); // $ Alert + rimraf.posix.sync(path); // $ Alert }); From 390d9ffe668f3218d939788ae7ebdeb6e61c6006 Mon Sep 17 00:00:00 2001 From: Napalys Date: Wed, 2 Apr 2025 12:50:53 +0200 Subject: [PATCH 3/3] Added change note --- javascript/ql/lib/change-notes/2025-04-02-rimraf.md | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 javascript/ql/lib/change-notes/2025-04-02-rimraf.md diff --git a/javascript/ql/lib/change-notes/2025-04-02-rimraf.md b/javascript/ql/lib/change-notes/2025-04-02-rimraf.md new file mode 100644 index 000000000000..3d0521643d59 --- /dev/null +++ b/javascript/ql/lib/change-notes/2025-04-02-rimraf.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* Added support for additional `rimraf` methods as sinks in path-injection queries.