T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/manage.py:22
- Finding
- Shell Command Injection Through the BOT_DIR Environment Variable<![CDATA[ ## Vulnerability Details **File Location**: `scripts/manage.py`, lines 22 and 56-64 **Vulnerability Type**: OS command injection **Risk Level**: High ### Evidence ```python BOT_DIR = Path(os.environ.get("BOT_DIR", str(_DEFAULT_BOT_DIR))) ``` ```python cmd = ( f"cd {BOT_DIR} && " f"python3 {ORCHESTRATOR} >> {LOG_FILE} 2>&1" ) result = subprocess.run( ["tmux", "new-session", "-d", "-s", TMUX_SESSION, cmd], capture_output=True, text=True, ) ``` ### Technical Analysis `BOT_DIR` is obtained from an environment variable and interpolated directly into a command string without validation or shell quoting. The string is supplied to `tmux new-session` as the session command. Tmux executes such command strings through a shell, so shell metacharacters in `BOT_DIR` are interpreted as command syntax rather than as part of a directory name. Although `subprocess.run` itself receives an argument list, this does not prevent injection because the final argument is subsequently interpreted as a shell command by tmux. ### Attack Path 1. An attacker gains control over the environment used to invoke the management script, or persuades an operator or Agent to configure a malicious `BOT_DIR`. 2. The attacker assigns a value containing shell syntax, for example: ```bash export BOT_DIR='/tmp/nonexistent; touch /tmp/invoice-bot-compromised #' ``` 3. The operator or Agent invokes: ```bash python3 scripts/manage.py start ``` 4. The generated command becomes equivalent to: ```bash cd /tmp/nonexistent; touch /tmp/invoice-bot-compromised # && python3 invoice_orchestrator.py ... ``` 5. Tmux passes the command to a shell, which executes the injected command. ### Impact Assessment Successful exploitation provides arbitrary command execution with the privileges of the user running `manage.py`. An attacker could read or modify files available to that user, access Feishu credentials in `.env`, alter the external invoice bot, submit ...[truncated 225 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions - Do not construct a shell command containing `BOT_DIR`. - Launch the orchestrator directly with an argument array and set its working directory through `cwd`. - If tmux is required, invoke a fixed wrapper script whose path and arguments are not interpreted as shell syntax. - If a shell command cannot be avoided, apply `shlex.quote` separately to every dynamic path. Quoting should be defense in depth rather than the primary design. - Resolve the path with `Path.resolve()`, require it to be an existing directory, and verify that `invoice_orchestrator.py` is a regular file in that directory. - Where deployment permits, restrict `BOT_DIR` to an approved base directory or exact configured path. - Add tests using values containing spaces, semicolons, command substitutions, redirections, and newline characters. A safer direct-process design would resemble: ```python subprocess.Popen( ["python3", str(BOT_DIR / ORCHESTRATOR)], cwd=BOT_DIR, stdout=log_handle, stderr=subprocess.STDOUT, ) ``` ]]>