-
Notifications
You must be signed in to change notification settings - Fork 210
Description
I've been doing source code analysis of certain types of public repos for a specific classes of problems, and I found a something in your repo from my research that you may want to take a look at.
Specifically:
ServerManagement/route/controlWin.py
Line 65 in 49491cc
| subprocess.Popen(shell,shell=True) |
If user-supplied input is passed directly to a command execution function without proper sanitization, a potential command injection vulnerability may exist.
Example:
@app.route("/lookup")
def lookup():
hostname = request.values.get(hostname)
cmd = 'nslookup ' + hostname
return subprocess.check_output(cmd, shell=True)Improved:
@app.route("/lookup")
def lookup():
hostname = request.values.get('hostname')
# Use a list of arguments instead of a concatenated string to avoid shell injection
return subprocess.check_output(["nslookup", hostname])Note: This research has taken some time to complete, so the commit I'm referencing is a few weeks old. You may have already fixed this issue in a later commit. If so, feel free to ignore/close. Just wanted to give you a heads up as a courtesy in case you found it helpful.