diff --git a/python/ql/lib/change-notes/2026-08-26-shell-interpreter-command-arguments.md b/python/ql/lib/change-notes/2026-08-26-shell-interpreter-command-arguments.md new file mode 100644 index 000000000000..d17368f059fd --- /dev/null +++ b/python/ql/lib/change-notes/2026-08-26-shell-interpreter-command-arguments.md @@ -0,0 +1,6 @@ +--- +category: minorAnalysis +--- +* Direct `-c` command arguments to recognized POSIX shell interpreters through `os.exec*`, + `os.spawn*`, `os.posix_spawn*`, and `subprocess` APIs are now treated as command-injection + sinks. diff --git a/python/ql/lib/semmle/python/frameworks/Stdlib.qll b/python/ql/lib/semmle/python/frameworks/Stdlib.qll index c02aa4bb6d8a..a88c3e84aff3 100644 --- a/python/ql/lib/semmle/python/frameworks/Stdlib.qll +++ b/python/ql/lib/semmle/python/frameworks/Stdlib.qll @@ -1171,6 +1171,36 @@ module StdlibPrivate { override predicate isShellInterpreted(DataFlow::Node arg) { arg = this.getCommand() } } + /** + * Holds if `flag` makes `interpreter` execute the following argument as a command. + * + * See https://docs.python.org/3/library/subprocess.html#popen-constructor. + */ + private predicate isShellCommandFlag(DataFlow::Node interpreter, DataFlow::Node flag) { + interpreter.asExpr().(StringLiteral).getText().regexpMatch("(.*/)?(sh|bash|dash|zsh)") and + flag.asExpr().(StringLiteral).getText() = "-c" + } + + /** Gets the command from separate interpreter, flag, and command arguments. */ + private DataFlow::Node getShellCommandFromArguments( + DataFlow::Node interpreter, DataFlow::Node flag, DataFlow::Node command + ) { + isShellCommandFlag(interpreter, flag) and + result = command + } + + /** Gets the command from an argument sequence passed to `interpreter`. */ + private DataFlow::Node getShellCommandFromSequence( + DataFlow::Node interpreter, DataFlow::Node arguments + ) { + exists(SequenceNode sequence, DataFlow::Node flag | + arguments.asCfgNode() = sequence and + flag.asCfgNode() = sequence.getElement(1) and + isShellCommandFlag(interpreter, flag) and + result.asCfgNode() = sequence.getElement(2) + ) + } + /** * A call to any of the `os.exec*` functions * See https://docs.python.org/3.8/library/os.html#os.execl @@ -1178,20 +1208,30 @@ module StdlibPrivate { private class OsExecCall extends SystemCommandExecution::Range, FileSystemAccess::Range, DataFlow::CallCfgNode { + string name; + OsExecCall() { - exists(string name | - name in ["execl", "execle", "execlp", "execlpe", "execv", "execve", "execvp", "execvpe"] and - this = os().getMember(name).getACall() - ) + name in ["execl", "execle", "execlp", "execlpe", "execv", "execve", "execvp", "execvpe"] and + this = os().getMember(name).getACall() + } + + private DataFlow::Node getShellCommand() { + name in ["execl", "execlp"] and + result = getShellCommandFromArguments(this.getArg(0), this.getArg(2), this.getArg(3)) + or + name in ["execle", "execlpe"] and + exists(this.getArg(4)) and + result = getShellCommandFromArguments(this.getArg(0), this.getArg(2), this.getArg(3)) + or + name in ["execv", "execve", "execvp", "execvpe"] and + result = getShellCommandFromSequence(this.getArg(0), this.getArg(1)) } - override DataFlow::Node getCommand() { result = this.getArg(0) } + override DataFlow::Node getCommand() { result in [this.getArg(0), this.getShellCommand()] } - override DataFlow::Node getAPathArgument() { result = this.getCommand() } + override DataFlow::Node getAPathArgument() { result = this.getArg(0) } - override predicate isShellInterpreted(DataFlow::Node arg) { - none() // this is a safe API. - } + override predicate isShellInterpreted(DataFlow::Node arg) { arg = this.getShellCommand() } } /** @@ -1201,16 +1241,16 @@ module StdlibPrivate { private class OsSpawnCall extends SystemCommandExecution::Range, FileSystemAccess::Range, DataFlow::CallCfgNode { + string name; + OsSpawnCall() { - exists(string name | - name in [ - "spawnl", "spawnle", "spawnlp", "spawnlpe", "spawnv", "spawnve", "spawnvp", "spawnvpe" - ] and - this = os().getMember(name).getACall() - ) + name in [ + "spawnl", "spawnle", "spawnlp", "spawnlpe", "spawnv", "spawnve", "spawnvp", "spawnvpe" + ] and + this = os().getMember(name).getACall() } - override DataFlow::Node getCommand() { + private DataFlow::Node getInterpreter() { result = this.getArg(1) or // `file` keyword argument only valid for the `v` variants, but this @@ -1218,11 +1258,27 @@ module StdlibPrivate { result = this.getArgByName("file") } - override DataFlow::Node getAPathArgument() { result = this.getCommand() } + private DataFlow::Node getArguments() { result in [this.getArg(2), this.getArgByName("args")] } - override predicate isShellInterpreted(DataFlow::Node arg) { - none() // this is a safe API. + private DataFlow::Node getShellCommand() { + name in ["spawnl", "spawnlp"] and + result = getShellCommandFromArguments(this.getInterpreter(), this.getArg(3), this.getArg(4)) + or + name in ["spawnle", "spawnlpe"] and + exists(this.getArg(5)) and + result = getShellCommandFromArguments(this.getInterpreter(), this.getArg(3), this.getArg(4)) + or + name in ["spawnv", "spawnve", "spawnvp", "spawnvpe"] and + result = getShellCommandFromSequence(this.getInterpreter(), this.getArguments()) + } + + override DataFlow::Node getCommand() { + result in [this.getInterpreter(), this.getShellCommand()] } + + override DataFlow::Node getAPathArgument() { result = this.getInterpreter() } + + override predicate isShellInterpreted(DataFlow::Node arg) { arg = this.getShellCommand() } } /** @@ -1234,13 +1290,23 @@ module StdlibPrivate { { OsPosixSpawnCall() { this = os().getMember(["posix_spawn", "posix_spawnp"]).getACall() } - override DataFlow::Node getCommand() { result in [this.getArg(0), this.getArgByName("path")] } + private DataFlow::Node getInterpreter() { + result in [this.getArg(0), this.getArgByName("path")] + } - override DataFlow::Node getAPathArgument() { result = this.getCommand() } + private DataFlow::Node getArguments() { result in [this.getArg(1), this.getArgByName("argv")] } - override predicate isShellInterpreted(DataFlow::Node arg) { - none() // this is a safe API. + private DataFlow::Node getShellCommand() { + result = getShellCommandFromSequence(this.getInterpreter(), this.getArguments()) + } + + override DataFlow::Node getCommand() { + result in [this.getInterpreter(), this.getShellCommand()] } + + override DataFlow::Node getAPathArgument() { result = this.getInterpreter() } + + override predicate isShellInterpreted(DataFlow::Node arg) { arg = this.getShellCommand() } } /** An additional taint step for calls to `os.path.join` */ @@ -1267,13 +1333,11 @@ module StdlibPrivate { * ref: https://docs.python.org/3/library/subprocess.html#legacy-shell-invocation-functions */ private class SubprocessPopenCall extends SystemCommandExecution::Range, API::CallNode { + string name; + SubprocessPopenCall() { - exists(string name | - name in [ - "Popen", "call", "check_call", "check_output", "run", "getoutput", "getstatusoutput" - ] and - this = subprocess().getMember(name).getACall() - ) + name in ["Popen", "call", "check_call", "check_output", "run", "getoutput", "getstatusoutput"] and + this = subprocess().getMember(name).getACall() } /** Gets the API-node for the `args` argument, if any. */ @@ -1296,20 +1360,43 @@ module StdlibPrivate { /** Gets the API-node for the `executable` argument, if any. */ private API::Node get_executable_arg() { result = this.getParameter(2, "executable") } + /** Holds if the `executable` argument overrides the executable from `args`. */ + private predicate hasExecutableOverride() { + exists(DataFlow::Node executable | + executable = this.get_executable_arg().asSink() and + not executable.asExpr() instanceof None + ) + } + + /** Gets the interpreter that will execute `args`, if it can be determined. */ + private DataFlow::Node getInterpreter() { + this.hasExecutableOverride() and + result = this.get_executable_arg().asSink() + or + not this.hasExecutableOverride() and + result.asCfgNode() = this.get_args_arg().asSink().asCfgNode().(SequenceNode).getElement(0) + } + + /** Gets a shell command passed in the `args` sequence. */ + private DataFlow::Node getShellCommand() { + name in ["Popen", "call", "check_call", "check_output", "run"] and + this.get_shell_arg_value() = false and + result = getShellCommandFromSequence(this.getInterpreter(), this.get_args_arg().asSink()) + } + override DataFlow::Node getCommand() { - // TODO: Track arguments ("args" and "shell") - // TODO: Handle using `args=["sh", "-c", ]` + this.hasExecutableOverride() and result = this.get_executable_arg().asSink() or exists(DataFlow::Node arg_args, boolean shell | arg_args = this.get_args_arg().asSink() and shell = this.get_shell_arg_value() | - // When "executable" argument is set, and "shell" argument is `False`, the - // "args" argument will only be used to set the program name and arguments to + // When the `executable` argument overrides `args[0]`, and `shell` is `False`, the + // `args` argument will only be used to set the program name and arguments to // the program, so we should not consider any of them as command execution. not ( - exists(this.get_executable_arg()) and + this.hasExecutableOverride() and shell = false ) and ( @@ -1327,11 +1414,15 @@ module StdlibPrivate { result = arg_args ) ) + or + result = this.getShellCommand() } override predicate isShellInterpreted(DataFlow::Node arg) { arg = [this.get_executable_arg(), this.get_args_arg()].asSink() and this.get_shell_arg_value() = true + or + arg = this.getShellCommand() } } diff --git a/python/ql/test/library-tests/frameworks/stdlib/SystemCommandExecution.py b/python/ql/test/library-tests/frameworks/stdlib/SystemCommandExecution.py index 824639dffeac..0129b0dff30b 100644 --- a/python/ql/test/library-tests/frameworks/stdlib/SystemCommandExecution.py +++ b/python/ql/test/library-tests/frameworks/stdlib/SystemCommandExecution.py @@ -105,30 +105,61 @@ def os_members(): ######################################## # actively using known shell as the executable -subprocess.Popen(["/bin/sh", "-c", "vuln"]) # $ getCommand="/bin/sh" MISSING: getCommand="vuln" -subprocess.Popen(["/bin/bash", "-c", "vuln"]) # $ getCommand="/bin/bash" MISSING: getCommand="vuln" -subprocess.Popen(["/bin/dash", "-c", "vuln"]) # $ getCommand="/bin/dash" MISSING: getCommand="vuln" -subprocess.Popen(["/bin/zsh", "-c", "vuln"]) # $ getCommand="/bin/zsh" MISSING: getCommand="vuln" - -subprocess.Popen(["sh", "-c", "vuln"]) # $ getCommand="sh" MISSING: getCommand="vuln" -subprocess.Popen(["bash", "-c", "vuln"]) # $ getCommand="bash" MISSING: getCommand="vuln" -subprocess.Popen(["dash", "-c", "vuln"]) # $ getCommand="dash" MISSING: getCommand="vuln" -subprocess.Popen(["zsh", "-c", "vuln"]) # $ getCommand="zsh" MISSING: getCommand="vuln" +subprocess.Popen(["/bin/sh", "-c", "vuln"]) # $ getCommand="/bin/sh" getCommand="vuln" +subprocess.Popen(["/bin/bash", "-c", "vuln"]) # $ getCommand="/bin/bash" getCommand="vuln" +subprocess.Popen(["/bin/dash", "-c", "vuln"]) # $ getCommand="/bin/dash" getCommand="vuln" +subprocess.Popen(["/bin/zsh", "-c", "vuln"]) # $ getCommand="/bin/zsh" getCommand="vuln" + +subprocess.Popen(["sh", "-c", "vuln"]) # $ getCommand="sh" getCommand="vuln" +subprocess.Popen(["bash", "-c", "vuln"]) # $ getCommand="bash" getCommand="vuln" +subprocess.Popen(["dash", "-c", "vuln"]) # $ getCommand="dash" getCommand="vuln" +subprocess.Popen(["zsh", "-c", "vuln"]) # $ getCommand="zsh" getCommand="vuln" +subprocess.run(("/usr/local/bin/sh", "-c", "vuln")) # $ getCommand="/usr/local/bin/sh" getCommand="vuln" +subprocess.run(args=["sh", "-c", "vuln"]) # $ getCommand="sh" getCommand="vuln" +subprocess.Popen(["sh", "-c", "vuln", "not-command"]) # $ getCommand="sh" getCommand="vuln" # Check that we don't consider ANY argument a command injection sink subprocess.Popen(["sh", "/bin/python"]) # $ getCommand="sh" +subprocess.Popen(["sh", "--command", "not-vuln"]) # $ getCommand="sh" +subprocess.Popen(["sh", "-C", "not-vuln"]) # $ getCommand="sh" +subprocess.Popen(["not-a-shell", "-c", "not-vuln"]) # $ getCommand="not-a-shell" +subprocess.Popen(["sh", "-c"]) # $ getCommand="sh" +subprocess.Popen(["sh", "-c", "not-vuln"], shell=True) # $ getCommand="sh" +subprocess.getoutput(["sh", "-c", "not-vuln"]) # $ getCommand="sh" +subprocess.getstatusoutput(("sh", "-c", "not-vuln")) # $ getCommand="sh" subprocess.Popen(["cmd.exe", "/c", "vuln"]) # $ getCommand="cmd.exe" MISSING: getCommand="vuln" subprocess.Popen(["cmd.exe", "/C", "vuln"]) # $ getCommand="cmd.exe" MISSING: getCommand="vuln" subprocess.Popen(["cmd", "/c", "vuln"]) # $ getCommand="cmd" MISSING: getCommand="vuln" subprocess.Popen(["cmd", "/C", "vuln"]) # $ getCommand="cmd" MISSING: getCommand="vuln" -subprocess.Popen(["", "-c", "vuln"], executable="/bin/bash") # $ getCommand="/bin/bash" MISSING: getCommand="vuln" +subprocess.Popen(["", "-c", "vuln"], executable="/bin/bash") # $ getCommand="/bin/bash" getCommand="vuln" +subprocess.Popen(["sh", "-c", "not-vuln"], executable="/bin/echo") # $ getCommand="/bin/echo" +subprocess.Popen(["sh", "-c", "vuln"], executable=None) # $ getCommand="sh" getCommand="vuln" if UNKNOWN: - os.execl("/bin/sh", "", "-c", "vuln") # $ getCommand="/bin/sh" getAPathArgument="/bin/sh" MISSING: getCommand="vuln" - -os.spawnl(os.P_WAIT, "/bin/sh", "", "-c", "vuln") # $ getCommand="/bin/sh" getAPathArgument="/bin/sh" MISSING: getCommand="vuln" + os.execl("/bin/sh", "", "-c", "vuln") # $ getCommand="/bin/sh" getAPathArgument="/bin/sh" getCommand="vuln" + os.execle("/bin/sh", "", "-c", "vuln", env) # $ getCommand="/bin/sh" getAPathArgument="/bin/sh" getCommand="vuln" + os.execlp("sh", "", "-c", "vuln") # $ getCommand="sh" getAPathArgument="sh" getCommand="vuln" + os.execlpe("sh", "", "-c", "vuln", env) # $ getCommand="sh" getAPathArgument="sh" getCommand="vuln" + os.execv("/bin/sh", ("", "-c", "vuln")) # $ getCommand="/bin/sh" getAPathArgument="/bin/sh" getCommand="vuln" + os.execvp("sh", ["", "-c", "vuln"]) # $ getCommand="sh" getAPathArgument="sh" getCommand="vuln" + + os.execl("/bin/sh", "", "--command", "not-vuln") # $ getCommand="/bin/sh" getAPathArgument="/bin/sh" + os.execle("/bin/sh", "", "-c", env) # $ getCommand="/bin/sh" getAPathArgument="/bin/sh" + os.execlpe("sh", "", "-c", env) # $ getCommand="sh" getAPathArgument="sh" + os.execv("not-a-shell", ["", "-c", "not-vuln"]) # $ getCommand="not-a-shell" getAPathArgument="not-a-shell" + +os.spawnl(os.P_WAIT, "/bin/sh", "", "-c", "vuln") # $ getCommand="/bin/sh" getAPathArgument="/bin/sh" getCommand="vuln" +os.spawnle(os.P_WAIT, "/bin/sh", "", "-c", "vuln", env) # $ getCommand="/bin/sh" getAPathArgument="/bin/sh" getCommand="vuln" +os.spawnle(os.P_WAIT, "/bin/sh", "", "-c", env) # $ getCommand="/bin/sh" getAPathArgument="/bin/sh" +os.spawnlpe(os.P_WAIT, "sh", "", "-c", "vuln", env) # $ getCommand="sh" getAPathArgument="sh" getCommand="vuln" +os.spawnlpe(os.P_WAIT, "sh", "", "-c", env) # $ getCommand="sh" getAPathArgument="sh" +os.spawnv(os.P_WAIT, "/bin/sh", ["", "-c", "vuln"]) # $ getCommand="/bin/sh" getAPathArgument="/bin/sh" getCommand="vuln" +os.spawnv(mode=os.P_WAIT, file="/bin/sh", args=["", "-c", "vuln"]) # $ getCommand="/bin/sh" getAPathArgument="/bin/sh" getCommand="vuln" +os.posix_spawn("/bin/sh", ["", "-c", "vuln"], env) # $ getCommand="/bin/sh" getAPathArgument="/bin/sh" getCommand="vuln" +os.posix_spawn(path="/bin/sh", argv=["", "-c", "vuln"], env=env) # $ getCommand="/bin/sh" getAPathArgument="/bin/sh" getCommand="vuln" +os.posix_spawnp("sh", ["", "-c", "vuln"], env) # $ getCommand="sh" getAPathArgument="sh" getCommand="vuln" ######################################## @@ -137,6 +168,10 @@ def os_members(): args = ["/bin/sh", "-c", "vuln"] subprocess.Popen(args) # $ getCommand=args +exec_args = ["", "-c", "vuln"] # $ MISSING: getCommand="vuln" +if UNKNOWN: + os.execv("/bin/sh", exec_args) # $ getCommand="/bin/sh" getAPathArgument="/bin/sh" + args = "" use_shell = False exe = "executable" diff --git a/python/ql/test/query-tests/Security/CWE-078-CommandInjection/CommandInjection.expected b/python/ql/test/query-tests/Security/CWE-078-CommandInjection/CommandInjection.expected index 1e75c67db66b..a4e88f9b75bf 100644 --- a/python/ql/test/query-tests/Security/CWE-078-CommandInjection/CommandInjection.expected +++ b/python/ql/test/query-tests/Security/CWE-078-CommandInjection/CommandInjection.expected @@ -1,3 +1,20 @@ +#select +| command_injection.py:13:15:13:27 | ControlFlowNode for BinaryExpr | command_injection.py:5:26:5:32 | ControlFlowNode for ImportMember | command_injection.py:13:15:13:27 | ControlFlowNode for BinaryExpr | This command line depends on a $@. | command_injection.py:5:26:5:32 | ControlFlowNode for ImportMember | user-provided value | +| command_injection.py:20:22:20:34 | ControlFlowNode for BinaryExpr | command_injection.py:5:26:5:32 | ControlFlowNode for ImportMember | command_injection.py:20:22:20:34 | ControlFlowNode for BinaryExpr | This command line depends on a $@. | command_injection.py:5:26:5:32 | ControlFlowNode for ImportMember | user-provided value | +| command_injection.py:26:23:26:25 | ControlFlowNode for cmd | command_injection.py:5:26:5:32 | ControlFlowNode for ImportMember | command_injection.py:26:23:26:25 | ControlFlowNode for cmd | This command line depends on a $@. | command_injection.py:5:26:5:32 | ControlFlowNode for ImportMember | user-provided value | +| command_injection.py:33:14:33:26 | ControlFlowNode for BinaryExpr | command_injection.py:5:26:5:32 | ControlFlowNode for ImportMember | command_injection.py:33:14:33:26 | ControlFlowNode for BinaryExpr | This command line depends on a $@. | command_injection.py:5:26:5:32 | ControlFlowNode for ImportMember | user-provided value | +| command_injection.py:41:15:41:21 | ControlFlowNode for command | command_injection.py:5:26:5:32 | ControlFlowNode for ImportMember | command_injection.py:41:15:41:21 | ControlFlowNode for command | This command line depends on a $@. | command_injection.py:5:26:5:32 | ControlFlowNode for ImportMember | user-provided value | +| command_injection.py:42:15:42:21 | ControlFlowNode for command | command_injection.py:5:26:5:32 | ControlFlowNode for ImportMember | command_injection.py:42:15:42:21 | ControlFlowNode for command | This command line depends on a $@. | command_injection.py:5:26:5:32 | ControlFlowNode for ImportMember | user-provided value | +| command_injection.py:55:15:55:21 | ControlFlowNode for command | command_injection.py:5:26:5:32 | ControlFlowNode for ImportMember | command_injection.py:55:15:55:21 | ControlFlowNode for command | This command line depends on a $@. | command_injection.py:5:26:5:32 | ControlFlowNode for ImportMember | user-provided value | +| command_injection.py:56:14:56:20 | ControlFlowNode for command | command_injection.py:5:26:5:32 | ControlFlowNode for ImportMember | command_injection.py:56:14:56:20 | ControlFlowNode for command | This command line depends on a $@. | command_injection.py:5:26:5:32 | ControlFlowNode for ImportMember | user-provided value | +| command_injection.py:57:21:57:27 | ControlFlowNode for command | command_injection.py:5:26:5:32 | ControlFlowNode for ImportMember | command_injection.py:57:21:57:27 | ControlFlowNode for command | This command line depends on a $@. | command_injection.py:5:26:5:32 | ControlFlowNode for ImportMember | user-provided value | +| command_injection.py:58:27:58:33 | ControlFlowNode for command | command_injection.py:5:26:5:32 | ControlFlowNode for ImportMember | command_injection.py:58:27:58:33 | ControlFlowNode for command | This command line depends on a $@. | command_injection.py:5:26:5:32 | ControlFlowNode for ImportMember | user-provided value | +| command_injection.py:59:20:59:26 | ControlFlowNode for command | command_injection.py:5:26:5:32 | ControlFlowNode for ImportMember | command_injection.py:59:20:59:26 | ControlFlowNode for command | This command line depends on a $@. | command_injection.py:5:26:5:32 | ControlFlowNode for ImportMember | user-provided value | +| command_injection.py:73:19:73:30 | ControlFlowNode for BinaryExpr | command_injection.py:5:26:5:32 | ControlFlowNode for ImportMember | command_injection.py:73:19:73:30 | ControlFlowNode for BinaryExpr | This command line depends on a $@. | command_injection.py:5:26:5:32 | ControlFlowNode for ImportMember | user-provided value | +| command_injection.py:80:19:80:30 | ControlFlowNode for BinaryExpr | command_injection.py:5:26:5:32 | ControlFlowNode for ImportMember | command_injection.py:80:19:80:30 | ControlFlowNode for BinaryExpr | This command line depends on a $@. | command_injection.py:5:26:5:32 | ControlFlowNode for ImportMember | user-provided value | +| command_injection.py:86:37:86:49 | ControlFlowNode for BinaryExpr | command_injection.py:5:26:5:32 | ControlFlowNode for ImportMember | command_injection.py:86:37:86:49 | ControlFlowNode for BinaryExpr | This command line depends on a $@. | command_injection.py:5:26:5:32 | ControlFlowNode for ImportMember | user-provided value | +| command_injection.py:92:33:92:45 | ControlFlowNode for BinaryExpr | command_injection.py:5:26:5:32 | ControlFlowNode for ImportMember | command_injection.py:92:33:92:45 | ControlFlowNode for BinaryExpr | This command line depends on a $@. | command_injection.py:5:26:5:32 | ControlFlowNode for ImportMember | user-provided value | +| command_injection.py:104:39:104:51 | ControlFlowNode for BinaryExpr | command_injection.py:5:26:5:32 | ControlFlowNode for ImportMember | command_injection.py:104:39:104:51 | ControlFlowNode for BinaryExpr | This command line depends on a $@. | command_injection.py:5:26:5:32 | ControlFlowNode for ImportMember | user-provided value | edges | command_injection.py:5:26:5:32 | ControlFlowNode for ImportMember | command_injection.py:5:26:5:32 | ControlFlowNode for request | provenance | | | command_injection.py:5:26:5:32 | ControlFlowNode for request | command_injection.py:11:13:11:19 | ControlFlowNode for request | provenance | | @@ -8,6 +25,9 @@ edges | command_injection.py:5:26:5:32 | ControlFlowNode for request | command_injection.py:54:15:54:21 | ControlFlowNode for request | provenance | | | command_injection.py:5:26:5:32 | ControlFlowNode for request | command_injection.py:71:12:71:18 | ControlFlowNode for request | provenance | | | command_injection.py:5:26:5:32 | ControlFlowNode for request | command_injection.py:78:12:78:18 | ControlFlowNode for request | provenance | | +| command_injection.py:5:26:5:32 | ControlFlowNode for request | command_injection.py:85:13:85:19 | ControlFlowNode for request | provenance | | +| command_injection.py:5:26:5:32 | ControlFlowNode for request | command_injection.py:91:13:91:19 | ControlFlowNode for request | provenance | | +| command_injection.py:5:26:5:32 | ControlFlowNode for request | command_injection.py:103:13:103:19 | ControlFlowNode for request | provenance | | | command_injection.py:11:5:11:9 | ControlFlowNode for files | command_injection.py:13:15:13:27 | ControlFlowNode for BinaryExpr | provenance | | | command_injection.py:11:13:11:19 | ControlFlowNode for request | command_injection.py:11:13:11:24 | ControlFlowNode for Attribute | provenance | AdditionalTaintStep | | command_injection.py:11:13:11:24 | ControlFlowNode for Attribute | command_injection.py:11:13:11:41 | ControlFlowNode for Attribute() | provenance | dict.get | @@ -45,6 +65,18 @@ edges | command_injection.py:78:12:78:18 | ControlFlowNode for request | command_injection.py:78:12:78:23 | ControlFlowNode for Attribute | provenance | AdditionalTaintStep | | command_injection.py:78:12:78:23 | ControlFlowNode for Attribute | command_injection.py:78:12:78:39 | ControlFlowNode for Attribute() | provenance | dict.get | | command_injection.py:78:12:78:39 | ControlFlowNode for Attribute() | command_injection.py:78:5:78:8 | ControlFlowNode for path | provenance | | +| command_injection.py:85:5:85:9 | ControlFlowNode for files | command_injection.py:86:37:86:49 | ControlFlowNode for BinaryExpr | provenance | | +| command_injection.py:85:13:85:19 | ControlFlowNode for request | command_injection.py:85:13:85:24 | ControlFlowNode for Attribute | provenance | AdditionalTaintStep | +| command_injection.py:85:13:85:24 | ControlFlowNode for Attribute | command_injection.py:85:13:85:41 | ControlFlowNode for Attribute() | provenance | dict.get | +| command_injection.py:85:13:85:41 | ControlFlowNode for Attribute() | command_injection.py:85:5:85:9 | ControlFlowNode for files | provenance | | +| command_injection.py:91:5:91:9 | ControlFlowNode for files | command_injection.py:92:33:92:45 | ControlFlowNode for BinaryExpr | provenance | | +| command_injection.py:91:13:91:19 | ControlFlowNode for request | command_injection.py:91:13:91:24 | ControlFlowNode for Attribute | provenance | AdditionalTaintStep | +| command_injection.py:91:13:91:24 | ControlFlowNode for Attribute | command_injection.py:91:13:91:41 | ControlFlowNode for Attribute() | provenance | dict.get | +| command_injection.py:91:13:91:41 | ControlFlowNode for Attribute() | command_injection.py:91:5:91:9 | ControlFlowNode for files | provenance | | +| command_injection.py:103:5:103:9 | ControlFlowNode for files | command_injection.py:104:39:104:51 | ControlFlowNode for BinaryExpr | provenance | | +| command_injection.py:103:13:103:19 | ControlFlowNode for request | command_injection.py:103:13:103:24 | ControlFlowNode for Attribute | provenance | AdditionalTaintStep | +| command_injection.py:103:13:103:24 | ControlFlowNode for Attribute | command_injection.py:103:13:103:41 | ControlFlowNode for Attribute() | provenance | dict.get | +| command_injection.py:103:13:103:41 | ControlFlowNode for Attribute() | command_injection.py:103:5:103:9 | ControlFlowNode for files | provenance | | nodes | command_injection.py:5:26:5:32 | ControlFlowNode for ImportMember | semmle.label | ControlFlowNode for ImportMember | | command_injection.py:5:26:5:32 | ControlFlowNode for request | semmle.label | ControlFlowNode for request | @@ -93,18 +125,19 @@ nodes | command_injection.py:78:12:78:23 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute | | command_injection.py:78:12:78:39 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() | | command_injection.py:80:19:80:30 | ControlFlowNode for BinaryExpr | semmle.label | ControlFlowNode for BinaryExpr | +| command_injection.py:85:5:85:9 | ControlFlowNode for files | semmle.label | ControlFlowNode for files | +| command_injection.py:85:13:85:19 | ControlFlowNode for request | semmle.label | ControlFlowNode for request | +| command_injection.py:85:13:85:24 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute | +| command_injection.py:85:13:85:41 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() | +| command_injection.py:86:37:86:49 | ControlFlowNode for BinaryExpr | semmle.label | ControlFlowNode for BinaryExpr | +| command_injection.py:91:5:91:9 | ControlFlowNode for files | semmle.label | ControlFlowNode for files | +| command_injection.py:91:13:91:19 | ControlFlowNode for request | semmle.label | ControlFlowNode for request | +| command_injection.py:91:13:91:24 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute | +| command_injection.py:91:13:91:41 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() | +| command_injection.py:92:33:92:45 | ControlFlowNode for BinaryExpr | semmle.label | ControlFlowNode for BinaryExpr | +| command_injection.py:103:5:103:9 | ControlFlowNode for files | semmle.label | ControlFlowNode for files | +| command_injection.py:103:13:103:19 | ControlFlowNode for request | semmle.label | ControlFlowNode for request | +| command_injection.py:103:13:103:24 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute | +| command_injection.py:103:13:103:41 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() | +| command_injection.py:104:39:104:51 | ControlFlowNode for BinaryExpr | semmle.label | ControlFlowNode for BinaryExpr | subpaths -#select -| command_injection.py:13:15:13:27 | ControlFlowNode for BinaryExpr | command_injection.py:5:26:5:32 | ControlFlowNode for ImportMember | command_injection.py:13:15:13:27 | ControlFlowNode for BinaryExpr | This command line depends on a $@. | command_injection.py:5:26:5:32 | ControlFlowNode for ImportMember | user-provided value | -| command_injection.py:20:22:20:34 | ControlFlowNode for BinaryExpr | command_injection.py:5:26:5:32 | ControlFlowNode for ImportMember | command_injection.py:20:22:20:34 | ControlFlowNode for BinaryExpr | This command line depends on a $@. | command_injection.py:5:26:5:32 | ControlFlowNode for ImportMember | user-provided value | -| command_injection.py:26:23:26:25 | ControlFlowNode for cmd | command_injection.py:5:26:5:32 | ControlFlowNode for ImportMember | command_injection.py:26:23:26:25 | ControlFlowNode for cmd | This command line depends on a $@. | command_injection.py:5:26:5:32 | ControlFlowNode for ImportMember | user-provided value | -| command_injection.py:33:14:33:26 | ControlFlowNode for BinaryExpr | command_injection.py:5:26:5:32 | ControlFlowNode for ImportMember | command_injection.py:33:14:33:26 | ControlFlowNode for BinaryExpr | This command line depends on a $@. | command_injection.py:5:26:5:32 | ControlFlowNode for ImportMember | user-provided value | -| command_injection.py:41:15:41:21 | ControlFlowNode for command | command_injection.py:5:26:5:32 | ControlFlowNode for ImportMember | command_injection.py:41:15:41:21 | ControlFlowNode for command | This command line depends on a $@. | command_injection.py:5:26:5:32 | ControlFlowNode for ImportMember | user-provided value | -| command_injection.py:42:15:42:21 | ControlFlowNode for command | command_injection.py:5:26:5:32 | ControlFlowNode for ImportMember | command_injection.py:42:15:42:21 | ControlFlowNode for command | This command line depends on a $@. | command_injection.py:5:26:5:32 | ControlFlowNode for ImportMember | user-provided value | -| command_injection.py:55:15:55:21 | ControlFlowNode for command | command_injection.py:5:26:5:32 | ControlFlowNode for ImportMember | command_injection.py:55:15:55:21 | ControlFlowNode for command | This command line depends on a $@. | command_injection.py:5:26:5:32 | ControlFlowNode for ImportMember | user-provided value | -| command_injection.py:56:14:56:20 | ControlFlowNode for command | command_injection.py:5:26:5:32 | ControlFlowNode for ImportMember | command_injection.py:56:14:56:20 | ControlFlowNode for command | This command line depends on a $@. | command_injection.py:5:26:5:32 | ControlFlowNode for ImportMember | user-provided value | -| command_injection.py:57:21:57:27 | ControlFlowNode for command | command_injection.py:5:26:5:32 | ControlFlowNode for ImportMember | command_injection.py:57:21:57:27 | ControlFlowNode for command | This command line depends on a $@. | command_injection.py:5:26:5:32 | ControlFlowNode for ImportMember | user-provided value | -| command_injection.py:58:27:58:33 | ControlFlowNode for command | command_injection.py:5:26:5:32 | ControlFlowNode for ImportMember | command_injection.py:58:27:58:33 | ControlFlowNode for command | This command line depends on a $@. | command_injection.py:5:26:5:32 | ControlFlowNode for ImportMember | user-provided value | -| command_injection.py:59:20:59:26 | ControlFlowNode for command | command_injection.py:5:26:5:32 | ControlFlowNode for ImportMember | command_injection.py:59:20:59:26 | ControlFlowNode for command | This command line depends on a $@. | command_injection.py:5:26:5:32 | ControlFlowNode for ImportMember | user-provided value | -| command_injection.py:73:19:73:30 | ControlFlowNode for BinaryExpr | command_injection.py:5:26:5:32 | ControlFlowNode for ImportMember | command_injection.py:73:19:73:30 | ControlFlowNode for BinaryExpr | This command line depends on a $@. | command_injection.py:5:26:5:32 | ControlFlowNode for ImportMember | user-provided value | -| command_injection.py:80:19:80:30 | ControlFlowNode for BinaryExpr | command_injection.py:5:26:5:32 | ControlFlowNode for ImportMember | command_injection.py:80:19:80:30 | ControlFlowNode for BinaryExpr | This command line depends on a $@. | command_injection.py:5:26:5:32 | ControlFlowNode for ImportMember | user-provided value | diff --git a/python/ql/test/query-tests/Security/CWE-078-CommandInjection/command_injection.py b/python/ql/test/query-tests/Security/CWE-078-CommandInjection/command_injection.py index fb4f09c1c2a8..8135cb21f9c5 100644 --- a/python/ql/test/query-tests/Security/CWE-078-CommandInjection/command_injection.py +++ b/python/ql/test/query-tests/Security/CWE-078-CommandInjection/command_injection.py @@ -78,3 +78,33 @@ def restricted_characters(): path = request.args.get('path', '') if re.match(r'^[a-zA-Z0-9_-]+$', path): os.system("ls " + path) # $ Alert SPURIOUS: result=BAD + + +@app.route("/exec-shell") +def exec_shell(): + files = request.args.get('files', '') + os.execl("/bin/sh", "sh", "-c", "ls " + files) # $ Alert result=BAD result=OK + + +@app.route("/subprocess-shell") +def subprocess_shell(): + files = request.args.get('files', '') + subprocess.run(["sh", "-c", "ls " + files]) # $ Alert result=BAD result=OK + + +@app.route("/safe-subprocess-arguments") +def safe_subprocess_arguments(): + files = request.args.get('files', '') + subprocess.run(["ls", files]) # $ result=OK + + +@app.route("/execv-shell") +def execv_shell(): + files = request.args.get('files', '') + os.execve("/bin/sh", ["sh", "-c", "ls " + files], {}) # $ Alert result=BAD result=OK + + +@app.route("/legacy-shell-helper") +def legacy_shell_helper(): + files = request.args.get('files', '') + subprocess.getoutput(["sh", "-c", "ls " + files]) # $ result=OK diff --git a/python/ql/test/query-tests/Security/CWE-078-UnsafeShellCommandConstruction/UnsafeShellCommandConstruction.expected b/python/ql/test/query-tests/Security/CWE-078-UnsafeShellCommandConstruction/UnsafeShellCommandConstruction.expected index 3bc075d618be..314e8c79fc5b 100644 --- a/python/ql/test/query-tests/Security/CWE-078-UnsafeShellCommandConstruction/UnsafeShellCommandConstruction.expected +++ b/python/ql/test/query-tests/Security/CWE-078-UnsafeShellCommandConstruction/UnsafeShellCommandConstruction.expected @@ -1,3 +1,17 @@ +#select +| src/unsafe_shell_test.py:5:15:5:28 | ControlFlowNode for BinaryExpr | src/unsafe_shell_test.py:4:22:4:25 | ControlFlowNode for name | src/unsafe_shell_test.py:5:25:5:28 | ControlFlowNode for name | This string concatenation which depends on $@ is later used in a $@. | src/unsafe_shell_test.py:4:22:4:25 | ControlFlowNode for name | library input | src/unsafe_shell_test.py:5:5:5:29 | ControlFlowNode for Attribute() | shell command | +| src/unsafe_shell_test.py:8:15:8:28 | ControlFlowNode for Fstring | src/unsafe_shell_test.py:4:22:4:25 | ControlFlowNode for name | src/unsafe_shell_test.py:8:23:8:26 | ControlFlowNode for name | This f-string which depends on $@ is later used in a $@. | src/unsafe_shell_test.py:4:22:4:25 | ControlFlowNode for name | library input | src/unsafe_shell_test.py:8:5:8:29 | ControlFlowNode for Attribute() | shell command | +| src/unsafe_shell_test.py:11:15:11:38 | ControlFlowNode for BinaryExpr | src/unsafe_shell_test.py:4:22:4:25 | ControlFlowNode for name | src/unsafe_shell_test.py:11:25:11:38 | ControlFlowNode for Attribute() | This string concatenation which depends on $@ is later used in a $@. | src/unsafe_shell_test.py:4:22:4:25 | ControlFlowNode for name | library input | src/unsafe_shell_test.py:11:5:11:39 | ControlFlowNode for Attribute() | shell command | +| src/unsafe_shell_test.py:14:15:14:40 | ControlFlowNode for BinaryExpr | src/unsafe_shell_test.py:4:22:4:25 | ControlFlowNode for name | src/unsafe_shell_test.py:14:25:14:40 | ControlFlowNode for Attribute() | This string concatenation which depends on $@ is later used in a $@. | src/unsafe_shell_test.py:4:22:4:25 | ControlFlowNode for name | library input | src/unsafe_shell_test.py:14:5:14:41 | ControlFlowNode for Attribute() | shell command | +| src/unsafe_shell_test.py:17:15:17:36 | ControlFlowNode for Attribute() | src/unsafe_shell_test.py:4:22:4:25 | ControlFlowNode for name | src/unsafe_shell_test.py:17:32:17:35 | ControlFlowNode for name | This formatted string which depends on $@ is later used in a $@. | src/unsafe_shell_test.py:4:22:4:25 | ControlFlowNode for name | library input | src/unsafe_shell_test.py:17:5:17:37 | ControlFlowNode for Attribute() | shell command | +| src/unsafe_shell_test.py:20:15:20:30 | ControlFlowNode for BinaryExpr | src/unsafe_shell_test.py:4:22:4:25 | ControlFlowNode for name | src/unsafe_shell_test.py:20:27:20:30 | ControlFlowNode for name | This formatted string which depends on $@ is later used in a $@. | src/unsafe_shell_test.py:4:22:4:25 | ControlFlowNode for name | library input | src/unsafe_shell_test.py:20:5:20:31 | ControlFlowNode for Attribute() | shell command | +| src/unsafe_shell_test.py:29:20:29:33 | ControlFlowNode for BinaryExpr | src/unsafe_shell_test.py:26:20:26:23 | ControlFlowNode for name | src/unsafe_shell_test.py:29:30:29:33 | ControlFlowNode for name | This string concatenation which depends on $@ is later used in a $@. | src/unsafe_shell_test.py:26:20:26:23 | ControlFlowNode for name | library input | src/unsafe_shell_test.py:29:5:29:46 | ControlFlowNode for Attribute() | shell command | +| src/unsafe_shell_test.py:39:20:39:33 | ControlFlowNode for BinaryExpr | src/unsafe_shell_test.py:36:22:36:25 | ControlFlowNode for name | src/unsafe_shell_test.py:39:30:39:33 | ControlFlowNode for name | This string concatenation which depends on $@ is later used in a $@. | src/unsafe_shell_test.py:36:22:36:25 | ControlFlowNode for name | library input | src/unsafe_shell_test.py:39:5:39:46 | ControlFlowNode for Attribute() | shell command | +| src/unsafe_shell_test.py:42:24:42:34 | ControlFlowNode for BinaryExpr | src/unsafe_shell_test.py:36:22:36:25 | ControlFlowNode for name | src/unsafe_shell_test.py:42:34:42:34 | ControlFlowNode for x | This string concatenation which depends on $@ is later used in a $@. | src/unsafe_shell_test.py:36:22:36:25 | ControlFlowNode for name | library input | src/unsafe_shell_test.py:42:9:42:47 | ControlFlowNode for Attribute() | shell command | +| src/unsafe_shell_test.py:49:33:49:46 | ControlFlowNode for BinaryExpr | src/unsafe_shell_test.py:48:32:48:35 | ControlFlowNode for name | src/unsafe_shell_test.py:49:43:49:46 | ControlFlowNode for name | This string concatenation which depends on $@ is later used in a $@. | src/unsafe_shell_test.py:48:32:48:35 | ControlFlowNode for name | library input | src/unsafe_shell_test.py:49:5:49:48 | ControlFlowNode for Attribute() | shell command | +| src/unsafe_shell_test.py:54:41:54:54 | ControlFlowNode for BinaryExpr | src/unsafe_shell_test.py:48:32:48:35 | ControlFlowNode for name | src/unsafe_shell_test.py:54:51:54:54 | ControlFlowNode for name | This string concatenation which depends on $@ is later used in a $@. | src/unsafe_shell_test.py:48:32:48:35 | ControlFlowNode for name | library input | src/unsafe_shell_test.py:54:9:54:55 | ControlFlowNode for Attribute() | shell command | +| src/unsafe_shell_test.py:55:53:55:66 | ControlFlowNode for BinaryExpr | src/unsafe_shell_test.py:48:32:48:35 | ControlFlowNode for name | src/unsafe_shell_test.py:55:63:55:66 | ControlFlowNode for name | This string concatenation which depends on $@ is later used in a $@. | src/unsafe_shell_test.py:48:32:48:35 | ControlFlowNode for name | library input | src/unsafe_shell_test.py:55:9:55:67 | ControlFlowNode for Attribute() | shell command | +| src/unsafe_shell_test.py:56:48:56:61 | ControlFlowNode for BinaryExpr | src/unsafe_shell_test.py:48:32:48:35 | ControlFlowNode for name | src/unsafe_shell_test.py:56:58:56:61 | ControlFlowNode for name | This string concatenation which depends on $@ is later used in a $@. | src/unsafe_shell_test.py:48:32:48:35 | ControlFlowNode for name | library input | src/unsafe_shell_test.py:56:9:56:67 | ControlFlowNode for Attribute() | shell command | edges | src/unsafe_shell_test.py:4:22:4:25 | ControlFlowNode for name | src/unsafe_shell_test.py:5:25:5:28 | ControlFlowNode for name | provenance | | | src/unsafe_shell_test.py:4:22:4:25 | ControlFlowNode for name | src/unsafe_shell_test.py:8:23:8:26 | ControlFlowNode for name | provenance | | @@ -13,6 +27,10 @@ edges | src/unsafe_shell_test.py:36:22:36:25 | ControlFlowNode for name | src/unsafe_shell_test.py:44:20:44:23 | ControlFlowNode for name | provenance | | | src/unsafe_shell_test.py:41:24:41:24 | ControlFlowNode for x | src/unsafe_shell_test.py:42:34:42:34 | ControlFlowNode for x | provenance | | | src/unsafe_shell_test.py:44:20:44:23 | ControlFlowNode for name | src/unsafe_shell_test.py:41:24:41:24 | ControlFlowNode for x | provenance | | +| src/unsafe_shell_test.py:48:32:48:35 | ControlFlowNode for name | src/unsafe_shell_test.py:49:43:49:46 | ControlFlowNode for name | provenance | | +| src/unsafe_shell_test.py:48:32:48:35 | ControlFlowNode for name | src/unsafe_shell_test.py:54:51:54:54 | ControlFlowNode for name | provenance | | +| src/unsafe_shell_test.py:48:32:48:35 | ControlFlowNode for name | src/unsafe_shell_test.py:55:63:55:66 | ControlFlowNode for name | provenance | | +| src/unsafe_shell_test.py:48:32:48:35 | ControlFlowNode for name | src/unsafe_shell_test.py:56:58:56:61 | ControlFlowNode for name | provenance | | nodes | src/unsafe_shell_test.py:4:22:4:25 | ControlFlowNode for name | semmle.label | ControlFlowNode for name | | src/unsafe_shell_test.py:5:25:5:28 | ControlFlowNode for name | semmle.label | ControlFlowNode for name | @@ -31,14 +49,9 @@ nodes | src/unsafe_shell_test.py:41:24:41:24 | ControlFlowNode for x | semmle.label | ControlFlowNode for x | | src/unsafe_shell_test.py:42:34:42:34 | ControlFlowNode for x | semmle.label | ControlFlowNode for x | | src/unsafe_shell_test.py:44:20:44:23 | ControlFlowNode for name | semmle.label | ControlFlowNode for name | +| src/unsafe_shell_test.py:48:32:48:35 | ControlFlowNode for name | semmle.label | ControlFlowNode for name | +| src/unsafe_shell_test.py:49:43:49:46 | ControlFlowNode for name | semmle.label | ControlFlowNode for name | +| src/unsafe_shell_test.py:54:51:54:54 | ControlFlowNode for name | semmle.label | ControlFlowNode for name | +| src/unsafe_shell_test.py:55:63:55:66 | ControlFlowNode for name | semmle.label | ControlFlowNode for name | +| src/unsafe_shell_test.py:56:58:56:61 | ControlFlowNode for name | semmle.label | ControlFlowNode for name | subpaths -#select -| src/unsafe_shell_test.py:5:15:5:28 | ControlFlowNode for BinaryExpr | src/unsafe_shell_test.py:4:22:4:25 | ControlFlowNode for name | src/unsafe_shell_test.py:5:25:5:28 | ControlFlowNode for name | This string concatenation which depends on $@ is later used in a $@. | src/unsafe_shell_test.py:4:22:4:25 | ControlFlowNode for name | library input | src/unsafe_shell_test.py:5:5:5:29 | ControlFlowNode for Attribute() | shell command | -| src/unsafe_shell_test.py:8:15:8:28 | ControlFlowNode for Fstring | src/unsafe_shell_test.py:4:22:4:25 | ControlFlowNode for name | src/unsafe_shell_test.py:8:23:8:26 | ControlFlowNode for name | This f-string which depends on $@ is later used in a $@. | src/unsafe_shell_test.py:4:22:4:25 | ControlFlowNode for name | library input | src/unsafe_shell_test.py:8:5:8:29 | ControlFlowNode for Attribute() | shell command | -| src/unsafe_shell_test.py:11:15:11:38 | ControlFlowNode for BinaryExpr | src/unsafe_shell_test.py:4:22:4:25 | ControlFlowNode for name | src/unsafe_shell_test.py:11:25:11:38 | ControlFlowNode for Attribute() | This string concatenation which depends on $@ is later used in a $@. | src/unsafe_shell_test.py:4:22:4:25 | ControlFlowNode for name | library input | src/unsafe_shell_test.py:11:5:11:39 | ControlFlowNode for Attribute() | shell command | -| src/unsafe_shell_test.py:14:15:14:40 | ControlFlowNode for BinaryExpr | src/unsafe_shell_test.py:4:22:4:25 | ControlFlowNode for name | src/unsafe_shell_test.py:14:25:14:40 | ControlFlowNode for Attribute() | This string concatenation which depends on $@ is later used in a $@. | src/unsafe_shell_test.py:4:22:4:25 | ControlFlowNode for name | library input | src/unsafe_shell_test.py:14:5:14:41 | ControlFlowNode for Attribute() | shell command | -| src/unsafe_shell_test.py:17:15:17:36 | ControlFlowNode for Attribute() | src/unsafe_shell_test.py:4:22:4:25 | ControlFlowNode for name | src/unsafe_shell_test.py:17:32:17:35 | ControlFlowNode for name | This formatted string which depends on $@ is later used in a $@. | src/unsafe_shell_test.py:4:22:4:25 | ControlFlowNode for name | library input | src/unsafe_shell_test.py:17:5:17:37 | ControlFlowNode for Attribute() | shell command | -| src/unsafe_shell_test.py:20:15:20:30 | ControlFlowNode for BinaryExpr | src/unsafe_shell_test.py:4:22:4:25 | ControlFlowNode for name | src/unsafe_shell_test.py:20:27:20:30 | ControlFlowNode for name | This formatted string which depends on $@ is later used in a $@. | src/unsafe_shell_test.py:4:22:4:25 | ControlFlowNode for name | library input | src/unsafe_shell_test.py:20:5:20:31 | ControlFlowNode for Attribute() | shell command | -| src/unsafe_shell_test.py:29:20:29:33 | ControlFlowNode for BinaryExpr | src/unsafe_shell_test.py:26:20:26:23 | ControlFlowNode for name | src/unsafe_shell_test.py:29:30:29:33 | ControlFlowNode for name | This string concatenation which depends on $@ is later used in a $@. | src/unsafe_shell_test.py:26:20:26:23 | ControlFlowNode for name | library input | src/unsafe_shell_test.py:29:5:29:46 | ControlFlowNode for Attribute() | shell command | -| src/unsafe_shell_test.py:39:20:39:33 | ControlFlowNode for BinaryExpr | src/unsafe_shell_test.py:36:22:36:25 | ControlFlowNode for name | src/unsafe_shell_test.py:39:30:39:33 | ControlFlowNode for name | This string concatenation which depends on $@ is later used in a $@. | src/unsafe_shell_test.py:36:22:36:25 | ControlFlowNode for name | library input | src/unsafe_shell_test.py:39:5:39:46 | ControlFlowNode for Attribute() | shell command | -| src/unsafe_shell_test.py:42:24:42:34 | ControlFlowNode for BinaryExpr | src/unsafe_shell_test.py:36:22:36:25 | ControlFlowNode for name | src/unsafe_shell_test.py:42:34:42:34 | ControlFlowNode for x | This string concatenation which depends on $@ is later used in a $@. | src/unsafe_shell_test.py:36:22:36:25 | ControlFlowNode for name | library input | src/unsafe_shell_test.py:42:9:42:47 | ControlFlowNode for Attribute() | shell command | diff --git a/python/ql/test/query-tests/Security/CWE-078-UnsafeShellCommandConstruction/src/unsafe_shell_test.py b/python/ql/test/query-tests/Security/CWE-078-UnsafeShellCommandConstruction/src/unsafe_shell_test.py index 22c7513f02d7..7a4e18caec4b 100644 --- a/python/ql/test/query-tests/Security/CWE-078-UnsafeShellCommandConstruction/src/unsafe_shell_test.py +++ b/python/ql/test/query-tests/Security/CWE-078-UnsafeShellCommandConstruction/src/unsafe_shell_test.py @@ -45,9 +45,22 @@ def indirect(flag, x): subprocess.Popen("ping " + name, shell=unknownValue) # OK - shell assumed to be False +def explicit_shell_interpreter(name): # $ Source + subprocess.run(["sh", "-c", "ping " + name]) # $ Alert result=BAD + # With `shell=True`, only the first sequence element is the outer shell's command. + subprocess.run(["sh", "-c", "ping " + name], shell=True) + + if unknownValue: + os.execl("/bin/sh", "sh", "-c", "ping " + name) # $ Alert result=BAD + os.spawnl(os.P_WAIT, "/bin/sh", "sh", "-c", "ping " + name) # $ Alert result=BAD + os.posix_spawn("/bin/sh", ["sh", "-c", "ping " + name], {}) # $ Alert result=BAD + def intentional(command): os.system("fish -ic " + command) # $ result=OK - intentional +def legacy_shell_helper(name): + subprocess.getstatusoutput(["sh", "-c", "ping " + name]) + import shlex def unsafe_shell_sanitized(name): os.system("ping " + shlex.quote(name)) # $ result=OK - sanitized