T09 · Insecure Skill Coding Practices
Error
- Location
scripts/state.sh:10- Finding
Arbitrary Command Execution Through Unsafe Username State File Sourcing
- Content
View full analysis
" >&2 exit 1 fi set_username "$username" echo "Stored Apple ID username: $username" ``` From `scripts/state.sh:10-13`: ```bash if [[ -f "$STATE_FILE" ]]; then # shellcheck disable=SC1090 source "$STATE_FILE" if [[ -n "${ICLOUD_FINDMY_USERNAME:-}" ]]; then ``` From `scripts/state.sh:22-27`: ```bash set_username() { local username="$1" ensure_state_dir cat > "$STATE_FILE" <- Remediation
View remediation
"$STATE_FILE" } get_username() { [[ -f "$STATE_FILE" ]] || return 1 local username IFS= read -r username < "$STATE_FILE" [[ -n "$username" ]] || return 1 printf '%s\n' "$username" } ``` 2. **Validate the input before storage.** Reject control characters, line breaks, NUL bytes, and values that do not conform to the expected Apple ID email format. Validation should supplement—not replace—removal of `source`. 3. **Protect state-file permissions.** Set a restrictive umask and explicitly enforce permissions: ```bash umask 077 mkdir -p "$STATE_DIR" chmod 700 "$STATE_DIR" printf '%s\n' "$username" > "$STATE_FILE" chmod 600 "$STATE_FILE" ``` 4. **Write state atomically.** Create a securely permissioned temporary file inside the state directory, write and validate the content, and then rename it over the destination. 5. **Avoid relying solely on shell escaping.** Although `printf '%q'` would reduce immediate exposure, treating the state file as executable shell code remains unnecessary and fragile. A non-executable data format is the preferred fix. 6. **Add regression tests** using values containing command substitutions, semicolons, quotes, backticks, variable expansions, and newline characters. Verify that retrieving a stored value never executes commands. ]]>