Merge pull request #1124 from guardicore/dlint

Add dlint to flake8

.pre-commit-config.yaml

@@ -19,6 +19,7 @@ repos:
     rev: 3.9.1
     hooks:
       - id: flake8
+        additional_dependencies: [dlint]
   - repo: https://github.com/pre-commit/pre-commit-hooks
     rev: v3.4.0
     hooks:

CHANGELOG.md

@@ -25,3 +25,6 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 
 ### Fixed
 - Attempted to delete a directory when monkey config reset was called. #1054
+
+### Security
+- Address minor issues discovered by Dlint. #1075

envs/monkey_zoo/blackbox/island_client/monkey_island_requests.py

@@ -66,8 +66,8 @@ class MonkeyIslandRequests(object):
             return request_function_wrapper
 
     def get_jwt_from_server(self):
-        resp = requests.post(
-            self.addr + "api/auth",  # noqa: DUO123
+        resp = requests.post(  # noqa: DUO123
+            self.addr + "api/auth",
             json={"username": NO_AUTH_CREDS, "password": NO_AUTH_CREDS},
             verify=False,
         )
@@ -75,8 +75,8 @@ class MonkeyIslandRequests(object):
 
     @_Decorators.refresh_jwt_token
     def get(self, url, data=None):
-        return requests.get(
-            self.addr + url,  # noqa: DUO123
+        return requests.get(  # noqa: DUO123
+            self.addr + url,
             headers=self.get_jwt_header(),
             params=data,
             verify=False,
@@ -84,25 +84,25 @@ class MonkeyIslandRequests(object):
 
     @_Decorators.refresh_jwt_token
     def post(self, url, data):
-        return requests.post(
-            self.addr + url, data=data, headers=self.get_jwt_header(), verify=False  # noqa: DUO123
+        return requests.post(  # noqa: DUO123
+            self.addr + url, data=data, headers=self.get_jwt_header(), verify=False
         )
 
     @_Decorators.refresh_jwt_token
     def post_json(self, url, data: Dict):
-        return requests.post(
-            self.addr + url, json=data, headers=self.get_jwt_header(), verify=False  # noqa: DUO123
+        return requests.post(  # noqa: DUO123
+            self.addr + url, json=data, headers=self.get_jwt_header(), verify=False
         )
 
     @_Decorators.refresh_jwt_token
     def patch(self, url, data: Dict):
-        return requests.patch(
-            self.addr + url, data=data, headers=self.get_jwt_header(), verify=False  # noqa: DUO123
+        return requests.patch(  # noqa: DUO123
+            self.addr + url, data=data, headers=self.get_jwt_header(), verify=False
         )
 
     @_Decorators.refresh_jwt_token
     def delete(self, url):
-        return requests.delete(  # noqa: DOU123
+        return requests.delete(  # noqa: DUO123
             self.addr + url, headers=self.get_jwt_header(), verify=False
         )
 

envs/monkey_zoo/blackbox/utils/gcp_machine_handlers.py

@@ -23,9 +23,9 @@ class GCPHandler(object):
             subprocess.call(GCPHandler.get_auth_command(key_path), shell=True)  # noqa: DUO116
             LOGGER.info("GCP Handler passed key")
             # set project
-            subprocess.call(
+            subprocess.call(  # noqa: DUO116
                 GCPHandler.get_set_project_command(project_id), shell=True
-            )  # noqa: DUO116
+            )
             LOGGER.info("GCP Handler set project")
             LOGGER.info("GCP Handler initialized successfully")
         except Exception as e:
@@ -39,18 +39,18 @@ class GCPHandler(object):
         """
         LOGGER.info("Setting up all GCP machines...")
         try:
-            subprocess.call(
+            subprocess.call(  # noqa: DUO116
                 (GCPHandler.MACHINE_STARTING_COMMAND % (machine_list, self.zone)), shell=True
-            )  # noqa: DUO116
+            )
             LOGGER.info("GCP machines successfully started.")
         except Exception as e:
             LOGGER.error("GCP Handler failed to start GCP machines: %s" % e)
 
     def stop_machines(self, machine_list):
         try:
-            subprocess.call(
+            subprocess.call(  # noqa: DUO116
                 (GCPHandler.MACHINE_STOPPING_COMMAND % (machine_list, self.zone)), shell=True
-            )  # noqa: DUO116
+            )
             LOGGER.info("GCP machines stopped successfully.")
         except Exception as e:
             LOGGER.error("GCP Handler failed to stop network machines: %s" % e)

monkey/infection_monkey/control.py

@@ -64,8 +64,8 @@ class ControlClient(object):
         if ControlClient.proxies:
             monkey["tunnel"] = ControlClient.proxies.get("https")
 
-        requests.post(
-            "https://%s/api/monkey" % (WormConfiguration.current_server,),  # noqa: DUO123
+        requests.post(  # noqa: DUO123
+            "https://%s/api/monkey" % (WormConfiguration.current_server,),
             data=json.dumps(monkey),
             headers={"content-type": "application/json"},
             verify=False,
@@ -92,8 +92,8 @@ class ControlClient(object):
                 if ControlClient.proxies:
                     debug_message += " through proxies: %s" % ControlClient.proxies
                 LOG.debug(debug_message)
-                requests.get(
-                    f"https://{server}/api?action=is-up",  # noqa: DUO123
+                requests.get(  # noqa: DUO123
+                    f"https://{server}/api?action=is-up",
                     verify=False,
                     proxies=ControlClient.proxies,
                     timeout=TIMEOUT_IN_SECONDS,
@@ -130,9 +130,8 @@ class ControlClient(object):
             monkey = {}
             if ControlClient.proxies:
                 monkey["tunnel"] = ControlClient.proxies.get("https")
-            requests.patch(
-                "https://%s/api/monkey/%s"
-                % (WormConfiguration.current_server, GUID),  # noqa: DUO123
+            requests.patch(  # noqa: DUO123
+                "https://%s/api/monkey/%s" % (WormConfiguration.current_server, GUID),
                 data=json.dumps(monkey),
                 headers={"content-type": "application/json"},
                 verify=False,
@@ -155,8 +154,8 @@ class ControlClient(object):
             return
         try:
             telemetry = {"monkey_guid": GUID, "telem_category": telem_category, "data": json_data}
-            requests.post(
-                "https://%s/api/telemetry" % (WormConfiguration.current_server,),  # noqa: DUO123
+            requests.post(  # noqa: DUO123
+                "https://%s/api/telemetry" % (WormConfiguration.current_server,),
                 data=json.dumps(telemetry),
                 headers={"content-type": "application/json"},
                 verify=False,
@@ -174,8 +173,8 @@ class ControlClient(object):
             return
         try:
             telemetry = {"monkey_guid": GUID, "log": json.dumps(log)}
-            requests.post(
-                "https://%s/api/log" % (WormConfiguration.current_server,),  # noqa: DUO123
+            requests.post(  # noqa: DUO123
+                "https://%s/api/log" % (WormConfiguration.current_server,),
                 data=json.dumps(telemetry),
                 headers={"content-type": "application/json"},
                 verify=False,
@@ -192,9 +191,8 @@ class ControlClient(object):
         if not WormConfiguration.current_server:
             return
         try:
-            reply = requests.get(
-                "https://%s/api/monkey/%s"
-                % (WormConfiguration.current_server, GUID),  # noqa: DUO123
+            reply = requests.get(  # noqa: DUO123
+                "https://%s/api/monkey/%s" % (WormConfiguration.current_server, GUID),
                 verify=False,
                 proxies=ControlClient.proxies,
                 timeout=MEDIUM_REQUEST_TIMEOUT,
@@ -230,9 +228,8 @@ class ControlClient(object):
         if not WormConfiguration.current_server:
             return
         try:
-            requests.patch(
-                "https://%s/api/monkey/%s"
-                % (WormConfiguration.current_server, GUID),  # noqa: DUO123
+            requests.patch(  # noqa: DUO123
+                "https://%s/api/monkey/%s" % (WormConfiguration.current_server, GUID),
                 data=json.dumps({"config_error": True}),
                 headers={"content-type": "application/json"},
                 verify=False,
@@ -292,9 +289,9 @@ class ControlClient(object):
             if (monkeyfs.isfile(dest_file)) and (size == monkeyfs.getsize(dest_file)):
                 return dest_file
             else:
-                download = requests.get(
+                download = requests.get(  # noqa: DUO123
                     "https://%s/api/monkey/download/%s"
-                    % (WormConfiguration.current_server, filename),  # noqa: DUO123
+                    % (WormConfiguration.current_server, filename),
                     verify=False,
                     proxies=ControlClient.proxies,
                     timeout=MEDIUM_REQUEST_TIMEOUT,
@@ -322,9 +319,8 @@ class ControlClient(object):
         if not WormConfiguration.current_server:
             return None, None
         try:
-            reply = requests.post(
-                "https://%s/api/monkey/download"
-                % (WormConfiguration.current_server,),  # noqa: DUO123
+            reply = requests.post(  # noqa: DUO123
+                "https://%s/api/monkey/download" % (WormConfiguration.current_server,),
                 data=json.dumps(host_dict),
                 headers={"content-type": "application/json"},
                 verify=False,
@@ -370,8 +366,8 @@ class ControlClient(object):
     @staticmethod
     def get_pba_file(filename):
         try:
-            return requests.get(
-                PBA_FILE_DOWNLOAD % (WormConfiguration.current_server, filename),  # noqa: DUO123
+            return requests.get(  # noqa: DUO123
+                PBA_FILE_DOWNLOAD % (WormConfiguration.current_server, filename),
                 verify=False,
                 proxies=ControlClient.proxies,
                 timeout=LONG_REQUEST_TIMEOUT,
@@ -382,9 +378,9 @@ class ControlClient(object):
     @staticmethod
     def get_T1216_pba_file():
         try:
-            return requests.get(
+            return requests.get(  # noqa: DUO123
                 urljoin(
-                    f"https://{WormConfiguration.current_server}/",  # noqa: DUO123
+                    f"https://{WormConfiguration.current_server}/",
                     T1216_PBA_FILE_DOWNLOAD_PATH,
                 ),
                 verify=False,
@@ -416,7 +412,9 @@ class ControlClient(object):
                 f"https://{WormConfiguration.current_server}/api/monkey_control"
                 f"/check_remote_port/{port}"
             )
-            response = requests.get(url, verify=False, timeout=SHORT_REQUEST_TIMEOUT)
+            response = requests.get(  # noqa: DUO123
+                url, verify=False, timeout=SHORT_REQUEST_TIMEOUT
+            )
             response = json.loads(response.content.decode())
             return response["status"] == "port_visible"
         except requests.exceptions.RequestException:
@@ -424,7 +422,7 @@ class ControlClient(object):
 
     @staticmethod
     def report_start_on_island():
-        requests.post(
+        requests.post(  # noqa: DUO123
             f"https://{WormConfiguration.current_server}/api/monkey_control/started_on_island",
             data=json.dumps({"started_on_island": True}),
             verify=False,

monkey/infection_monkey/dropper.py

@@ -4,6 +4,7 @@ import filecmp
 import logging
 import os
 import pprint
+import shlex
 import shutil
 import subprocess
 import sys
@@ -13,11 +14,7 @@ from ctypes import c_char_p
 from common.utils.attack_utils import ScanStatus, UsageEnum
 from infection_monkey.config import WormConfiguration
 from infection_monkey.exploit.tools.helpers import build_monkey_commandline_explicitly
-from infection_monkey.model import (
-    GENERAL_CMDLINE_LINUX,
-    MONKEY_CMDLINE_LINUX,
-    MONKEY_CMDLINE_WINDOWS,
-)
+from infection_monkey.model import MONKEY_CMDLINE_LINUX, MONKEY_CMDLINE_WINDOWS
 from infection_monkey.system_info import OperatingSystem, SystemInfoCollector
 from infection_monkey.telemetry.attack.t1106_telem import T1106Telem
 
@@ -146,33 +143,46 @@ class MonkeyDrops(object):
         )
 
         if OperatingSystem.Windows == SystemInfoCollector.get_os():
+            # TODO: Replace all of this string templating with a function that accepts
+            #       the necessary parameters and returns a list of arguments.
             monkey_cmdline = (
                 MONKEY_CMDLINE_WINDOWS % {"monkey_path": self._config["destination_path"]}
                 + monkey_options
             )
+            monkey_cmdline_split = shlex.split(
+                monkey_cmdline,
+                posix=False,  # won't try resolving "\" in paths as part of escape sequences
+            )
+
+            monkey_process = subprocess.Popen(
+                monkey_cmdline_split,
+                stdin=subprocess.PIPE,
+                stdout=subprocess.PIPE,
+                stderr=subprocess.PIPE,
+                close_fds=True,
+                creationflags=DETACHED_PROCESS,
+            )
         else:
             dest_path = self._config["destination_path"]
-            # In linux we have a more complex commandline. There's a general outer one,
-            # and the inner one which actually
-            # runs the monkey
-            inner_monkey_cmdline = (
+            # In Linux, we need to change the directory first, which is done
+            # using thw `cwd` argument in `subprocess.Popen` below
+            # TODO: Replace all of this string templating with a function that accepts
+            #       the necessary parameters and returns a list of arguments.
+            monkey_cmdline = (
                 MONKEY_CMDLINE_LINUX % {"monkey_filename": dest_path.split("/")[-1]}
                 + monkey_options
             )
-            monkey_cmdline = GENERAL_CMDLINE_LINUX % {
-                "monkey_directory": dest_path[0 : dest_path.rfind("/")],
-                "monkey_commandline": inner_monkey_cmdline,
-            }
+            monkey_cmdline_split = shlex.split(monkey_cmdline)
 
-        monkey_process = subprocess.Popen(
-            monkey_cmdline,
-            shell=True,
-            stdin=subprocess.PIPE,
-            stdout=subprocess.PIPE,
-            stderr=subprocess.PIPE,
-            close_fds=True,
-            creationflags=DETACHED_PROCESS,
-        )
+            monkey_process = subprocess.Popen(
+                monkey_cmdline_split,
+                stdin=subprocess.PIPE,
+                stdout=subprocess.PIPE,
+                stderr=subprocess.PIPE,
+                close_fds=True,
+                cwd="/".join(dest_path.split("/")[0:-1]),
+                creationflags=DETACHED_PROCESS,
+            )
 
         LOG.info(
             "Executed monkey process (PID=%d) with command line: %s",

monkey/infection_monkey/exploit/drupal.py

@@ -82,8 +82,8 @@ class DrupalExploiter(WebRCE):
         """
         payload = build_exploitability_check_payload(url)
 
-        response = requests.get(
-            f"{url}?_format=hal_json",  # noqa: DUO123
+        response = requests.get(  # noqa: DUO123
+            f"{url}?_format=hal_json",
             json=payload,
             headers={"Content-Type": "application/hal+json"},
             verify=False,
@@ -102,8 +102,8 @@ class DrupalExploiter(WebRCE):
         base = remove_port(url)
         payload = build_cmd_execution_payload(base, cmd)
 
-        r = requests.get(
-            f"{url}?_format=hal_json",  # noqa: DUO123
+        r = requests.get(  # noqa: DUO123
+            f"{url}?_format=hal_json",
             json=payload,
             headers={"Content-Type": "application/hal+json"},
             verify=False,
@@ -157,9 +157,9 @@ def find_exploitbale_article_ids(base_url: str, lower: int = 1, upper: int = 100
     articles = set()
     while lower < upper:
         node_url = urljoin(base_url, str(lower))
-        response = requests.get(
+        response = requests.get(  # noqa: DUO123
             node_url, verify=False, timeout=LONG_REQUEST_TIMEOUT
-        )  # noqa: DUO123
+        )
         if response.status_code == 200:
             if is_response_cached(response):
                 LOG.info(f"Found a cached article at: {node_url}, skipping")

monkey/infection_monkey/exploit/hadoop.py

@@ -7,8 +7,8 @@
 import json
 import logging
 import posixpath
-import random
 import string
+from random import SystemRandom
 
 import requests
 
@@ -69,8 +69,9 @@ class HadoopExploiter(WebRCE):
         resp = json.loads(resp.content)
         app_id = resp["application-id"]
         # Create a random name for our application in YARN
+        safe_random = SystemRandom()
         rand_name = ID_STRING + "".join(
-            [random.choice(string.ascii_lowercase) for _ in range(self.RAN_STR_LEN)]
+            [safe_random.choice(string.ascii_lowercase) for _ in range(self.RAN_STR_LEN)]
         )
         payload = self.build_payload(app_id, rand_name, command)
         resp = requests.post(

monkey/infection_monkey/exploit/shellshock.py

@@ -3,7 +3,7 @@
 
 import logging
 import string
-from random import choice
+from random import SystemRandom
 
 import requests
 
@@ -37,8 +37,9 @@ class ShellShockExploiter(HostExploiter):
     def __init__(self, host):
         super(ShellShockExploiter, self).__init__(host)
         self.HTTP = [str(port) for port in self._config.HTTP_PORTS]
+        safe_random = SystemRandom()
         self.success_flag = "".join(
-            choice(string.ascii_uppercase + string.digits) for _ in range(20)
+            safe_random.choice(string.ascii_uppercase + string.digits) for _ in range(20)
         )
         self.skip_exist = self._config.skip_exploit_if_file_exist
 
@@ -242,9 +243,9 @@ class ShellShockExploiter(HostExploiter):
         try:
             LOG.debug("Header is: %s" % header)
             LOG.debug("Attack is: %s" % attack)
-            r = requests.get(
+            r = requests.get(  # noqa: DUO123
                 url, headers={header: attack}, verify=False, timeout=TIMEOUT
-            )  # noqa: DUO123
+            )
             result = r.content.decode()
             return result
         except requests.exceptions.RequestException as exc:

monkey/infection_monkey/exploit/struts2.py

@@ -52,7 +52,7 @@ class Struts2Exploiter(WebRCE):
         request = urllib.request.Request(url, headers=headers)
         try:
             return urllib.request.urlopen(
-                request, context=ssl._create_unverified_context()
+                request, context=ssl._create_unverified_context()  # noqa: DUO122
             ).geturl()
         except urllib.error.URLError:
             LOG.error("Can't reach struts2 server")

monkey/infection_monkey/exploit/weblogic.py

@@ -83,9 +83,9 @@ class WebLogic201710271(WebRCE):
         else:
             payload = self.get_exploit_payload("cmd", "/c", command + " 1> NUL 2> NUL")
         try:
-            post(
+            post(  # noqa: DUO123
                 url, data=payload, headers=HEADERS, timeout=EXECUTION_TIMEOUT, verify=False
-            )  # noqa: DUO123
+            )
         except Exception as e:
             LOG.error("Connection error: %s" % e)
             return False
@@ -121,9 +121,9 @@ class WebLogic201710271(WebRCE):
     def check_if_exploitable_weblogic(self, url, httpd):
         payload = self.get_test_payload(ip=httpd.local_ip, port=httpd.local_port)
         try:
-            post(
+            post(  # noqa: DUO123
                 url, data=payload, headers=HEADERS, timeout=REQUEST_DELAY, verify=False
-            )  # noqa: DUO123
+            )
         except exceptions.ReadTimeout:
             # Our request will not get response thus we get ReadTimeout error
             pass

monkey/infection_monkey/network/info.py

@@ -1,7 +1,7 @@
 import itertools
 import socket
 import struct
-from random import randint
+from random import randint  # noqa: DUO102
 from subprocess import check_output
 
 import netifaces

monkey/infection_monkey/network/tcp_scanner.py

@@ -1,5 +1,5 @@
 from itertools import zip_longest
-from random import shuffle
+from random import shuffle  # noqa: DUO102
 
 import infection_monkey.config
 from infection_monkey.network.HostFinger import HostFinger

monkey/infection_monkey/post_breach/actions/clear_command_history.py

@@ -46,8 +46,8 @@ class ClearCommandHistory(PBA):
             def run(self):
                 if self.command:
                     try:
-                        output = subprocess.check_output(
-                            self.command, stderr=subprocess.STDOUT, shell=True  # noqa: DUO116
+                        output = subprocess.check_output(  # noqa: DUO116
+                            self.command, stderr=subprocess.STDOUT, shell=True
                         ).decode()
                         return output, True
                     except subprocess.CalledProcessError as e:

monkey/infection_monkey/post_breach/actions/communicate_as_new_user.py

@@ -52,8 +52,8 @@ class CommunicateAsNewUser(PBA):
     @staticmethod
     def get_random_new_user_name():
         return USERNAME_PREFIX + "".join(
-            random.choice(string.ascii_lowercase) for _ in range(5)
-        )  # noqa: DUO102
+            random.choice(string.ascii_lowercase) for _ in range(5)  # noqa: DUO102
+        )
 
     @staticmethod
     def get_commandline_for_http_request(url, is_windows=is_windows_os()):

monkey/infection_monkey/post_breach/actions/modify_shell_startup_files.py

@@ -58,8 +58,8 @@ class ModifyShellStartupFiles(PBA):
             def run(self):
                 if self.command:
                     try:
-                        output = subprocess.check_output(
-                            self.command, stderr=subprocess.STDOUT, shell=True  # noqa: DUO116
+                        output = subprocess.check_output(  # noqa: DUO116
+                            self.command, stderr=subprocess.STDOUT, shell=True
                         ).decode()
                         return output, True
                     except subprocess.CalledProcessError as e:

monkey/infection_monkey/post_breach/actions/use_signed_scripts.py

@@ -21,10 +21,9 @@ class SignedScriptProxyExecution(PBA):
         try:
             original_comspec = ""
             if is_windows_os():
-                original_comspec = subprocess.check_output(
+                original_comspec = subprocess.check_output(  # noqa: DUO116
                     "if defined COMSPEC echo %COMSPEC%", shell=True
-                ).decode()  # noqa: DUO116
-
+                ).decode()
             super().run()
         except Exception as e:
             LOG.warning(

monkey/infection_monkey/post_breach/pba.py

@@ -90,7 +90,7 @@ class PBA(Plugin):
         :return: Tuple of command's output string and boolean, indicating if it succeeded
         """
         try:
-            output = subprocess.check_output(
+            output = subprocess.check_output(  # noqa: DUO116
                 self.command, stderr=subprocess.STDOUT, shell=True
             ).decode()
             return output, True

monkey/infection_monkey/post_breach/shell_startup_files/windows/shell_startup_files_modification.py

@@ -13,8 +13,10 @@ def get_windows_commands_to_modify_shell_startup_files():
 
     # get list of usernames
     USERS = (
-        subprocess.check_output("dir C:\\Users /b", shell=True).decode().split("\r\n")[:-1]
-    )  # noqa: DUO116
+        subprocess.check_output("dir C:\\Users /b", shell=True)  # noqa: DUO116
+        .decode()
+        .split("\r\n")[:-1]
+    )
     USERS.remove("Public")
 
     STARTUP_FILES_PER_USER = [

monkey/infection_monkey/post_breach/signed_script_proxy/signed_script_proxy.py

@@ -15,7 +15,7 @@ def get_commands_to_proxy_execution_using_signed_script():
 
 def cleanup_changes(original_comspec):
     if is_windows_os():
-        subprocess.run(
+        subprocess.run(  # noqa: DUO116
             get_windows_commands_to_reset_comspec(original_comspec), shell=True
-        )  # noqa: DUO116
+        )
         subprocess.run(get_windows_commands_to_delete_temp_comspec(), shell=True)  # noqa: DUO116

monkey/infection_monkey/system_info/windows_info_collector.py

@@ -1,4 +1,5 @@
 import logging
+import shlex
 import subprocess
 import sys
 
@@ -51,10 +52,10 @@ class WindowsInfoCollector(InfoCollector):
     def get_installed_packages(self):
         LOG.info("Getting installed packages")
 
-        packages = subprocess.check_output("dism /online /get-packages", shell=True)
+        packages = subprocess.check_output(shlex.split("dism /online /get-packages"))
         self.info["installed_packages"] = packages.decode("utf-8", errors="ignore")
 
-        features = subprocess.check_output("dism /online /get-features", shell=True)
+        features = subprocess.check_output(shlex.split("dism /online /get-features"))
         self.info["installed_features"] = features.decode("utf-8", errors="ignore")
 
         LOG.debug("Got installed packages")

monkey/infection_monkey/transport/http.py

@@ -126,7 +126,7 @@ class HTTPConnectProxyHandler(http.server.BaseHTTPRequestHandler):
             LOG.info("Received bootloader's request: {}".format(post_data))
             try:
                 dest_path = self.path
-                r = requests.post(
+                r = requests.post(  # noqa: DUO123
                     url=dest_path,
                     data=post_data,
                     verify=False,

monkey/infection_monkey/utils/hidden_files.py

@@ -26,9 +26,9 @@ def get_commands_to_hide_folders():
 
 
 def cleanup_hidden_files(is_windows=is_windows_os()):
-    subprocess.run(
+    subprocess.run(  # noqa: DUO116
         get_windows_commands_to_delete()
-        if is_windows  # noqa: DUO116
+        if is_windows
         else " ".join(get_linux_commands_to_delete()),
         shell=True,
     )

monkey/infection_monkey/utils/linux/users.py

@@ -1,6 +1,6 @@
 import datetime
 import logging
-import os
+import shlex
 import subprocess
 
 from infection_monkey.utils.auto_new_user import AutoNewUser
@@ -43,18 +43,16 @@ class AutoNewLinuxUser(AutoNewUser):
         logger.debug(
             "Trying to add {} with commands {}".format(self.username, str(commands_to_add_user))
         )
-        _ = subprocess.check_output(
-            " ".join(commands_to_add_user), stderr=subprocess.STDOUT, shell=True
-        )
+        _ = subprocess.check_output(commands_to_add_user, stderr=subprocess.STDOUT)
 
     def __enter__(self):
         return self  # No initialization/logging on needed in Linux
 
     def run_as(self, command):
-        command_as_new_user = "sudo -u {username} {command}".format(
-            username=self.username, command=command
+        command_as_new_user = shlex.split(
+            "sudo -u {username} {command}".format(username=self.username, command=command)
         )
-        return os.system(command_as_new_user)
+        return subprocess.call(command_as_new_user)
 
     def __exit__(self, exc_type, exc_val, exc_tb):
         # delete the user.
@@ -64,6 +62,4 @@ class AutoNewLinuxUser(AutoNewUser):
                 self.username, str(commands_to_delete_user)
             )
         )
-        _ = subprocess.check_output(
-            " ".join(commands_to_delete_user), stderr=subprocess.STDOUT, shell=True
-        )
+        _ = subprocess.check_output(commands_to_delete_user, stderr=subprocess.STDOUT)

monkey/infection_monkey/utils/windows/users.py

@@ -39,7 +39,7 @@ class AutoNewWindowsUser(AutoNewUser):
 
         windows_cmds = get_windows_commands_to_add_user(self.username, self.password, True)
         logger.debug("Trying to add {} with commands {}".format(self.username, str(windows_cmds)))
-        _ = subprocess.check_output(windows_cmds, stderr=subprocess.STDOUT, shell=True)
+        _ = subprocess.check_output(windows_cmds, stderr=subprocess.STDOUT)
 
     def __enter__(self):
         # Importing these only on windows, as they won't exist on linux.
@@ -127,9 +127,7 @@ class AutoNewWindowsUser(AutoNewUser):
                     self.username, str(commands_to_deactivate_user)
                 )
             )
-            _ = subprocess.check_output(
-                commands_to_deactivate_user, stderr=subprocess.STDOUT, shell=True
-            )
+            _ = subprocess.check_output(commands_to_deactivate_user, stderr=subprocess.STDOUT)
         except Exception as err:
             raise NewUserError("Can't deactivate user {}. Info: {}".format(self.username, err))
 
@@ -141,8 +139,6 @@ class AutoNewWindowsUser(AutoNewUser):
                     self.username, str(commands_to_delete_user)
                 )
             )
-            _ = subprocess.check_output(
-                commands_to_delete_user, stderr=subprocess.STDOUT, shell=True
-            )
+            _ = subprocess.check_output(commands_to_delete_user, stderr=subprocess.STDOUT)
         except Exception as err:
             raise NewUserError("Can't delete user {}. Info: {}".format(self.username, err))

monkey/infection_monkey/windows_upgrader.py

@@ -1,4 +1,5 @@
 import logging
+import shlex
 import shutil
 import subprocess
 import sys
@@ -45,14 +46,20 @@ class WindowsUpgrader(object):
             opts.parent, opts.tunnel, opts.server, opts.depth
         )
 
+        # TODO: Replace all of this string templating with a function that accepts
+        #       the necessary parameters and returns a list of arguments.
         monkey_cmdline = (
             MONKEY_CMDLINE_WINDOWS % {"monkey_path": WormConfiguration.dropper_target_path_win_64}
             + monkey_options
         )
 
-        monkey_process = subprocess.Popen(
+        monkey_cmdline_split = shlex.split(
             monkey_cmdline,
-            shell=True,
+            posix=False,  # won't try resolving "\" in paths as part of escape sequences
+        )
+
+        monkey_process = subprocess.Popen(
+            monkey_cmdline_split,
             stdin=None,
             stdout=None,
             stderr=None,

monkey/monkey_island/Pipfile

@@ -36,6 +36,7 @@ mongomock = "==3.19.0"
 pytest = ">=5.4"
 requests-mock = "==1.8.0"
 black = "==20.8b1"
+dlint = "==0.11.0"
 flake8 = "==3.9.0"
 pytest-cov = "*"
 isort = "==5.8.0"

monkey/monkey_island/Pipfile.lock

@@ -1,7 +1,7 @@
 {
     "_meta": {
         "hash": {
-            "sha256": "9d575bcb35dba0389e8a68dd285f9ed7015fbcc4a42cffff5301777497337616"
+            "sha256": "4fdfe90af14139cf855a0363ad0acbe7fb307b35b038e2c099c4d1227322a13b"
         },
         "pipfile-spec": 6,
         "requires": {
@@ -171,7 +171,7 @@
                 "sha256:7d73d2a99753107a36ac6b455ee49046802e59d9d076ef8e47b61499fa29afff",
                 "sha256:e96da0d330793e2cb9485e9ddfd918d456036c7149416295932478192f4436a1"
             ],
-            "markers": "python_version != '3.4' and sys_platform == 'win32'",
+            "markers": "python_version != '3.4'",
             "version": "==0.4.3"
         },
         "coloredlogs": {
@@ -1076,7 +1076,7 @@
                 "sha256:7d73d2a99753107a36ac6b455ee49046802e59d9d076ef8e47b61499fa29afff",
                 "sha256:e96da0d330793e2cb9485e9ddfd918d456036c7149416295932478192f4436a1"
             ],
-            "markers": "python_version != '3.4' and sys_platform == 'win32'",
+            "markers": "python_version != '3.4'",
             "version": "==0.4.3"
         },
         "coverage": {
@@ -1144,6 +1144,13 @@
             ],
             "version": "==0.3.1"
         },
+        "dlint": {
+            "hashes": [
+                "sha256:e7297325f57e6b5318d88fba2497f9fea6830458cd5aecb36150856db010f409"
+            ],
+            "index": "pypi",
+            "version": "==0.11.0"
+        },
         "filelock": {
             "hashes": [
                 "sha256:18d82244ee114f543149c66a6e0c14e9c4f8a1044b5cdaadd0f82159d6a6ff59",

monkey/monkey_island/cc/server_utils/bootloader_server.py

@@ -33,9 +33,9 @@ class BootloaderHTTPRequestHandler(BaseHTTPRequestHandler):
         # The island server doesn't always have a correct SSL cert installed
         # (By default it comes with a self signed one),
         # that's why we're not verifying the cert in this request.
-        r = requests.post(
+        r = requests.post(  # noqa: DUO123
             url=island_server_path, data=post_data, verify=False, timeout=SHORT_REQUEST_TIMEOUT
-        )  # noqa: DUO123
+        )
 
         try:
             if r.status_code != 200:

monkey/tests/infection_monkey/utils/linux/test_users.py

@@ -9,7 +9,7 @@ TEST_USER = "test_user"
 
 @pytest.fixture
 def subprocess_check_output_spy(monkeypatch):
-    def mock_check_output(command, stderr, shell):
+    def mock_check_output(command, stderr):
         mock_check_output.command = command
 
     mock_check_output.command = ""
@@ -21,11 +21,11 @@ def subprocess_check_output_spy(monkeypatch):
 
 def test_new_user_expires(subprocess_check_output_spy):
     with (AutoNewLinuxUser(TEST_USER, "password")):
-        assert "--expiredate" in subprocess_check_output_spy.command
-        assert "--inactive 0" in subprocess_check_output_spy.command
+        assert "--expiredate" in " ".join(subprocess_check_output_spy.command)
+        assert "--inactive 0" in " ".join(subprocess_check_output_spy.command)
 
 
 def test_new_user_try_delete(subprocess_check_output_spy):
     with (AutoNewLinuxUser(TEST_USER, "password")):
         pass
-    assert f"deluser {TEST_USER}" in subprocess_check_output_spy.command
+    assert f"deluser {TEST_USER}" in " ".join(subprocess_check_output_spy.command)