diff --git a/javascript/ql/lib/change-notes/2025-04-30-shelljs.md b/javascript/ql/lib/change-notes/2025-04-30-shelljs.md new file mode 100644 index 000000000000..90a5f5a2a308 --- /dev/null +++ b/javascript/ql/lib/change-notes/2025-04-30-shelljs.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* Improved modeling of the [`shelljs`](https://www.npmjs.com/package/shelljs) and [`async-shelljs`](https://www.npmjs.com/package/async-shelljs) libraries by adding support for the `which`, `cmd`, `asyncExec` and `env`. diff --git a/javascript/ql/lib/ext/shelljs.model.yml b/javascript/ql/lib/ext/shelljs.model.yml new file mode 100644 index 000000000000..8d2c8db645d8 --- /dev/null +++ b/javascript/ql/lib/ext/shelljs.model.yml @@ -0,0 +1,6 @@ +extensions: + - addsTo: + pack: codeql/javascript-all + extensible: sourceModel + data: + - ["shelljs", "Member[env]", "environment"] diff --git a/javascript/ql/lib/semmle/javascript/frameworks/ShellJS.qll b/javascript/ql/lib/semmle/javascript/frameworks/ShellJS.qll index 9f3eeb6e0dc9..fc0e366df4b8 100644 --- a/javascript/ql/lib/semmle/javascript/frameworks/ShellJS.qll +++ b/javascript/ql/lib/semmle/javascript/frameworks/ShellJS.qll @@ -14,7 +14,8 @@ module ShellJS { shellJSMember() .getMember([ "exec", "cd", "cp", "touch", "chmod", "pushd", "find", "ls", "ln", "mkdir", "mv", - "rm", "cat", "head", "sort", "tail", "uniq", "grep", "sed", "to", "toEnd", "echo" + "rm", "cat", "head", "sort", "tail", "uniq", "grep", "sed", "to", "toEnd", "echo", + "which", "cmd", "asyncExec" ]) .getReturn() } @@ -99,7 +100,8 @@ module ShellJS { */ private class ShellJSGenericFileAccess extends FileSystemAccess, ShellJSCall { ShellJSGenericFileAccess() { - name = ["cd", "cp", "touch", "chmod", "pushd", "find", "ls", "ln", "mkdir", "mv", "rm"] + name = + ["cd", "cp", "touch", "chmod", "pushd", "find", "ls", "ln", "mkdir", "mv", "rm", "which"] } override DataFlow::Node getAPathArgument() { result = this.getAnArgument() } @@ -111,7 +113,8 @@ module ShellJS { private class ShellJSFilenameSource extends FileNameSource, ShellJSCall { ShellJSFilenameSource() { name = "find" or - name = "ls" + name = "ls" or + name = "which" } } @@ -151,16 +154,24 @@ module ShellJS { } /** - * A call to `shelljs.exec()` modeled as command execution. + * A call to `shelljs.exec()`, `shelljs.cmd()`, or `async-shelljs.asyncExec()` modeled as command execution. */ private class ShellJSExec extends SystemCommandExecution, ShellJSCall { - ShellJSExec() { name = "exec" } - - override DataFlow::Node getACommandArgument() { result = this.getArgument(0) } + ShellJSExec() { name = ["exec", "cmd", "asyncExec"] } + + override DataFlow::Node getACommandArgument() { + if name = "cmd" + then + result = this.getArgument(_) and + not result = this.getOptionsArg() + else + // For exec/asyncExec: only first argument is command + result = this.getArgument(0) + } override predicate isShellInterpreted(DataFlow::Node arg) { arg = this.getACommandArgument() } - override predicate isSync() { none() } + override predicate isSync() { name = "cmd" } override DataFlow::Node getOptionsArg() { result = this.getLastArgument() and diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/CleartextLoggingCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/CleartextLoggingCustomizations.qll index 5dca4cf1df28..dbb775f99b58 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/CleartextLoggingCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/CleartextLoggingCustomizations.qll @@ -171,7 +171,7 @@ module CleartextLogging { /** An access to the sensitive object `process.env`. */ class ProcessEnvSource extends Source { - ProcessEnvSource() { this = NodeJSLib::process().getAPropertyRead("env") } + ProcessEnvSource() { this.(ThreatModelSource).getThreatModel() = "environment" } override string describe() { result = "process environment" } } diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/IndirectCommandInjectionCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/IndirectCommandInjectionCustomizations.qll index 9dd6ab4b4a91..84c4600fbc03 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/IndirectCommandInjectionCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/IndirectCommandInjectionCustomizations.qll @@ -29,7 +29,7 @@ module IndirectCommandInjection { * A read of `process.env`, considered as a flow source for command injection. */ private class ProcessEnvAsSource extends Source { - ProcessEnvAsSource() { this = NodeJSLib::process().getAPropertyRead("env") } + ProcessEnvAsSource() { this.(ThreatModelSource).getThreatModel() = "environment" } override string describe() { result = "environment variable" } } @@ -37,7 +37,7 @@ module IndirectCommandInjection { /** Gets a data flow node referring to `process.env`. */ private DataFlow::SourceNode envObject(DataFlow::TypeTracker t) { t.start() and - result = NodeJSLib::process().getAPropertyRead("env") + result.(ThreatModelSource).getThreatModel() = "environment" or exists(DataFlow::TypeTracker t2 | result = envObject(t2).track(t2, t)) } diff --git a/javascript/ql/src/Security/CWE-295/DisablingCertificateValidation.ql b/javascript/ql/src/Security/CWE-295/DisablingCertificateValidation.ql index d9f7267d425a..b5e52d7c6c79 100644 --- a/javascript/ql/src/Security/CWE-295/DisablingCertificateValidation.ql +++ b/javascript/ql/src/Security/CWE-295/DisablingCertificateValidation.ql @@ -37,7 +37,7 @@ DataFlow::ObjectLiteralNode tlsOptions() { result.flowsTo(tlsInvocation().getAnA from DataFlow::PropWrite disable where exists(DataFlow::SourceNode env | - env = NodeJSLib::process().getAPropertyRead("env") and + env.(ThreatModelSource).getThreatModel() = "environment" and disable = env.getAPropertyWrite("NODE_TLS_REJECT_UNAUTHORIZED") and disable.getRhs().mayHaveStringValue("0") ) diff --git a/javascript/ql/test/library-tests/frameworks/Shelljs/ShellJS.expected b/javascript/ql/test/library-tests/frameworks/Shelljs/ShellJS.expected index 003ae3f442f8..5c3c922a84fc 100644 --- a/javascript/ql/test/library-tests/frameworks/Shelljs/ShellJS.expected +++ b/javascript/ql/test/library-tests/frameworks/Shelljs/ShellJS.expected @@ -55,14 +55,19 @@ test_FileSystemAccess | tst.js:60:1:60:17 | shelljs.cat(file) | | tst.js:60:1:60:41 | shelljs ... cement) | | tst.js:61:1:61:17 | shelljs.cat(file) | +| tst.js:65:1:65:19 | shelljs.which(file) | test_MissingFileSystemAccess test_SystemCommandExecution | tst.js:14:1:14:27 | shelljs ... ts, cb) | | tst.js:60:1:60:51 | shelljs ... ec(cmd) | | tst.js:61:1:61:27 | shelljs ... ec(cmd) | +| tst.js:63:1:63:37 | shelljs ... ptions) | +| tst.js:64:1:64:16 | shelljs.cmd(cmd) | +| tst.js:68:1:68:36 | shelljs ... ts, cb) | test_FileNameSource | tst.js:15:1:15:26 | shelljs ... file2) | | tst.js:24:1:24:16 | shelljs.ls(file) | | tst.js:25:1:25:22 | shelljs ... , file) | | tst.js:26:1:26:30 | shelljs ... file2) | | tst.js:27:1:27:24 | shelljs ... file2) | +| tst.js:65:1:65:19 | shelljs.which(file) | diff --git a/javascript/ql/test/library-tests/frameworks/Shelljs/tst.js b/javascript/ql/test/library-tests/frameworks/Shelljs/tst.js index 9e7905c98477..2bbcd51f5821 100644 --- a/javascript/ql/test/library-tests/frameworks/Shelljs/tst.js +++ b/javascript/ql/test/library-tests/frameworks/Shelljs/tst.js @@ -59,3 +59,10 @@ shelljs.uniq(opts, file1, file2); shelljs.cat(file).sed(regex, replacement).exec(cmd); shelljs.cat(file).exec(cmd); + +shelljs.cmd(cmd, arg1, arg2, options); +shelljs.cmd(cmd); +shelljs.which(file); + +const shelljssync = require("async-shelljs"); +shelljssync.asyncExec(cmd, opts, cb); diff --git a/javascript/ql/test/query-tests/Security/CWE-078/IndirectCommandInjection/IndirectCommandInjection.expected b/javascript/ql/test/query-tests/Security/CWE-078/IndirectCommandInjection/IndirectCommandInjection.expected index 7c7321845776..9fc6f6b1bc4e 100644 --- a/javascript/ql/test/query-tests/Security/CWE-078/IndirectCommandInjection/IndirectCommandInjection.expected +++ b/javascript/ql/test/query-tests/Security/CWE-078/IndirectCommandInjection/IndirectCommandInjection.expected @@ -2,6 +2,9 @@ | actions.js:4:6:4:29 | process ... _DATA'] | actions.js:4:6:4:16 | process.env | actions.js:4:6:4:29 | process ... _DATA'] | This command depends on an unsanitized $@. | actions.js:4:6:4:16 | process.env | environment variable | | actions.js:8:10:8:23 | e['TEST_DATA'] | actions.js:12:6:12:16 | process.env | actions.js:8:10:8:23 | e['TEST_DATA'] | This command depends on an unsanitized $@. | actions.js:12:6:12:16 | process.env | environment variable | | actions.js:14:6:14:21 | getInput('data') | actions.js:14:6:14:21 | getInput('data') | actions.js:14:6:14:21 | getInput('data') | This command depends on an unsanitized $@. | actions.js:14:6:14:21 | getInput('data') | GitHub Actions user input | +| actions.js:18:10:18:40 | 'rm -rf ... 'SOME'] | actions.js:18:22:18:32 | shelljs.env | actions.js:18:10:18:40 | 'rm -rf ... 'SOME'] | This command depends on an unsanitized $@. | actions.js:18:22:18:32 | shelljs.env | environment variable | +| actions.js:19:10:19:37 | 'rm -rf ... nv.SOME | actions.js:19:22:19:32 | shelljs.env | actions.js:19:10:19:37 | 'rm -rf ... nv.SOME | This command depends on an unsanitized $@. | actions.js:19:22:19:32 | shelljs.env | environment variable | +| actions.js:20:10:20:32 | 'rm -rf ... ljs.env | actions.js:20:22:20:32 | shelljs.env | actions.js:20:10:20:32 | 'rm -rf ... ljs.env | This command depends on an unsanitized $@. | actions.js:20:22:20:32 | shelljs.env | environment variable | | command-line-parameter-command-injection.js:4:10:4:21 | process.argv | command-line-parameter-command-injection.js:4:10:4:21 | process.argv | command-line-parameter-command-injection.js:4:10:4:21 | process.argv | This command depends on an unsanitized $@. | command-line-parameter-command-injection.js:4:10:4:21 | process.argv | command-line argument | | command-line-parameter-command-injection.js:8:10:8:36 | "cmd.sh ... argv[2] | command-line-parameter-command-injection.js:8:22:8:33 | process.argv | command-line-parameter-command-injection.js:8:10:8:36 | "cmd.sh ... argv[2] | This command depends on an unsanitized $@. | command-line-parameter-command-injection.js:8:22:8:33 | process.argv | command-line argument | | command-line-parameter-command-injection.js:11:14:11:20 | args[0] | command-line-parameter-command-injection.js:10:13:10:24 | process.argv | command-line-parameter-command-injection.js:11:14:11:20 | args[0] | This command depends on an unsanitized $@. | command-line-parameter-command-injection.js:10:13:10:24 | process.argv | command-line argument | @@ -44,6 +47,9 @@ edges | actions.js:7:15:7:15 | e | actions.js:8:10:8:10 | e | provenance | | | actions.js:8:10:8:10 | e | actions.js:8:10:8:23 | e['TEST_DATA'] | provenance | | | actions.js:12:6:12:16 | process.env | actions.js:7:15:7:15 | e | provenance | | +| actions.js:18:22:18:32 | shelljs.env | actions.js:18:10:18:40 | 'rm -rf ... 'SOME'] | provenance | | +| actions.js:19:22:19:32 | shelljs.env | actions.js:19:10:19:37 | 'rm -rf ... nv.SOME | provenance | | +| actions.js:20:22:20:32 | shelljs.env | actions.js:20:10:20:32 | 'rm -rf ... ljs.env | provenance | | | command-line-parameter-command-injection.js:8:22:8:33 | process.argv | command-line-parameter-command-injection.js:8:10:8:36 | "cmd.sh ... argv[2] | provenance | | | command-line-parameter-command-injection.js:10:6:10:33 | args | command-line-parameter-command-injection.js:11:14:11:17 | args | provenance | | | command-line-parameter-command-injection.js:10:6:10:33 | args | command-line-parameter-command-injection.js:12:26:12:29 | args | provenance | | @@ -181,6 +187,12 @@ nodes | actions.js:8:10:8:23 | e['TEST_DATA'] | semmle.label | e['TEST_DATA'] | | actions.js:12:6:12:16 | process.env | semmle.label | process.env | | actions.js:14:6:14:21 | getInput('data') | semmle.label | getInput('data') | +| actions.js:18:10:18:40 | 'rm -rf ... 'SOME'] | semmle.label | 'rm -rf ... 'SOME'] | +| actions.js:18:22:18:32 | shelljs.env | semmle.label | shelljs.env | +| actions.js:19:10:19:37 | 'rm -rf ... nv.SOME | semmle.label | 'rm -rf ... nv.SOME | +| actions.js:19:22:19:32 | shelljs.env | semmle.label | shelljs.env | +| actions.js:20:10:20:32 | 'rm -rf ... ljs.env | semmle.label | 'rm -rf ... ljs.env | +| actions.js:20:22:20:32 | shelljs.env | semmle.label | shelljs.env | | command-line-parameter-command-injection.js:4:10:4:21 | process.argv | semmle.label | process.argv | | command-line-parameter-command-injection.js:8:10:8:36 | "cmd.sh ... argv[2] | semmle.label | "cmd.sh ... argv[2] | | command-line-parameter-command-injection.js:8:22:8:33 | process.argv | semmle.label | process.argv | diff --git a/javascript/ql/test/query-tests/Security/CWE-078/IndirectCommandInjection/actions.js b/javascript/ql/test/query-tests/Security/CWE-078/IndirectCommandInjection/actions.js index 021715395217..8fb6994a597c 100644 --- a/javascript/ql/test/query-tests/Security/CWE-078/IndirectCommandInjection/actions.js +++ b/javascript/ql/test/query-tests/Security/CWE-078/IndirectCommandInjection/actions.js @@ -12,3 +12,10 @@ function test(e) { test(process.env); // $ Source exec(getInput('data')); // $ Alert + +function test2(e) { + const shelljs = require('shelljs'); + exec('rm -rf ' + shelljs.env['SOME']); // $ Alert + exec('rm -rf ' + shelljs.env.SOME); // $ Alert + exec('rm -rf ' + shelljs.env); // $ Alert +}