Skip to content

Commit 5abf31f

Browse files
authored
pySCG adding CWE-78 code and doc (#689)
Adding doc and code for CWE-78 as part of #531 Signed-off-by: Helge Wehder <[email protected]> Co-authored-by: Georg Kunz <[email protected]> and BartyBoi1128
1 parent 904ef17 commit 5abf31f

File tree

5 files changed

+319
-0
lines changed

5 files changed

+319
-0
lines changed
Lines changed: 216 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,216 @@
1+
# CWE-78: Improper Neutralization of Special Elements Used in an OS Command ("OS Command Injection")
2+
3+
Avoid input from untrusted sources to be used directly as part of an OS command and use specialized Python modules where possible instead.
4+
5+
Python can run shell commands either with an active `shell=True` where an actual shell is invoked to run a line of commands such `/bin/bash -c "ls -la *.txt"` or via non-interactive `shell=False` expecting a Python list object.
6+
7+
Using `shell=False` is recommended but is not going to prevent all attacks.
8+
9+
Examples of reduced functionality with `shell=False`:
10+
11+
* Asterisks `ls -1 *.txt` get surrounded by single quotes `ls -1 '*.txt'` so that some Unix commands to no longer work.
12+
* Piping commands `ls -1 |grep *.txt` is prohibited.
13+
* Escape sequences can be difficult to manage
14+
15+
Specialized Python modules, such as `pathlib` or `shutil`, provide a platform-independent solution for most needs and should generally be preferred.
16+
17+
Following table 00 provides a limited list of Unix shell commands to Python module mapping, see [Python Module index](https://docs.python.org/3/py-modindex.html) for more.
18+
19+
|Action|Unix|Python|
20+
|:---|:---|:---|
21+
|Compress or decompress files|gzip, unzip|zlib, gzip, bz2, lzma|
22+
|Filesystem operations|`find .`<br>`tree`<br>`ls -1 *.txt`<br>`test -d`<br>`test -f`<br>`cp`|`Path.rglob("*.txt")`<br>`Path.glob("*.txt")`<br>`Path.is_dir()`<br>`Path.is_file()`<br>`shutil.copy()`|
23+
|Access control operations|`chown`<br>`chmod`|`shutil.chown()`<br>`shutil.chmod()`<br>`stat`|
24+
|Environment variables|`export`<br>`set`|`os.getenv()`<br>`os.setenv()`|
25+
|Get user/group id|id|`os.getuid()`<br>`os.setuid()`|
26+
|Get OS and/or kernel type and name|uname -as|`os.uname()`|
27+
28+
<sub>table 00, example list of Unix commands and their Python equivalents.</sub>
29+
30+
Any variation of using input from a lesser trusted source as part of a command line program has a very high probability of resulting in a potential attack including the use of specialized modules. Consider:
31+
32+
* *CWE-184: Incomplete List of Disallowed Input.*
33+
* *CWE-209: Generation of Error Message Containing Sensitive Information.*
34+
* *[CWE-501: Trust Boundary Violation](https://github.com/ossf/wg-best-practices-os-developers/blob/main/docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-501/README.md)*
35+
36+
## Non-Compliant Code Example (Read Only)
37+
38+
This scenario demonstrates a potential remote command execution. The `FileOperations.list_dir()` method allows an attacker to inject commands into the string dirname such as `head -1 /etc/passwd` under Linux or `net user` under Windows. Older versions of `Python < 3.9.12` allow to turn a non-interactive shell into an active shell in Windows by providing `cmd.exe /C` as an argument [[python.org 3.12.5 - Subprocess management]](https://docs.python.org/3/library/subprocess.html).
39+
40+
*[noncompliant01.py](noncompliant01.py):*
41+
42+
```python
43+
""" Non-compliant Code Example """
44+
# SPDX-FileCopyrightText: OpenSSF project contributors
45+
# SPDX-License-Identifier: MIT
46+
""" Non-compliant Code Example """
47+
from subprocess import Popen
48+
import os
49+
50+
51+
class FileOperations:
52+
"""Helper class for file system operations"""
53+
54+
def list_dir(self, dirname: str):
55+
"""List the contents of a directory"""
56+
if "nt" in os.name:
57+
Popen("dir " + dirname, shell=True).communicate()
58+
if "posix" in os.name:
59+
Popen("ls " + dirname, shell=True).communicate()
60+
61+
62+
#####################
63+
# Trying to exploit above code example
64+
#####################
65+
if "nt" in os.name:
66+
FileOperations().list_dir("%HOMEPATH% & net user")
67+
if "posix" in os.name:
68+
FileOperations().list_dir("/etc/shadow; head -1 /etc/passwd")
69+
70+
```
71+
72+
The code in `noncompliant01.py` prints the first line of `/etc/passwd` on Linux or starts `net user` under Windows.
73+
The `FileOperations().list_dir()` method allows an attacker to add commands via `;` in Linux and `&` in Windows.
74+
75+
## Non-Compliant Code Example (Read, Write)
76+
77+
The attack surface increases if a user is also allowed to upload or create files or folders.
78+
79+
The `noncompliant02.py` example demonstrates the injection via file or folder name that is created prior to using the `list_dir()` method. We assume here that an untrusted user is allowed to create files or folders named `& calc.exe or ;ps aux` as part of another service such as upload area, submit form, or as a result of a zip-bomb as per *CWE-409: Improper Handling of Highly Compressed Data (Data Amplification)*. Encoding issues as described in *CWE-180: Incorrect Behavior Order: Validate Before Canonicalize* must also be considered.
80+
81+
The issue occurs when mixing shell commands with data from a lesser trusted source.
82+
83+
Some shell commands, such as `find` with `-exec`, allow running secondary commands via arguments [[CTFOBins]](https://gtfobins.github.io/) [[LOLBAS]](https://lolbas-project.github.io/) that can be misused for shell injections if no shell is provided `shell=False`. The `shlex.split()` method is frequently used to split a string into a list for `subprocess.run()` in order to run a non-interactive shell such as `ls -la` into `["ls", "-la"]` and plays a minor role in simplifying the attack. The `noncompliant02.py` code only works on Linux, in this example calling a rather harmless uptime.
84+
85+
*[noncompliant02.py](noncompliant02.py):*
86+
87+
```python
88+
# SPDX-FileCopyrightText: OpenSSF project contributors
89+
# SPDX-License-Identifier: MIT
90+
""" Non-compliant Code Example """
91+
import os
92+
import shlex
93+
from subprocess import run
94+
95+
96+
def list_dir(dirname: str):
97+
"""Lists only 2 levels of folders in a default directory"""
98+
os.chdir(dirname)
99+
cmd = "find . -maxdepth 1 -type d"
100+
result = run(shlex.split(cmd), check=True, capture_output=True)
101+
102+
for subfolder in result.stdout.decode("utf-8").splitlines():
103+
cmd = "find " + subfolder + " -maxdepth 1 -type d"
104+
subresult = run(shlex.split(cmd), check=True, capture_output=True)
105+
for item in subresult.stdout.decode("utf-8").splitlines():
106+
print(item)
107+
108+
109+
#####################
110+
# Trying to exploit above code example
111+
#####################
112+
# just to keep it clean we create folder for this test
113+
os.makedirs("temp", exist_ok=True)
114+
115+
# simulating upload area (payload):
116+
print("Testing Corrupted Directory")
117+
if "posix" in os.name:
118+
with open("temp/toast.sh", "w", encoding="utf-8") as file_handle:
119+
file_handle.write("uptime\n")
120+
os.makedirs("temp/. -exec bash toast.sh {} +", exist_ok=True)
121+
122+
# running the query:
123+
list_dir("temp")
124+
125+
```
126+
127+
In `noncompliant02.py` the attacker creates a `toast.sh` file that contains the commands to run. The attacker also creates a folder named `. -exec bash toast.sh {} +` that will later become part of the shell `find` command forming `find . -exec bash toast.sh {} +`.
128+
129+
The result is that `list_dir(dirname)` will run the `toast.sh` as a shell script. The `toast.sh` file does not require execute rights and can contain any quantity of shell command complexity.
130+
131+
## Compliant Solution
132+
133+
The `compliant01.py` code using the cross-platform compatible pathlib module and restricting filesystem area. The `pathlib` on its own will not prevent all attacks.
134+
135+
*[compliant01.py](compliant01.py):*
136+
137+
```python
138+
""" Compliant Code Example """
139+
140+
# SPDX-FileCopyrightText: OpenSSF project contributors
141+
# SPDX-License-Identifier: MIT
142+
""" Compliant Code Example """
143+
import os
144+
from pathlib import Path
145+
146+
147+
def list_dir(dirname: str):
148+
"""List the contents of a directory recursively
149+
150+
Parameters:
151+
dirname (string): Directory name
152+
"""
153+
path = Path(dirname)
154+
allowed_directory = Path.home()
155+
# TODO: input sanitation
156+
# TODO: Add secure logging
157+
if Path(
158+
allowed_directory.joinpath(dirname)
159+
.resolve()
160+
.relative_to(allowed_directory.resolve())
161+
):
162+
for item in path.glob("*"):
163+
print(item)
164+
165+
166+
#####################
167+
# Trying to exploit above code example
168+
#####################
169+
# just to keep it clean we create folder for this test
170+
os.makedirs("temp", exist_ok=True)
171+
172+
# simulating upload area (payload):
173+
print("Testing Corrupted Directory")
174+
if "posix" in os.name:
175+
with open("temp/toast.sh", "w", encoding="utf-8") as file_handle:
176+
file_handle.write("uptime\n")
177+
os.makedirs("temp/. -exec bash toast.sh {} +", exist_ok=True)
178+
179+
# running the query:
180+
list_dir("temp")
181+
182+
```
183+
184+
The `compliant01.py` does not use data that origins from a lesser trusted source in order to form a shell command and would throw an error for an attempt to list content outside of the allowed area. The code is actually not "neutralizing" data itself from an untrusted source as such, the attack is "neutralized" by no longer using `subprocess` or `os` to run `find`.
185+
186+
## Automated Detection
187+
188+
|Tool|Version|Checker|Description|
189+
|:---|:---|:---|:---|
190+
|Pycharm|2022.3.3 Python 3.11.6|[PR100](https://pycharm-security.readthedocs.io/en/latest/checks/PR100.html)|Calling `subprocess.call`, `subprocess.run`, or `subprocess.Popen` with `shell=True` can leave the host shell open to local code execution or remote code execution attacks|
191+
|bandit|1.7.9 on python 3.11.4|[B404](https://bandit.readthedocs.io/en/latest/blacklists/blacklist_imports.html#b404-import-subprocess)|Consider possible security implications associated with these modules|
192+
|bandit|1.7.9 on python 3.11.4|[B602](https://bandit.readthedocs.io/en/latest/plugins/b602_subprocess_popen_with_shell_equals_true.html)|Bsubprocess call with `shell=True` identified, security issue.bandit|
193+
|bandit|1.7.9 on python 3.11.4|[B603](https://bandit.readthedocs.io/en/latest/plugins/b603_subprocess_without_shell_equals_true.html)|`subprocess` call - check for execution of untrusted input.|
194+
|bandit|1.7.9 on python 3.11.4|[B604](https://bandit.readthedocs.io/en/latest/plugins/b604_any_other_function_with_shell_equals_true.html)|Consider possible security implications associated with the `subprocess` module|
195+
|bandit|1.7.9 on python 3.11.4|[B605](https://bandit.readthedocs.io/en/1.7.4/plugins/b605_start_process_with_a_shell.html)|Bandit seems to detect any use of `os.system()` whether sanitized or not.|
196+
197+
## Related Guidelines
198+
199+
|||
200+
|:---|:---|
201+
|[MITRE CWE](http://cwe.mitre.org/)|Pillar: [CWE-707: Improper Neutralization](hhttps://cwe.mitre.org/data/definitions/707.html)|
202+
|[MITRE CWE](http://cwe.mitre.org/)|Base: [CWE-78, Improper Neutralization of Special Elements Used in an OS Command ("OS Command Injection")](https://cwe.mitre.org/data/definitions/000.html)|
203+
|[SEI CERT Coding Standard for Java](https://wiki.sei.cmu.edu/confluence/display/java/SEI+CERT+Oracle+Coding+Standard+for+Java)|[IDS07-J. Sanitize untrusted data passed to the Runtime.exec() method](https://wiki.sei.cmu.edu/confluence/display/java/IDS07-J.+Sanitize+untrusted+data+passed+to+the+Runtime.exec%28%29+method)|
204+
|[SEI CERT C Coding Standard](https://wiki.sei.cmu.edu/confluence/display/c/SEI+CERT+C+Coding+Standard)|[ENV03-C. Sanitize the environment when invoking external programs](https://wiki.sei.cmu.edu/confluence/display/c/ENV03-C.+Sanitize+the+environment+when+invoking+external+programs)|
205+
|[SEI CERT C Coding Standard](https://wiki.sei.cmu.edu/confluence/display/c/SEI+CERT+C+Coding+Standard)|[ENV33-C. Do not call system()](https://wiki.sei.cmu.edu/confluence/pages/viewpage.action?pageId=87152177)|
206+
|[SEI CERT C++ Coding Standard](https://wiki.sei.cmu.edu/confluence/pages/viewpage.action?pageId=88046682)|[ENV03-CPP. Sanitize the environment when invoking external programs VOID ENV02-CPP. Do not call system() if you do not need a command processor](https://wiki.sei.cmu.edu/confluence/pages/viewpage.action?pageId=88046815)|
207+
|[ISO/IEC TR 24772:2013](https://wiki.sei.cmu.edu/confluence/display/java/Rule+AA.+References#RuleAA.References-ISO/IECTR24772-2013)|Injection [RST]|
208+
209+
## Bibliography
210+
211+
|||
212+
|:---|:---|
213+
|[[Python docs](https://docs.python.org/3/reference/expressions.html#binary-arithmetic-operations)]|subprocess — Subprocess management — Python 3.10.4 documentation [online]. Available from: [https://docs.python.org/3/library/subprocess.html](https://docs.python.org/3/library/subprocess.html), [accessed 1 November 2024] |
214+
|[[Python docs](https://docs.python.org/3/reference/expressions.html#binary-arithmetic-operations)]|os — Miscellaneous operating system interfaces — Python 3.10.4 documentation [online]. Available from: [https://docs.python.org/3/library/os.html#os.system](https://docs.python.org/3/library/os.html#os.system), [accessed 1 November 2024] |
215+
|[CTFOBins]|GTFOBins is a curated list of Unix binaries that can be used to bypass local security restrictions in misconfigured systems. [online]. Available from: [https://gtfobins.github.io/](https://gtfobins.github.io/), [accessed 1 November 2024] |
216+
|[LOLBAS]|LOLBAS Living Off The Land Binaries, Scripts and Libraries. [online]. Available from: [https://lolbas-project.github.io/](https://lolbas-project.github.io/), [accessed 1 November 2024] |
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
# SPDX-FileCopyrightText: OpenSSF project contributors
2+
# SPDX-License-Identifier: MIT
3+
""" Compliant Code Example """
4+
import os
5+
from pathlib import Path
6+
7+
8+
def list_dir(dirname: str):
9+
"""List the contents of a directory recursively
10+
11+
Parameters:
12+
dirname (string): Directory name
13+
"""
14+
path = Path(dirname)
15+
allowed_directory = Path.home()
16+
# TODO: input sanitation
17+
# TODO: Add secure logging
18+
if Path(
19+
allowed_directory.joinpath(dirname)
20+
.resolve()
21+
.relative_to(allowed_directory.resolve())
22+
):
23+
for item in path.glob("*"):
24+
print(item)
25+
26+
27+
#####################
28+
# Trying to exploit above code example
29+
#####################
30+
# just to keep it clean we create folder for this test
31+
os.makedirs("temp", exist_ok=True)
32+
33+
# simulating upload area (payload):
34+
print("Testing Corrupted Directory")
35+
if "posix" in os.name:
36+
with open("temp/toast.sh", "w", encoding="utf-8") as file_handle:
37+
file_handle.write("uptime\n")
38+
os.makedirs("temp/. -exec bash toast.sh {} +", exist_ok=True)
39+
40+
# running the query:
41+
list_dir("temp")
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
# SPDX-FileCopyrightText: OpenSSF project contributors
2+
# SPDX-License-Identifier: MIT
3+
""" Non-compliant Code Example """
4+
from subprocess import Popen
5+
import os
6+
7+
8+
class FileOperations:
9+
"""Helper class for file system operations"""
10+
11+
def list_dir(self, dirname: str):
12+
"""List the contents of a directory"""
13+
if "nt" in os.name:
14+
Popen("dir " + dirname, shell=True).communicate()
15+
if "posix" in os.name:
16+
Popen("ls " + dirname, shell=True).communicate()
17+
18+
19+
#####################
20+
# Trying to exploit above code example
21+
#####################
22+
if "nt" in os.name:
23+
FileOperations().list_dir("%HOMEPATH% & net user")
24+
if "posix" in os.name:
25+
FileOperations().list_dir("/etc/shadow; head -1 /etc/passwd")
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
# SPDX-FileCopyrightText: OpenSSF project contributors
2+
# SPDX-License-Identifier: MIT
3+
""" Non-compliant Code Example """
4+
import os
5+
import shlex
6+
from subprocess import run
7+
8+
9+
def list_dir(dirname: str):
10+
"""Lists only 2 levels of folders in a default directory"""
11+
os.chdir(dirname)
12+
cmd = "find . -maxdepth 1 -type d"
13+
result = run(shlex.split(cmd), check=True, capture_output=True)
14+
15+
for subfolder in result.stdout.decode("utf-8").splitlines():
16+
cmd = "find " + subfolder + " -maxdepth 1 -type d"
17+
subresult = run(shlex.split(cmd), check=True, capture_output=True)
18+
for item in subresult.stdout.decode("utf-8").splitlines():
19+
print(item)
20+
21+
22+
#####################
23+
# Trying to exploit above code example
24+
#####################
25+
# just to keep it clean we create folder for this test
26+
os.makedirs("temp", exist_ok=True)
27+
28+
# simulating upload area (payload):
29+
print("Testing Corrupted Directory")
30+
if "posix" in os.name:
31+
with open("temp/toast.sh", "w", encoding="utf-8") as file_handle:
32+
file_handle.write("uptime\n")
33+
os.makedirs("temp/. -exec bash toast.sh {} +", exist_ok=True)
34+
35+
# running the query:
36+
list_dir("temp")

docs/Secure-Coding-Guide-for-Python/readme.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@ It is **not production code** and requires code-style or python best practices t
8787

8888
|[CWE-707: Improper Neutralization](https://cwe.mitre.org/data/definitions/707.html)|Prominent CVE|
8989
|:----------------------------------------------------------------|:----|
90+
|[CWE-78: Improper Neutralization of Special Elements Used in an OS Command ("OS Command Injection")](CWE-707/CWE-78/README.md)||
9091
|[CWE-89: Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection')](CWE-707/CWE-89/.)|[CVE-2019-8600](https://www.cvedetails.com/cve/CVE-2019-8600/),<br/>CVSSv3.1: **9.8**,<br/>EPSS: **01.43** (18.02.2024)|
9192
|[CWE-117: Improper Output Neutralization for Logs](CWE-707/CWE-117/.)||
9293
|[CWE-180: Incorrect behavior order: Validate before Canonicalize](CWE-707/CWE-180/.)|[CVE-2022-26136](https://www.cvedetails.com/cve/CVE-2022-26136/),<br/>CVSSv3.1: **9.8**,<br/>EPSS: **00.77** (05.11.2024)|

0 commit comments

Comments
 (0)