hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2025-12-20 10:46:30 +01:00
Code Issues Packages Projects Releases Wiki Activity

use more API-nodes to model subprocess.run (and friends)

Browse Source
This commit is contained in:
erik-krogh
2023-02-02 11:23:27 +01:00
parent bce83bfc4e
commit d228cf0e7b
3 changed files with 25 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files

41
python/ql/lib/semmle/python/frameworks/Stdlib.qll
View File

@@ -1185,7 +1185,7 @@ private module StdlibPrivate {
* See https://docs.python.org/3.8/library/subprocess.html#subprocess.Popen
* ref: https://docs.python.org/3/library/subprocess.html#legacy-shell-invocation-functions
*/
private class SubprocessPopenCall extends SystemCommandExecution::Range, DataFlow::CallCfgNode {
private class SubprocessPopenCall extends SystemCommandExecution::Range, API::CallNode {
SubprocessPopenCall() {
exists(string name |
name in [
@@ -1195,44 +1195,33 @@ private module StdlibPrivate {
)
}
/** Gets the ControlFlowNode for the `args` argument, if any. */
private DataFlow::Node get_args_arg() { result in [this.getArg(0), this.getArgByName("args")] }
/** Gets the API-node for the `args` argument, if any. */
private API::Node get_args_arg() { result = this.getParameter(0, "args") }
/** Gets the ControlFlowNode for the `shell` argument, if any. */
private DataFlow::Node get_shell_arg() {
result in [this.getArg(8), this.getArgByName("shell")]
}
/** Gets the API-node for the `shell` argument, if any. */
private API::Node get_shell_arg() { result = this.getParameter(8, "shell") }
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
exists(DataFlow::Node shell_arg | shell_arg = this.get_shell_arg() |
result = shell_arg.asCfgNode().getNode().(ImmutableLiteral).booleanValue()
or
// TODO: Track the "shell" argument to determine possible values
not shell_arg.asCfgNode().getNode() instanceof ImmutableLiteral and
(
result = true
or
result = false
)
)
result =
this.get_shell_arg().getAValueReachingSink().asExpr().(ImmutableLiteral).booleanValue()
or
not this.get_shell_arg().getAValueReachingSink().asExpr() instanceof ImmutableLiteral and
result = [true, false]
}
/** Gets the ControlFlowNode for the `executable` argument, if any. */
private DataFlow::Node get_executable_arg() {
result in [this.getArg(2), this.getArgByName("executable")]
}
/** Gets the API-node for the `executable` argument, if any. */
private API::Node get_executable_arg() { result = this.getParameter(2, "executable") }
override DataFlow::Node getCommand() {
// TODO: Track arguments ("args" and "shell")
// TODO: Handle using `args=["sh", "-c", <user-input>]`
result = this.get_executable_arg()
result = this.get_executable_arg().asSink()
or
exists(DataFlow::Node arg_args, boolean shell |
arg_args = this.get_args_arg() and
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
@@ -1260,7 +1249,7 @@ private module StdlibPrivate {
}
override predicate isShellInterpreted(DataFlow::Node arg) {
arg = [this.get_executable_arg(), this.get_args_arg()] and
arg = [this.get_executable_arg(), this.get_args_arg()].asSink() and
this.get_shell_arg_value() = true
}
}

7
python/ql/test/query-tests/Security/CWE-078-UnsafeShellCommandConstruction/UnsafeShellCommandConstruction.expected
View File

@@ -8,6 +8,9 @@ edges
| src/unsafe_shell_test.py:14:34:14:39 | ControlFlowNode for List | src/unsafe_shell_test.py:14:25:14:40 | ControlFlowNode for Attribute() |
| src/unsafe_shell_test.py:26:20:26:23 | ControlFlowNode for name | src/unsafe_shell_test.py:29:30:29:33 | ControlFlowNode for name |
| src/unsafe_shell_test.py:36:22:36:25 | ControlFlowNode for name | src/unsafe_shell_test.py:39:30:39:33 | ControlFlowNode for name |
| src/unsafe_shell_test.py:36:22:36:25 | ControlFlowNode for name | src/unsafe_shell_test.py:44:20:44:23 | ControlFlowNode for name |
| src/unsafe_shell_test.py:41:24:41:24 | ControlFlowNode for x | src/unsafe_shell_test.py:42:34:42:34 | ControlFlowNode for x |
| src/unsafe_shell_test.py:44:20:44:23 | ControlFlowNode for name | src/unsafe_shell_test.py:41:24:41:24 | ControlFlowNode for x |
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 |
@@ -21,6 +24,9 @@ nodes
| src/unsafe_shell_test.py:29:30:29:33 | ControlFlowNode for name | semmle.label | ControlFlowNode for name |
| src/unsafe_shell_test.py:36:22:36:25 | ControlFlowNode for name | semmle.label | ControlFlowNode for name |
| src/unsafe_shell_test.py:39:30:39:33 | ControlFlowNode for name | semmle.label | ControlFlowNode for name |
| 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 |
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 |
@@ -31,3 +37,4 @@ subpaths
| 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 |

6
python/ql/test/query-tests/Security/CWE-078-UnsafeShellCommandConstruction/src/unsafe_shell_test.py
View File

@@ -38,7 +38,7 @@ def subprocess_flag (name):
subprocess.run("ping " + name, shell=True) # $result=BAD
def indirect(flag):
subprocess.run("ping " + name, shell=flag) # $ MISSING: result=BAD
def indirect(flag, x):
subprocess.run("ping " + x, shell=flag) # $result=BAD
indirect(True)
indirect(True, name)