T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/session-closeout.sh:54
- Finding
- Unsanitized Repository Paths Enable Structured-Output and Terminal Injection## Vulnerability Details **File Location**: `scripts/session-closeout.sh`, lines 54–56 and 134–135 **Vulnerability Type**: Unsanitized output generated from attacker-controlled filenames **Risk Level**: Medium ### Vulnerable Code ```bash GIT_DIRS=() while IFS= read -r d; do GIT_DIRS+=("$d"); done < <( find "$ROOT" -maxdepth 4 -type d -name .git 2>/dev/null | sed 's#/\.git$##' | sort ) ``` ```bash if ((${#DIRTY[@]} > 0)); then echo "DIRTY_REPOS:" for d in "${DIRTY[@]}"; do echo "- $(rel_path "$d")"; done fi ``` ### Technical Analysis Repository paths are derived from directory names in the audited workspace, which may be controlled by an attacker. Unix filenames can contain newlines, carriage returns, tabs, and terminal escape characters. Shell quoting prevents these paths from being interpreted as shell commands, so this is not command injection. However, the script emits each path without encoding or filtering it, despite presenting its output as a line-oriented report containing `key=value` records. A crafted repository directory name can consequently: - Inject additional lines that resemble trusted report fields. - Inject false exception or status information. - Alter terminal rendering through ANSI escape sequences. - Cause downstream parsers to misinterpret report boundaries. The newline-delimited discovery pipeline is also unable to represent filenames containing newlines reliably. Such names may be split into multiple array entries before report generation. ### Attack Path 1. An attacker creates or supplies a workspace containing a dirty Git repository within the script's four-level search depth. 2. The repository's parent directory is assigned a crafted name containing a newline, carriage return, or ANSI terminal escape sequence. 3. The script discovers the `.git` directory and processes its path through the newline-delimited `find`, `sed`, and `sort` pipeline. 4. If `git status --porcelain` reports changes, the corresponding repository ...[truncated 724 chars]
- Remediation
- ## Remediation Suggestions Use an unambiguous serialization format for all workspace-derived values: 1. Replace newline-delimited path discovery with NUL-delimited processing, such as `find ... -print0`, a NUL-aware sorting mechanism, and `read -r -d ''`. 2. Encode repository paths before including them in structured output. Suitable options include: - JSON string encoding. - Percent encoding. - Base64 with an explicitly documented field format. 3. If human-readable output is required, escape all control characters, including newlines, carriage returns, tabs, and bytes in the terminal escape range. 4. Avoid mixing machine-readable `key=value` output with unescaped free-form records. Prefer one JSON object containing arrays of encoded paths. 5. Document the selected encoding and require consumers to decode fields rather than treating raw lines as trusted records. 6. Add regression tests using repository names containing newlines, carriage returns, tabs, leading field-like text, and ANSI escape sequences.