T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/common.sh:8
- Finding
- Arbitrary Shell Command Execution Through Unsafe Path Expansion<![CDATA[ ## Vulnerability Details **File Location**: `scripts/common.sh:8-13` **Vulnerability Type**: Shell command injection through `eval` **Risk Level**: High ### Vulnerable Code ```bash expand_path() { local input="$1" if [[ "$input" == ~* ]]; then eval "printf '%s' $input" else printf '%s' "$input" fi } ``` ### Technical Analysis The `expand_path` function passes a caller-controlled directory argument into `eval`. Although the intended behavior is tilde expansion, `eval` parses the resulting string as shell code. Consequently, command substitutions, variable expansions, redirections, separators, and other shell syntax contained in an argument beginning with `~` are evaluated. The function is reachable through `rotate_once.sh`, `list_images.sh`, and `install_launchagent.sh`. Quoting the argument when invoking these scripts does not prevent exploitation because the value is later reparsed by `eval`. ### Attack Path 1. An attacker persuades a user or agent to invoke one of the affected scripts with a crafted directory argument beginning with `~`. 2. The script assigns the untrusted argument to `DIR_INPUT`. 3. The script calls `expand_path "$DIR_INPUT"`. 4. `expand_path` constructs an `eval` expression containing the untrusted value. 5. Shell syntax embedded in the argument is evaluated before directory validation occurs. 6. The injected command executes as the user running the skill. The target directory does not need to pass `ensure_dir_exists` for command execution to occur because evaluation happens first. ### Impact Assessment Successful exploitation permits arbitrary command execution with the privileges of the current user. An attacker could read or modify files accessible to that user, access user-level secrets, install additional user-level persistence, or invoke other locally available applications and utilities. This issue does not independently provide root privileges, but it compromises the confidentiality, integrity, a ...[truncated 48 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions Remove `eval` entirely and implement narrowly scoped tilde expansion with ordinary parameter substitution: ```bash expand_path() { local input="$1" case "$input" in "~") printf '%s' "$HOME" ;; "~/"*) printf '%s/%s' "$HOME" "${input#\~/}" ;; *) printf '%s' "$input" ;; esac } ``` Additional hardening should include: 1. Reject unsupported forms such as `~otheruser` unless they are explicitly required and resolved through a safe account lookup. 2. Continue quoting every path expansion at its point of use. 3. Validate that the resolved path is an expected directory after expansion. 4. Add regression tests using arguments containing command substitutions, semicolons, spaces, quotes, and newline characters, verifying that none are evaluated. ]]>