T09 · Insecure Skill Coding Practices
Error
- Location
- agents/technical_analyst.md:11
- Finding
- User-Controlled Ticker Interpolated into Shell Commands<![CDATA[ ## Vulnerability Details **File Location**: - `SKILL.md:64-69` - `agents/fundamental_analyst.md:13-17` - `agents/technical_analyst.md:11-21` **Vulnerability Type**: OS command injection through unquoted template substitution **Risk Level**: High ### Vulnerable Code `SKILL.md:64-69` defines the ticker as a value extracted from the user's message without requiring validation: ```markdown Extract from the user's message: - **Ticker(s)**: The stock symbol(s) to analyze (e.g., NVDA, AAPL) - **Date context**: Whether they want current analysis or historical (default: today) - **Debate rounds**: If specified, how many bull/bear rounds (default: 1) - **Focus areas**: Any specific concerns (e.g., "worried about earnings", "considering for long-term hold") ``` `agents/fundamental_analyst.md:13-17` inserts that value directly into a shell command: ```markdown 1. **Run the market data script** to get financial statements and key metrics: ```bash cd {SKILL_PATH} && uv run scripts/fetch_market_data.py {TICKER} ``` ``` `agents/technical_analyst.md:11-21` repeats the unsafe pattern: ```markdown 1. **Run the market data script** to get price history: ```bash cd {SKILL_PATH} && uv run scripts/fetch_market_data.py {TICKER} ``` 2. **Run the technical indicators script**: ```bash cd {SKILL_PATH} && uv run scripts/technical_indicators.py {TICKER} ``` ``` ### Technical Analysis The `{TICKER}` placeholder is derived from untrusted user input and is inserted unquoted into Bash command templates. No validation or normalization rule limits this value to characters valid in a stock symbol. If the resulting command is passed to a shell, metacharacters such as semicolons, command substitutions, redirection operators, or logical operators are interpreted as shell syntax rather than as part of a ticker argument. Quoting alone would reduce the immediate risk, but argument-array execution combined with strict input validation is the safer desig ...[truncated 1522 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Validate the ticker before it reaches any command: - Use an explicit allowlist appropriate to supported exchanges. - A baseline rule could be `^[A-Za-z0-9.^-]{1,20}$`. - Reject control characters, whitespace, path separators, and shell metacharacters. - Where exchange-specific suffixes are supported, define them explicitly rather than broadening the rule indiscriminately. 2. Do not execute generated command strings through a shell. Invoke the process with a structured argument array equivalent to: ```python subprocess.run( ["uv", "run", "scripts/fetch_market_data.py", validated_ticker], cwd=skill_path, check=True, shell=False, ) ``` 3. Apply the same protection to the technical-indicator invocation. 4. Avoid generating executable Bash snippets in subagent prompts when the orchestration layer can invoke the scripts directly. 5. Resolve and validate `SKILL_PATH` separately. Pass it through a working-directory parameter instead of interpolating it into `cd`. 6. Add regression tests using ticker values containing semicolons, command substitutions, redirection operators, newlines, spaces, and path separators. Confirm that every invalid value is rejected before execution. ]]>