fix: improve WinRM validation and data handling

.pre-commit-config.yaml

@@ -8,13 +8,13 @@ default_stages: [pre-commit]
 # This is a template for connector pre-commit hooks
 repos:
   - repo: https://github.com/compilerla/conventional-pre-commit
-    rev: v4.1.0
+    rev: v4.4.0
     hooks:
       - id: conventional-pre-commit
         stages: [commit-msg]
         args: [--verbose]
   - repo: https://github.com/pre-commit/pre-commit-hooks
-    rev: v5.0.0
+    rev: v6.0.0
     hooks:
       - id: check-merge-conflict
       - id: end-of-file-fixer
@@ -27,13 +27,13 @@ repos:
       - id: check-json
       - id: check-yaml
   - repo: https://github.com/astral-sh/ruff-pre-commit
-    rev: v0.11.7
+    rev: v0.15.22
     hooks:
       - id: ruff
         args: [ "--fix", "--unsafe-fixes"] # Allow unsafe fixes (ruff pretty strict about what it can fix)
       - id: ruff-format
   - repo: https://github.com/djlint/djLint
-    rev: v1.36.4
+    rev: v1.40.8
     hooks:
       - id: djlint-reformat-django
       - id: djlint-django
@@ -43,12 +43,12 @@ repos:
       - id: soar-app-linter
         args: ["--single-repo", "--message-level", "error"]
   - repo: https://github.com/hukkin/mdformat
-    rev: 0.7.22
+    rev: 1.0.0
     hooks:
       - id: mdformat
         exclude: "release_notes/.*"
   - repo: https://github.com/returntocorp/semgrep
-    rev: v1.136.0
+    rev: v1.165.0
     hooks:
       - id: semgrep
         additional_dependencies: ["setuptools==81.0.0"]
@@ -61,7 +61,7 @@ repos:
         exclude: "README.md"
   # Central hooks
   - repo: https://github.com/phantomcyber/dev-cicd-tools
-    rev: v2.1.4
+    rev: v2.2.8
     hooks:
       - id: build-docs
         language: python

.pylintrc

@@ -0,0 +1,2 @@
+[MASTER]
+ignore=.venv

README.md

@@ -182,7 +182,7 @@ VARIABLE | REQUIRED | TYPE | DESCRIPTION
 -------- | -------- | ---- | -----------
 **endpoint** | optional | string | IP/Hostname (For TEST CONNECTIVITY and default, if not provided in an action) |
 **verify_server_cert** | optional | boolean | Verify Server Certificate |
-**default_protocol** | optional | string | Default protocol for actions |
+**default_protocol** | optional | string | Default protocol for actions. Basic authentication over HTTP exposes credentials in cleartext. |
 **default_port** | optional | numeric | Default port for actions |
 **domain** | optional | string | Domain |
 **username** | required | string | Username |

parse_callbacks.py

@@ -39,6 +39,12 @@ def basic(action_result, response):
     data["std_err"] = response.std_err
 
     action_result.add_data(data)
+    if response.status_code:
+        std_err = response.std_err.decode("utf-8", errors="replace") if isinstance(response.std_err, bytes) else response.std_err
+        return action_result.set_status(
+            phantom.APP_ERROR,
+            f"Command failed with exit code {response.status_code}: {clean_str(std_err)}",
+        )
     return phantom.APP_SUCCESS
 
 
@@ -352,11 +358,17 @@ def _parse_rule(rule):
 def list_applocker_policies(action_result, response):
     if response.status_code:
         return action_result.set_status(phantom.APP_ERROR, f"Error running command: {clean_str(response.std_err)}")
+    xml_response = response.std_out
+    if len(xml_response) > 20 * 1024 * 1024:
+        return action_result.set_status(phantom.APP_ERROR, "AppLocker XML response exceeds the 20 MiB safety limit")
+    if isinstance(xml_response, bytes):
+        xml_response = xml_response.decode("utf-8", errors="replace")
+    if "<!DOCTYPE" in xml_response.upper():
+        return action_result.set_status(phantom.APP_ERROR, "AppLocker XML response contains a prohibited document type declaration")
+
     try:
         # Get rid of all the linebreaks to prevent errors during reading
-        data = xmltodict.parse("".join(response.std_out.splitlines()))
-    except TypeError:
-        data = xmltodict.parse("".join((response.std_out.decode("utf-8")).splitlines()))
+        data = xmltodict.parse("".join(xml_response.splitlines()))
     except Exception as e:
         return action_result.set_status(phantom.APP_ERROR, f"Error parsing XML response: {e!s}")
 

release_notes/unreleased.md

@@ -1 +1,6 @@
 **Unreleased**
+
+* Use literal PowerShell path arguments for copy and delete file actions.
+* Default WinRM assets to certificate-validated HTTPS connections.
+* Reject oversized AppLocker XML and documents containing type declarations.
+* Report nonzero run command and run script exit codes as action failures.

requirements.txt

@@ -1,3 +1,3 @@
-pykerberos==1.2.4
+pykerberos==1.2.4; sys_platform == "linux"
 pywinrm==0.5.0
 # Pinned to the current platform version

winrm.json

@@ -30,13 +30,13 @@
         "verify_server_cert": {
             "description": "Verify Server Certificate",
             "data_type": "boolean",
-            "default": false,
+            "default": true,
             "order": 1
         },
         "default_protocol": {
-            "description": "Default protocol for actions",
+            "description": "Default protocol for actions. Basic authentication over HTTP exposes credentials in cleartext.",
             "data_type": "string",
-            "default": "http",
+            "default": "https",
             "order": 2,
             "value_list": [
                 "http",
@@ -46,7 +46,7 @@
         "default_port": {
             "description": "Default port for actions",
             "data_type": "numeric",
-            "default": 5985,
+            "default": 5986,
             "order": 3
         },
         "domain": {
@@ -3326,7 +3326,7 @@
             },
             {
                 "module": "cryptography",
-                "input_file": "wheels/py3/cryptography-46.0.3-cp38-abi3-manylinux_2_28_x86_64.whl"
+                "input_file": "wheels/py39/cryptography-49.0.0-cp39-abi3-manylinux_2_28_x86_64.whl"
             },
             {
                 "module": "pycparser",
@@ -3338,7 +3338,7 @@
             },
             {
                 "module": "pyspnego",
-                "input_file": "wheels/py3/pyspnego-0.12.0-py3-none-any.whl"
+                "input_file": "wheels/py3/pyspnego-0.12.1-py3-none-any.whl"
             },
             {
                 "module": "pywinrm",
@@ -3350,31 +3350,31 @@
             },
             {
                 "module": "typing_extensions",
-                "input_file": "wheels/py3/typing_extensions-4.15.0-py3-none-any.whl"
+                "input_file": "wheels/py3/typing_extensions-4.16.0-py3-none-any.whl"
             }
         ]
     },
     "pip313_dependencies": {
         "wheel": [
             {
                 "module": "cffi",
-                "input_file": "wheels/py313/cffi-2.0.0-cp313-cp313-manylinux2014_x86_64.manylinux_2_17_x86_64.manylinux_2_28_x86_64.whl"
+                "input_file": "wheels/py313/cffi-2.1.0-cp313-cp313-manylinux2014_x86_64.manylinux_2_17_x86_64.manylinux_2_28_x86_64.whl"
             },
             {
                 "module": "cryptography",
-                "input_file": "wheels/py3/cryptography-46.0.3-cp311-abi3-manylinux_2_28_x86_64.whl"
+                "input_file": "wheels/py3/cryptography-49.0.0-cp311-abi3-manylinux_2_28_x86_64.whl"
             },
             {
                 "module": "pycparser",
-                "input_file": "wheels/py3/pycparser-2.23-py3-none-any.whl"
+                "input_file": "wheels/py3/pycparser-3.0-py3-none-any.whl"
             },
             {
                 "module": "pykerberos",
                 "input_file": "wheels/py313/pykerberos-1.2.4-cp313-cp313-manylinux_2_27_x86_64.manylinux_2_28_x86_64.whl"
             },
             {
                 "module": "pyspnego",
-                "input_file": "wheels/py3/pyspnego-0.12.0-py3-none-any.whl"
+                "input_file": "wheels/py3/pyspnego-0.12.1-py3-none-any.whl"
             },
             {
                 "module": "pywinrm",

winrm_connector.py

@@ -143,7 +143,7 @@ def _get_vault_file_text(self, action_result, vault_id):
         # Get the contents of a file in the vault
 
         try:
-            success, message, file_info = phantom_rules.vault_info(vault_id=vault_id)
+            _success, _message, file_info = phantom_rules.vault_info(vault_id=vault_id)
             if not file_info:
                 return action_result.set_status(phantom.APP_ERROR, consts.WINRM_ERROR_INVALID_VAULT_ID), None
             file_path = next(iter(file_info)).get("path")
@@ -164,7 +164,7 @@ def _get_custom_parser_method(self, action_result, vault_id):
             return phantom.APP_SUCCESS, pc.basic
 
         try:
-            success, message, file_info = phantom_rules.vault_info(vault_id=vault_id)
+            _success, _message, file_info = phantom_rules.vault_info(vault_id=vault_id)
             if not file_info:
                 return action_result.set_status(phantom.APP_ERROR, consts.WINRM_ERROR_INVALID_VAULT_ID), None
             file_path = next(iter(file_info)).get("path")
@@ -198,7 +198,20 @@ def _sanitize_string(self, string):
         # To avoid any shenanigans, we need to quote the arguments
         # The breaking character in PS is '`', so first we break any breaking characters, then we
         # break any double quotes which are found, then we break any $, which is used to declare variables
-        return string.replace("`", "``").replace('"', '`"').replace("$", "`$").replace("&", "`&").replace(")", "`)").replace("(", "`(")
+        return (
+            string.replace("`", "``")
+            .replace('"', '`"')
+            .replace("$", "`$")
+            .replace("&", "`&")
+            .replace(")", "`)")
+            .replace("(", "`(")
+            .replace(";", "`;")
+            .replace("|", "`|")
+            .replace("<", "`<")
+            .replace(">", "`>")
+            .replace("\r", "`r")
+            .replace("\n", "`n")
+        )
 
     def _get_fips_enabled(self):
         fips_enabled = is_fips_enabled()
@@ -237,9 +250,9 @@ def _create_ps_script(self, action_result, args, whitelist_args=set(), cmd_prefi
     def _init_session(self, action_result, param=None):
         config = self.get_config()
 
-        default_protocol = config.get(consts.WINRM_CONFIG_PROTOCOL, "http")
+        default_protocol = config.get(consts.WINRM_CONFIG_PROTOCOL, "https")
         ret_val, default_port = self._validate_integer(
-            action_result, config.get(consts.WINRM_CONFIG_PORT, 5985 if default_protocol == "http" else 5986), "Default port", True
+            action_result, config.get(consts.WINRM_CONFIG_PORT, 5986 if default_protocol == "https" else 5985), "Default port", True
         )
         if phantom.is_fail(ret_val):
             return action_result.get_status()
@@ -259,7 +272,7 @@ def _init_session(self, action_result, param=None):
         transport = config.get(consts.WINRM_CONFIG_TRANSPORT)
         domain = self._handle_py_ver_compat_for_input_str(config.get(consts.WINRM_CONFIG_DOMAIN))
 
-        verify_bool = config.get(phantom.APP_JSON_VERIFY, False)
+        verify_bool = config.get(phantom.APP_JSON_VERIFY, True)
         cert_pem_path = None
         cert_key_pem_path = None
         cert_ca_trust_path = config.get(consts.WINRM_CONFIG_CA_TRUST, "legacy_requests")
@@ -270,6 +283,8 @@ def _init_session(self, action_result, param=None):
             verify = "ignore"
 
         if transport == "basic" or transport == "plaintext":
+            if endpoint.lower().startswith("http://"):
+                self.save_progress("Warning: Basic or plaintext transport over HTTP exposes credentials in cleartext")
             if domain:
                 self.save_progress("Warning: Domain is set but transport type is set to 'basic'")
         elif transport == "ntlm":
@@ -900,7 +915,7 @@ def _handle_send_file(self, param):
 
         try:
             vault_id = self._handle_py_ver_compat_for_input_str(param["vault_id"])
-            success, message, file_info = phantom_rules.vault_info(vault_id=vault_id)
+            _success, _message, file_info = phantom_rules.vault_info(vault_id=vault_id)
             if not file_info:
                 return action_result.set_status(phantom.APP_ERROR, consts.WINRM_ERROR_INVALID_VAULT_ID)
             path = next(iter(file_info)).get("path")
@@ -946,7 +961,7 @@ def _handle_copy_file(self, param):
         path_from = self._handle_py_ver_compat_for_input_str(param["from"])
         path_to = self._handle_py_ver_compat_for_input_str(param["to"])
 
-        ps_script = f"& copy {self._sanitize_string(path_from)} {self._sanitize_string(path_to)}"
+        ps_script = f'& copy "{self._sanitize_string(path_from)}" "{self._sanitize_string(path_to)}"'
 
         ret_val = self._run_ps(action_result, ps_script, parse_callback=pc.check_exit_no_data2)
         if phantom.is_fail(ret_val):
@@ -962,7 +977,7 @@ def _handle_delete_file(self, param):
         file_path = self._handle_py_ver_compat_for_input_str(param["file_path"])
         force_delete = "-Force " if param.get("force") else ""
 
-        ps_script = f"& del {force_delete}{self._sanitize_string(file_path)}"
+        ps_script = f'& del {force_delete}"{self._sanitize_string(file_path)}"'
 
         ret_val = self._run_ps(action_result, ps_script, parse_callback=pc.check_exit_no_data2)
         if phantom.is_fail(ret_val):