T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/db-reset.py:33
- Finding
- Shell Command Injection Through DATABASE_URL<![CDATA[ ## Vulnerability Details **File Location**: `scripts/db-reset.py`, lines 33–42 and 49–94 **Vulnerability Type**: OS command injection **Risk Level**: High ### Vulnerable Code ```python def run_command(command, capture=False): """运行 shell 命令""" try: if capture: result = subprocess.run(command, shell=True, capture_output=True, text=True) return result.returncode, result.stdout, result.stderr else: subprocess.run(command, shell=True) return 0, "", "" except Exception as e: print_colored(f"Error: {e}", Colors.RED) return 1, "", str(e) ``` ```python def check_env(): """检查环境变量""" db_url = os.environ.get('DATABASE_URL') if not db_url: if os.path.exists('.env'): with open('.env', 'r') as f: for line in f: if line.startswith('DATABASE_URL='): db_url = line.split('=', 1)[1].strip() break ``` ```python # 执行 cmd = f"psql {db_url} -f /tmp/drop_tables.sql" returncode, stdout, stderr = run_command(cmd, capture=True) ``` ### Technical Analysis The database URL is read from an environment variable or `.env` file, interpolated directly into a command string, and passed to `subprocess.run` with `shell=True`. No shell escaping or argument separation is applied. Consequently, shell metacharacters in `DATABASE_URL`, including `;`, `&&`, command substitutions, pipes, or redirections, are interpreted by the shell rather than being passed only to `psql`. ### Attack Path 1. An attacker gains the ability to influence `DATABASE_URL`, the project `.env` file, or the environment from which the script is launched. 2. The attacker supplies a value containing a valid-looking connection string followed by shell syntax, such as: ```text postgresql://user:pass@localhost/db; attacker-command # ``` 3. A developer or automation process invokes `scripts/db-reset.py`. 4. ...[truncated 688 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions - Remove `shell=True` and pass arguments as a list: ```python subprocess.run( ["psql", db_url, "-f", sql_path], check=True, capture_output=True, text=True, ) ``` - Replace the generic string-based `run_command` function with an API that accepts only an argument list. - Parse `DATABASE_URL` with a PostgreSQL URI parser and reject malformed or unsupported schemes. - Do not attempt to solve this only through shell quoting; avoiding shell interpretation is the safer control. - Protect `.env` from untrusted modification and apply restrictive filesystem permissions. - Use `check=True` or explicitly inspect every subprocess return code. ]]>