T09 · Insecure Skill Coding Practices
Error
- Location
SKILL.md:80- Finding
Shell Injection Through Sourceable Telemetry State
- Content
View full analysis
~/.gstack/analytics/.tel-design-panel-"$_SESSION_ID".sh </dev/null || true ``` The `_TEL` value originates from an external helper: ```bash _TEL=$(~/.claude/skills/gstack/bin/gstack-config get telemetry 2>/dev/null || echo "off") ``` ### Technical Analysis The Skill serializes runtime values by placing them directly inside a shell script. No shell escaping, validation, or allowlist is applied before `_TEL` is interpolated into the here-document. The resulting file is then executed with `source`. If `_TEL` contains quotation marks, command substitutions, newlines, or other shell syntax, it can terminate the intended assignment and introduce additional commands. For example, a malicious value structurally equivalent to the following would produce executable statements in the state file: ```text "; attacker_command; # ``` This is unsafe even if telemetry is normally expected to return a simple value such as `off`. The security boundary depends on the behavior and integrity of an external executable and its configuration, neither of which is validated by this Skill. The state file is also executable data stored under a persistent home-directory path. Any party capable of modifying it before the epilogue can inj ...[truncated 1396 chars]- Remediation
View remediation
"$STATE_FILE" ``` 3. Recover each field as data using `jq -r`, never as shell syntax. 4. Validate telemetry mode against a strict allowlist before use: ```bash case "$_TEL" in off|anonymous|full) ;; *) _TEL="off" ;; esac ``` 5. Create the analytics directory and state file with restrictive permissions: ```bash umask 077 mkdir -p -- "$HOME/.gstack/analytics" ``` 6. Use `mktemp` or an unpredictable, securely created state filename rather than constructing one directly. 7. If shell serialization is unavoidable, use a robust shell-escaping mechanism such as `printf '%q'`; however, structured non-executable storage is strongly preferred. ]]>