T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/sector_hunter.py:20
- Finding
- Shell Command Injection Sink in Search Helper<![CDATA[ ## Vulnerability Details **File Location**: `scripts/sector_hunter.py`, lines 20-31 **Vulnerability Type**: Shell command injection **Risk Level**: Medium ### Vulnerable Code ```python script_path = "/root/.openclaw/workspace/skills/byted-web-search/scripts/web_search.py" cmd = f'python3 "{script_path}" "{query}" --count {count}' try: result = subprocess.run( cmd, shell=True, capture_output=True, text=True, timeout=30, cwd="/root/.openclaw/workspace/skills/byted-web-search" ) ``` ### Technical Analysis The `search()` function constructs a command string by interpolating `query` and `count`, then executes that string through a shell with `shell=True`. Quoting the query with double quotes does not prevent shell interpretation: command substitution and other shell constructs can still be evaluated inside double quotes. The current `main()` function generates the search query from fixed templates and an allowlisted sector keyword, which substantially limits direct exploitation through the documented command-line flow. Nevertheless, `search()` is a reusable function that accepts arbitrary arguments, and any future or external caller that passes attacker-controlled data can expose the command-injection sink. ### Attack Path 1. An attacker obtains influence over a value passed to `search()`, such as through a future API, plugin integration, or direct module invocation. 2. The attacker supplies shell syntax in `query` or `count`, such as command substitution. 3. The value is interpolated into the command string without shell-safe escaping. 4. `subprocess.run(..., shell=True)` passes the command to the system shell. 5. The shell evaluates the injected syntax and runs the attacker's command with the privileges of the Python process. The documented `main()` path does not currently pass the complete raw CLI query to `search()`, so exploitation requires another caller or a future code change that in ...[truncated 465 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions Avoid invoking a shell. Pass each command argument separately: ```python result = subprocess.run( ["python3", script_path, query, "--count", str(count)], shell=False, capture_output=True, text=True, timeout=30, cwd="/root/.openclaw/workspace/skills/byted-web-search", check=False, ) ``` Additionally: - Validate that `count` is an integer within an expected range. - Keep sector selection restricted to an explicit allowlist. - Resolve and verify the search script path before execution. - Avoid exposing `search()` to arbitrary input unless callers enforce equivalent validation. - Run the Skill using a dedicated, least-privileged operating-system account. ]]>