T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/quicker_connector.py:570
- Finding
- Windows Command Injection Through shell=True<![CDATA[ ## Vulnerability Details **File Location**: `scripts/quicker_connector.py:570-620` **Vulnerability Type**: OS command injection **Risk Level**: High ### Vulnerable Code ```python cmd = [self.starter_path] if wait_for_result: cmd.append(f"-c{timeout}") if action_identifier.startswith('quicker:'): action_cmd = action_identifier else: action_cmd = f"runaction:{action_identifier}" if parameters: action_cmd = f"{action_cmd}?{parameters}" cmd.append(action_cmd) if wait_for_result: process = subprocess.Popen( cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True, encoding='utf-8', shell=True ) else: subprocess.Popen( cmd, shell=True, creationflags=subprocess.DETACHED_PROCESS ) ``` ### Technical Analysis `action_identifier` and `parameters` are incorporated into a command passed to `subprocess.Popen` with `shell=True`. On Windows, this causes the command to be processed through the command shell. Shell metacharacters contained in either value may therefore be interpreted as command separators or redirection operators rather than as literal Quicker arguments. Although the normal CLI path usually selects an action ID from a local CSV or database, the public methods `execute_action`, `run_action`, `run_by_id`, and `run_by_name` accept caller-controlled strings directly. The code does not implement the argument validation claimed by the Skill documentation. The executable path check does not prevent this vulnerability. Verifying that `self.starter_path` points to an existing `QuickerStarter.exe` does not stop the shell from interpreting additional commands embedded in later arguments. ### Attack Path 1. An untrusted caller, agent instruction, integration, or imported Python module obtains access to `QuickerConnector.execute_action` or `QuickerActionRunner.run_action`. 2. The caller supplies an action identifier or parameter containing Win ...[truncated 750 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions - Set `shell=False`, which is already the default, and continue passing arguments as a list. - Reject action identifiers that do not match an explicit Quicker ID or URI grammar. - Validate parameter names and values separately instead of concatenating a query string manually. - Resolve and compare the executable path against an explicit allowlist using canonical paths. - Do not permit arbitrary action names, URIs, or parameters to reach the process boundary without validation. - Add regression tests containing Windows shell metacharacters such as `&`, `|`, `<`, `>`, `%`, `^`, and line breaks. - Consider using an official Quicker API or IPC interface that does not involve a command shell. ]]>