Skip to content

Commit 04f422e

Browse files
erik-krogherik-krogh
authored
Merge pull request github#12047 from erik-krogh/py-shell
Py: add unsafe-shell-command-construction
2 parents 5792b4d + 6a5d6eb commit 04f422e

23 files changed

+547
-30
lines changed

‎python/ql/lib/semmle/python/Concepts.qll

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,9 @@ private import semmle.python.security.internal.EncryptionKeySizes
1919
* extend `SystemCommandExecution::Range` instead.
2020
*/
2121
class SystemCommandExecution extends DataFlow::Node instanceof SystemCommandExecution::Range {
22+
/** Holds if a shell interprets `arg`. */
23+
predicate isShellInterpreted(DataFlow::Node arg) { super.isShellInterpreted(arg) }
24+
2225
/** Gets the argument that specifies the command to be executed. */
2326
DataFlow::Node getCommand() { result = super.getCommand() }
2427
}
@@ -35,6 +38,9 @@ module SystemCommandExecution {
3538
abstract class Range extends DataFlow::Node {
3639
/** Gets the argument that specifies the command to be executed. */
3740
abstract DataFlow::Node getCommand();
41+
42+
/** Holds if a shell interprets `arg`. */
43+
predicate isShellInterpreted(DataFlow::Node arg) { none() }
3844
}
3945
}
4046

‎python/ql/lib/semmle/python/Frameworks.qll

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ private import semmle.python.frameworks.Simplejson
5151
private import semmle.python.frameworks.SqlAlchemy
5252
private import semmle.python.frameworks.Starlette
5353
private import semmle.python.frameworks.Stdlib
54+
private import semmle.python.frameworks.Setuptools
5455
private import semmle.python.frameworks.Toml
5556
private import semmle.python.frameworks.Tornado
5657
private import semmle.python.frameworks.Twisted

‎python/ql/lib/semmle/python/frameworks/Fabric.qll

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,14 +43,22 @@ private module FabricV1 {
4343
* - https://docs.fabfile.org/en/1.14/api/core/operations.html#fabric.operations.run
4444
* - https://docs.fabfile.org/en/1.14/api/core/operations.html#fabric.operations.sudo
4545
*/
46-
private class FabricApiLocalRunSudoCall extends SystemCommandExecution::Range,
47-
DataFlow::CallCfgNode
48-
{
46+
private class FabricApiLocalRunSudoCall extends SystemCommandExecution::Range, API::CallNode {
4947
FabricApiLocalRunSudoCall() { this = api().getMember(["local", "run", "sudo"]).getACall() }
5048

5149
override DataFlow::Node getCommand() {
5250
result = [this.getArg(0), this.getArgByName("command")]
5351
}
52+
53+
override predicate isShellInterpreted(DataFlow::Node arg) {
54+
arg = this.getCommand() and
55+
// defaults to running in a shell
56+
not this.getParameter(1, "shell")
57+
.getAValueReachingSink()
58+
.asExpr()
59+
.(ImmutableLiteral)
60+
.booleanValue() = false
61+
}
5462
}
5563
}
5664
}
@@ -163,6 +171,8 @@ private module FabricV2 {
163171
override DataFlow::Node getCommand() {
164172
result = [this.getArg(0), this.getArgByName("command")]
165173
}
174+
175+
override predicate isShellInterpreted(DataFlow::Node arg) { arg = this.getCommand() }
166176
}
167177

168178
// -------------------------------------------------------------------------
@@ -246,6 +256,8 @@ private module FabricV2 {
246256
override DataFlow::Node getCommand() {
247257
result = [this.getArg(0), this.getArgByName("command")]
248258
}
259+
260+
override predicate isShellInterpreted(DataFlow::Node arg) { arg = this.getCommand() }
249261
}
250262

251263
/**

‎python/ql/lib/semmle/python/frameworks/Invoke.qll

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,5 +81,7 @@ private module Invoke {
8181
override DataFlow::Node getCommand() {
8282
result in [this.getArg(0), this.getArgByName("command")]
8383
}
84+
85+
override predicate isShellInterpreted(DataFlow::Node arg) { arg = this.getCommand() }
8486
}
8587
}

‎python/ql/lib/semmle/python/frameworks/Setuptools.qll

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
/**
2+
* Provides classes modeling package setup as defined by `setuptools`.
3+
*/
4+
5+
private import python
6+
private import semmle.python.dataflow.new.DataFlow
7+
8+
/** Provides models for the use of `setuptools` in setup scripts, and the APIs exported by the library defined using `setuptools`. */
9+
module Setuptools {
10+
/**
11+
* Gets a file that sets up a package using `setuptools` (or the deprecated `distutils`).
12+
*/
13+
private File setupFile() {
14+
// all of these might not be extracted, but the support is ready for when they are
15+
result.getBaseName() = ["setup.py", "setup.cfg", "pyproject.toml"]
16+
}
17+
18+
/**
19+
* Gets a file or folder that is exported by a library.
20+
*/
21+
private Container getALibraryExportedContainer() {
22+
// a child folder of the root that has a setup.py file
23+
result = setupFile().getParent().(Folder).getAFolder() and
24+
// where the folder has __init__.py file
25+
exists(result.(Folder).getFile("__init__.py")) and
26+
// and is not a test folder
27+
not result.(Folder).getBaseName() = ["test", "tests", "testing"]
28+
or
29+
// child of a library exported container
30+
result = getALibraryExportedContainer().getAChildContainer() and
31+
(
32+
// either any file
33+
not result instanceof Folder
34+
or
35+
// or a folder with an __init__.py file
36+
exists(result.(Folder).getFile("__init__.py"))
37+
)
38+
}
39+
40+
/**
41+
* Gets an AST node that is exported by a library.
42+
*/
43+
private AstNode getAnExportedLibraryFeature() {
44+
result.(Module).getFile() = getALibraryExportedContainer()
45+
or
46+
result = getAnExportedLibraryFeature().(Module).getAStmt()
47+
or
48+
result = getAnExportedLibraryFeature().(ClassDef).getDefinedClass().getAMethod()
49+
or
50+
result = getAnExportedLibraryFeature().(ClassDef).getDefinedClass().getInitMethod()
51+
or
52+
result = getAnExportedLibraryFeature().(FunctionDef).getDefinedFunction()
53+
}
54+
55+
/**
56+
* Gets a public function (or __init__) that is exported by a library.
57+
*/
58+
private Function getAnExportedFunction() {
59+
result = getAnExportedLibraryFeature() and
60+
(
61+
result.isPublic()
62+
or
63+
result.isInitMethod()
64+
)
65+
}
66+
67+
/**
68+
* Gets a parameter to a public function that is exported by a library.
69+
*/
70+
DataFlow::ParameterNode getALibraryInput() {
71+
result.getParameter() = getAnExportedFunction().getAnArg() and
72+
not result.getParameter().isSelf()
73+
}
74+
}

‎python/ql/lib/semmle/python/frameworks/Stdlib.qll

Lines changed: 43 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1060,7 +1060,11 @@ private module StdlibPrivate {
10601060
private class OsSystemCall extends SystemCommandExecution::Range, DataFlow::CallCfgNode {
10611061
OsSystemCall() { this = os().getMember("system").getACall() }
10621062

1063-
override DataFlow::Node getCommand() { result = this.getArg(0) }
1063+
override DataFlow::Node getCommand() {
1064+
result in [this.getArg(0), this.getArgByName("command")]
1065+
}
1066+
1067+
override predicate isShellInterpreted(DataFlow::Node arg) { arg = this.getCommand() }
10641068
}
10651069

10661070
/**
@@ -1071,7 +1075,7 @@ private module StdlibPrivate {
10711075
* Although deprecated since version 2.6, they still work in 2.7.
10721076
* See https://docs.python.org/2.7/library/os.html#os.popen2
10731077
*/
1074-
private class OsPopenCall extends SystemCommandExecution::Range, DataFlow::CallCfgNode {
1078+
private class OsPopenCall extends SystemCommandExecution::Range, API::CallNode {
10751079
string name;
10761080

10771081
OsPopenCall() {
@@ -1085,6 +1089,8 @@ private module StdlibPrivate {
10851089
not name = "popen" and
10861090
result = this.getArgByName("cmd")
10871091
}
1092+
1093+
override predicate isShellInterpreted(DataFlow::Node arg) { arg = this.getCommand() }
10881094
}
10891095

10901096
/**
@@ -1104,6 +1110,10 @@ private module StdlibPrivate {
11041110
override DataFlow::Node getCommand() { result = this.getArg(0) }
11051111

11061112
override DataFlow::Node getAPathArgument() { result = this.getCommand() }
1113+
1114+
override predicate isShellInterpreted(DataFlow::Node arg) {
1115+
none() // this is a safe API.
1116+
}
11071117
}
11081118

11091119
/**
@@ -1131,6 +1141,10 @@ private module StdlibPrivate {
11311141
}
11321142

11331143
override DataFlow::Node getAPathArgument() { result = this.getCommand() }
1144+
1145+
override predicate isShellInterpreted(DataFlow::Node arg) {
1146+
none() // this is a safe API.
1147+
}
11341148
}
11351149

11361150
/**
@@ -1145,6 +1159,10 @@ private module StdlibPrivate {
11451159
override DataFlow::Node getCommand() { result in [this.getArg(0), this.getArgByName("path")] }
11461160

11471161
override DataFlow::Node getAPathArgument() { result = this.getCommand() }
1162+
1163+
override predicate isShellInterpreted(DataFlow::Node arg) {
1164+
none() // this is a safe API.
1165+
}
11481166
}
11491167

11501168
/** An additional taint step for calls to `os.path.join` */
@@ -1170,7 +1188,7 @@ private module StdlibPrivate {
11701188
* See https://docs.python.org/3.8/library/subprocess.html#subprocess.Popen
11711189
* ref: https://docs.python.org/3/library/subprocess.html#legacy-shell-invocation-functions
11721190
*/
1173-
private class SubprocessPopenCall extends SystemCommandExecution::Range, DataFlow::CallCfgNode {
1191+
private class SubprocessPopenCall extends SystemCommandExecution::Range, API::CallNode {
11741192
SubprocessPopenCall() {
11751193
exists(string name |
11761194
name in [
@@ -1180,43 +1198,33 @@ private module StdlibPrivate {
11801198
)
11811199
}
11821200

1183-
/** Gets the ControlFlowNode for the `args` argument, if any. */
1184-
private DataFlow::Node get_args_arg() { result in [this.getArg(0), this.getArgByName("args")] }
1201+
/** Gets the API-node for the `args` argument, if any. */
1202+
private API::Node get_args_arg() { result = this.getParameter(0, "args") }
11851203

1186-
/** Gets the ControlFlowNode for the `shell` argument, if any. */
1187-
private DataFlow::Node get_shell_arg() {
1188-
result in [this.getArg(8), this.getArgByName("shell")]
1189-
}
1204+
/** Gets the API-node for the `shell` argument, if any. */
1205+
private API::Node get_shell_arg() { result = this.getParameter(8, "shell") }
11901206

11911207
private boolean get_shell_arg_value() {
11921208
not exists(this.get_shell_arg()) and
11931209
result = false
11941210
or
1195-
exists(DataFlow::Node shell_arg | shell_arg = this.get_shell_arg() |
1196-
result = shell_arg.asCfgNode().getNode().(ImmutableLiteral).booleanValue()
1197-
or
1198-
// TODO: Track the "shell" argument to determine possible values
1199-
not shell_arg.asCfgNode().getNode() instanceof ImmutableLiteral and
1200-
(
1201-
result = true
1202-
or
1203-
result = false
1204-
)
1205-
)
1211+
result =
1212+
this.get_shell_arg().getAValueReachingSink().asExpr().(ImmutableLiteral).booleanValue()
1213+
or
1214+
not this.get_shell_arg().getAValueReachingSink().asExpr() instanceof ImmutableLiteral and
1215+
result = false // defaults to `False`
12061216
}
12071217

1208-
/** Gets the ControlFlowNode for the `executable` argument, if any. */
1209-
private DataFlow::Node get_executable_arg() {
1210-
result in [this.getArg(2), this.getArgByName("executable")]
1211-
}
1218+
/** Gets the API-node for the `executable` argument, if any. */
1219+
private API::Node get_executable_arg() { result = this.getParameter(2, "executable") }
12121220

12131221
override DataFlow::Node getCommand() {
12141222
// TODO: Track arguments ("args" and "shell")
12151223
// TODO: Handle using `args=["sh", "-c", <user-input>]`
1216-
result = this.get_executable_arg()
1224+
result = this.get_executable_arg().asSink()
12171225
or
12181226
exists(DataFlow::Node arg_args, boolean shell |
1219-
arg_args = this.get_args_arg() and
1227+
arg_args = this.get_args_arg().asSink() and
12201228
shell = this.get_shell_arg_value()
12211229
|
12221230
// When "executable" argument is set, and "shell" argument is `False`, the
@@ -1242,6 +1250,11 @@ private module StdlibPrivate {
12421250
)
12431251
)
12441252
}
1253+
1254+
override predicate isShellInterpreted(DataFlow::Node arg) {
1255+
arg = [this.get_executable_arg(), this.get_args_arg()].asSink() and
1256+
this.get_shell_arg_value() = true
1257+
}
12451258
}
12461259

12471260
// ---------------------------------------------------------------------------
@@ -1389,6 +1402,8 @@ private module StdlibPrivate {
13891402
}
13901403

13911404
override DataFlow::Node getCommand() { result in [this.getArg(0), this.getArgByName("cmd")] }
1405+
1406+
override predicate isShellInterpreted(DataFlow::Node arg) { arg = this.getCommand() }
13921407
}
13931408

13941409
// ---------------------------------------------------------------------------
@@ -1405,6 +1420,8 @@ private module StdlibPrivate {
14051420
PlatformPopenCall() { this = platform().getMember("popen").getACall() }
14061421

14071422
override DataFlow::Node getCommand() { result in [this.getArg(0), this.getArgByName("cmd")] }
1423+
1424+
override predicate isShellInterpreted(DataFlow::Node arg) { arg = this.getCommand() }
14081425
}
14091426

14101427
// ---------------------------------------------------------------------------

0 commit comments

Comments
 (0)