diff --git a/python/ql/lib/semmle/python/Concepts.qll b/python/ql/lib/semmle/python/Concepts.qll index b4d2a64cb45..8ea64a1373c 100644 --- a/python/ql/lib/semmle/python/Concepts.qll +++ b/python/ql/lib/semmle/python/Concepts.qll @@ -19,6 +19,9 @@ private import semmle.python.security.internal.EncryptionKeySizes * extend `SystemCommandExecution::Range` instead. */ class SystemCommandExecution extends DataFlow::Node instanceof SystemCommandExecution::Range { + /** Holds if a shell interprets `arg`. */ + predicate isShellInterpreted(DataFlow::Node arg) { super.isShellInterpreted(arg) } + /** Gets the argument that specifies the command to be executed. */ DataFlow::Node getCommand() { result = super.getCommand() } } @@ -35,6 +38,9 @@ module SystemCommandExecution { abstract class Range extends DataFlow::Node { /** Gets the argument that specifies the command to be executed. */ abstract DataFlow::Node getCommand(); + + /** Holds if a shell interprets `arg`. */ + predicate isShellInterpreted(DataFlow::Node arg) { none() } } } diff --git a/python/ql/lib/semmle/python/frameworks/Fabric.qll b/python/ql/lib/semmle/python/frameworks/Fabric.qll index 5fd9d2afe18..31511dbeaf4 100644 --- a/python/ql/lib/semmle/python/frameworks/Fabric.qll +++ b/python/ql/lib/semmle/python/frameworks/Fabric.qll @@ -43,13 +43,19 @@ private module FabricV1 { * - https://docs.fabfile.org/en/1.14/api/core/operations.html#fabric.operations.run * - https://docs.fabfile.org/en/1.14/api/core/operations.html#fabric.operations.sudo */ - private class FabricApiLocalRunSudoCall extends SystemCommandExecution::Range, - DataFlow::CallCfgNode { + private class FabricApiLocalRunSudoCall extends SystemCommandExecution::Range, API::CallNode { FabricApiLocalRunSudoCall() { this = api().getMember(["local", "run", "sudo"]).getACall() } override DataFlow::Node getCommand() { result = [this.getArg(0), this.getArgByName("command")] } + + override predicate isShellInterpreted(DataFlow::Node arg) { + arg = this.getCommand() and + // defaults to running in a shell + not this.getParameter(1, "shell").asSink().asExpr().(ImmutableLiteral).booleanValue() = + false // TODO: Test this in unsafe-shell-command-construction - and add tracking as a separate step. + } } } } @@ -161,6 +167,8 @@ private module FabricV2 { override DataFlow::Node getCommand() { result = [this.getArg(0), this.getArgByName("command")] } + + override predicate isShellInterpreted(DataFlow::Node arg) { arg = this.getCommand() } } // ------------------------------------------------------------------------- @@ -243,6 +251,8 @@ private module FabricV2 { override DataFlow::Node getCommand() { result = [this.getArg(0), this.getArgByName("command")] } + + override predicate isShellInterpreted(DataFlow::Node arg) { arg = this.getCommand() } } /** diff --git a/python/ql/lib/semmle/python/frameworks/Invoke.qll b/python/ql/lib/semmle/python/frameworks/Invoke.qll index b1ceb078907..30637610ac9 100644 --- a/python/ql/lib/semmle/python/frameworks/Invoke.qll +++ b/python/ql/lib/semmle/python/frameworks/Invoke.qll @@ -81,5 +81,7 @@ private module Invoke { override DataFlow::Node getCommand() { result in [this.getArg(0), this.getArgByName("command")] } + + override predicate isShellInterpreted(DataFlow::Node arg) { arg = this.getCommand() } } } diff --git a/python/ql/lib/semmle/python/frameworks/Stdlib.qll b/python/ql/lib/semmle/python/frameworks/Stdlib.qll index 3fce979c147..d3512aca668 100644 --- a/python/ql/lib/semmle/python/frameworks/Stdlib.qll +++ b/python/ql/lib/semmle/python/frameworks/Stdlib.qll @@ -1060,7 +1060,11 @@ private module StdlibPrivate { private class OsSystemCall extends SystemCommandExecution::Range, DataFlow::CallCfgNode { OsSystemCall() { this = os().getMember("system").getACall() } - override DataFlow::Node getCommand() { result = this.getArg(0) } + override DataFlow::Node getCommand() { + result in [this.getArg(0), this.getArgByName("command")] + } + + override predicate isShellInterpreted(DataFlow::Node arg) { arg = this.getCommand() } } /** @@ -1071,7 +1075,7 @@ private module StdlibPrivate { * Although deprecated since version 2.6, they still work in 2.7. * See https://docs.python.org/2.7/library/os.html#os.popen2 */ - private class OsPopenCall extends SystemCommandExecution::Range, DataFlow::CallCfgNode { + private class OsPopenCall extends SystemCommandExecution::Range, API::CallNode { string name; OsPopenCall() { @@ -1085,6 +1089,8 @@ private module StdlibPrivate { not name = "popen" and result = this.getArgByName("cmd") } + + override predicate isShellInterpreted(DataFlow::Node arg) { arg = this.getCommand() } } /** @@ -1103,6 +1109,10 @@ private module StdlibPrivate { override DataFlow::Node getCommand() { result = this.getArg(0) } override DataFlow::Node getAPathArgument() { result = this.getCommand() } + + override predicate isShellInterpreted(DataFlow::Node arg) { + none() // this is a safe API. + } } /** @@ -1129,6 +1139,10 @@ private module StdlibPrivate { } override DataFlow::Node getAPathArgument() { result = this.getCommand() } + + override predicate isShellInterpreted(DataFlow::Node arg) { + none() // this is a safe API. + } } /** @@ -1142,6 +1156,10 @@ private module StdlibPrivate { override DataFlow::Node getCommand() { result in [this.getArg(0), this.getArgByName("path")] } override DataFlow::Node getAPathArgument() { result = this.getCommand() } + + override predicate isShellInterpreted(DataFlow::Node arg) { + none() // this is a safe API. + } } /** An additional taint step for calls to `os.path.join` */ @@ -1186,6 +1204,7 @@ private module StdlibPrivate { } private boolean get_shell_arg_value() { + // TODO: API-node this thing - with added tests for unsafe-shell-command-construction not exists(this.get_shell_arg()) and result = false or @@ -1239,6 +1258,11 @@ private module StdlibPrivate { ) ) } + + override predicate isShellInterpreted(DataFlow::Node arg) { + arg = this.get_executable_arg() and + this.get_shell_arg_value() = true + } } // --------------------------------------------------------------------------- @@ -1385,6 +1409,8 @@ private module StdlibPrivate { } override DataFlow::Node getCommand() { result in [this.getArg(0), this.getArgByName("cmd")] } + + override predicate isShellInterpreted(DataFlow::Node arg) { arg = this.getCommand() } } // --------------------------------------------------------------------------- @@ -1401,6 +1427,8 @@ private module StdlibPrivate { PlatformPopenCall() { this = platform().getMember("popen").getACall() } override DataFlow::Node getCommand() { result in [this.getArg(0), this.getArgByName("cmd")] } + + override predicate isShellInterpreted(DataFlow::Node arg) { arg = this.getCommand() } } // ---------------------------------------------------------------------------