T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/precision_picker.py:55
- Finding
- Shell Command Injection Through Unvalidated Stock Codes<![CDATA[ ## Vulnerability Details **File Location**: `scripts/precision_picker.py:55-60`, `scripts/precision_picker.py:154`, `scripts/precision_picker.py:217`, and `scripts/precision_picker.py:693-701` **Vulnerability Type**: OS command injection **Risk Level**: High ### Vulnerable Code Candidate codes are accepted from command-line arguments or a user-supplied file without format validation: ```python if args.pool: codes = [c.strip() for c in args.pool.split(",") if c.strip()] candidates = [{"code": c, "name": ""} for c in codes] elif args.pool_file: with open(args.pool_file) as f: for line in f: line = line.strip() if not line: continue parts = line.split() code = parts[0] name = parts[1] if len(parts) > 1 else "" candidates.append({"code": code, "name": name}) ``` The resulting values are directly interpolated into command strings: ```python cmd = f"npx -y westock-data-skillhub@1.0.5 finance {','.join(working_codes)} --num 1" stdout, rc, success = run_cmd(cmd, cwd=str(SKILL_DIR), timeout=90) ``` ```python code_list = ",".join(codes) cmd = f"npx -y westock-data-skillhub@1.0.5 fund flow {code_list}" stdout, rc, success = run_cmd(cmd, cwd=str(SKILL_DIR), timeout=90) ``` These command strings are executed through a shell: ```python def run_cmd(cmd, cwd=None, timeout=60): """Execute a command and return stdout and a success indicator.""" try: result = subprocess.run( cmd, shell=True, capture_output=True, text=True, timeout=timeout, cwd=cwd ) return result.stdout.strip(), result.returncode, True except subprocess.TimeoutExpired: return "", -1, False except Exception: return "", -1, False ``` ### Technical Analysis The code treats stock identifiers as trusted command fragments. No strict allowlist, regular-expression validation, argument escaping, or shell-free process i ...[truncated 2655 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Eliminate shell interpretation and pass arguments as a list: ```python result = subprocess.run( [ "npx", "-y", "westock-data-skillhub@1.0.5", "finance", ",".join(working_codes), "--num", "1" ], shell=False, capture_output=True, text=True, timeout=90, cwd=str(SKILL_DIR), check=False, ) ``` Apply the same change to the fund-flow invocation. 2. Validate every candidate before it reaches any processing layer. For the currently supported exchanges, use a strict allowlist such as: ```python import re STOCK_CODE_RE = re.compile(r"^(?:sh|sz)\d{6}$") def validate_stock_code(code): if not STOCK_CODE_RE.fullmatch(code): raise ValueError(f"Invalid stock code: {code!r}") return code ``` 3. Apply validation consistently to all input sources: - `--pool` - `--pool-file` - Automatically loaded VPS JSON signals - Any future API or scheduled-task input 4. Do not allow the Layer 1 offline fallback to bypass identifier validation. Syntax validation must occur before external market-data checks and must remain mandatory even when dependencies are unavailable. 5. Add security regression tests using candidates containing command separators, substitutions, redirects, whitespace, and newline characters. Verify that invalid values are rejected and that no shell is invoked. 6. Consider imposing limits on candidate count and input length to reduce denial-of-service and malformed-input risks. ]]>