T09 · Insecure Skill Coding Practices
- Location
scripts/price.sh:39- Finding
Arbitrary Command Execution Through Bash Arithmetic Injection in Price Options
- Content
View full analysis
0" | bc -l 2>/dev/null || echo 0) )); then arrow="📈" elif (( $(echo "$change < 0" | bc -l 2>/dev/null || echo 0) )); then arrow="📉" else arrow="➡️" fi local sign="" (( $(echo "$change > 0" | bc -l 2>/dev/null || echo 0) )) && sign="+" printf "%s #%s %s: \$%'.2f (%s%.1f%%) | MCap: \$%'.0f | Vol: \$%'.0f\n" \ "$arrow" "$rank" "${symbol^^}" "$price" "$sign" "$change" "$(( $(printf '%.0f' "$mcap") ))" "$(( $(printf '%.0f' "$vol") ))" done ``` ```bash main() { while true; do DATA=$(fetch_prices) || { echo "❌ Failed to fetch prices"; exit 1; } case "$FORMAT" in json) output_json "$DATA" ;; csv) output_csv "$DATA" ;; *) output_text "$DATA" ;; esac [[ "$WATCH" -gt 0 ]] && sleep "$WATCH" || break done } ``` ### Technical Analysis The `--top` and `--watch` arguments are stored as arbitrary strings without checking that they contain only decimal digits. They are later evaluated by Bash arithmetic operators through ...[truncated 2130 chars]- Remediation
View remediation
&2 exit 2 fi } require_uint "top value" "$TOP" require_uint "watch interval" "$WATCH" ``` Apply explicit bounds to prevent excessive resource use: ```bash (( TOP >= 1 && TOP <= 100 )) || { echo "Top value must be between 1 and 100" >&2 exit 2 } (( WATCH == 0 || (WATCH >= 5 && WATCH <= 86400) )) || { echo "Watch interval must be 0 or between 5 and 86400 seconds" >&2 exit 2 } ``` Additional hardening should include: 1. Check that options requiring values actually have a following argument before reading `$2`. 2. Reject unknown options instead of treating all unrecognized arguments as coin identifiers. 3. Avoid passing untrusted text into Bash arithmetic contexts. 4. Add negative tests using malformed values, arithmetic syntax, array references, and command-substitution payloads. 5. Run the Skill as a non-privileged user with minimal filesystem and environment access. ]]>
