T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/self_check.py:501
- Finding
- Shell Command Injection Through a Crafted Log Filename<![CDATA[ ## Vulnerability Details **File Location**: `scripts/self_check.py`, lines 501–502 **Vulnerability Type**: OS command injection through unsafe shell interpolation **Risk Level**: Medium ### Vulnerable Code The selected log path is interpolated into a shell command: ```python latest_log = log_files[0] code, stdout, _ = run_cmd(f"tail -100 '{latest_log}'") ``` The invoked helper enables shell interpretation by default: ```python def run_cmd(cmd: str, shell: bool = True, timeout: int = 30) -> Tuple[int, str, str]: """运行命令并返回 (returncode, stdout, stderr)""" try: result = subprocess.run( cmd, shell=shell, capture_output=True, text=True, timeout=timeout ) return result.returncode, result.stdout.strip(), result.stderr.strip() except subprocess.TimeoutExpired: return -1, "", "Timeout" except Exception as e: return -1, "", str(e) ``` ### Technical Analysis The script finds the newest `*.log` file in `~/.openclaw/logs` and embeds its full path directly into a command string passed to `subprocess.run()` with `shell=True`. Although the path is surrounded by single quotes, POSIX filenames may themselves contain single-quote characters, semicolons, redirection operators, and other shell metacharacters. A single quote in the filename can terminate the intended quoted argument, after which the remaining filename content is parsed as shell syntax. For example, a filename resembling the following can introduce an additional command: ```text audit';id>self-check-proof;#.log ``` The resulting command is structurally equivalent to: ```sh tail -100 '/home/user/.openclaw/logs/audit';id>self-check-proof;#.log' ``` Consequently, the shell runs both `tail` and the injected `id` command. The issue is caused by combining attacker-influenced filesystem data with string-based shell execution; quoting alone does not safely handle embedded quote characters. ### Attack Path 1. An attacker obtains the ...[truncated 1702 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions Do not pass the log path through a command shell. Invoke `tail` using an argument list and explicitly disable shell processing: ```python try: completed = subprocess.run( ["tail", "-100", str(latest_log)], shell=False, capture_output=True, text=True, timeout=30, ) code = completed.returncode stdout = completed.stdout.strip() except subprocess.TimeoutExpired: code, stdout = -1, "" ``` A broader hardening plan should include: 1. Refactor `run_cmd()` to accept a sequence of arguments and use `shell=False` by default: ```python def run_cmd(cmd: list[str], timeout: int = 30) -> Tuple[int, str, str]: result = subprocess.run( cmd, shell=False, capture_output=True, text=True, timeout=timeout, ) return result.returncode, result.stdout.strip(), result.stderr.strip() ``` 2. Convert commands that do not require shell features into argument arrays. 3. Replace shell pipelines and redirections with Python processing or explicitly connected subprocesses. 4. Avoid treating shell quoting as a substitute for argument-safe process invocation. 5. Add a regression test using a log filename containing quotes, semicolons, spaces, and redirection characters, verifying that no additional command executes. 6. Restrict permissions on `~/.openclaw/logs` so that only the intended user and trusted logging processes can create or rename files there. ]]>