T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/safe-gateway-update.sh:8
- Finding
- Command Injection Through Unvalidated Timeout Arithmetic<![CDATA[ ## Vulnerability Details **File Location**: `scripts/safe-gateway-update.sh`, lines 8 and 50 **Vulnerability Type**: Shell arithmetic-expression injection **Risk Level**: High ### Vulnerable Code ```bash TIMEOUT="${2:-30}" ``` ```bash for ((i=1; i<=$TIMEOUT; i++)); do STATUS=$(openclaw gateway status 2>/dev/null) if echo "$STATUS" | grep -q "RPC probe: ok"; then log "Gateway is back online and healthy (attempt $i)." SUCCESS=1 break fi sleep 1 done ``` ### Technical Analysis The second command-line argument is accepted without validating that it is a bounded decimal integer. It is subsequently inserted into a Bash arithmetic expression. Bash arithmetic expressions may recursively interpret variable contents as arithmetic syntax. Specially constructed expressions, including expressions containing array subscripts and command substitutions, can cause commands to be evaluated. Even when command execution is not achieved, negative, malformed, or extremely large values can bypass the intended health-check behavior or make the script run for an excessive period. The script modifies gateway configuration and restarts a service, making execution in its process context security-sensitive. ### Attack Path 1. An attacker or compromised automation invokes the script and controls its second argument. 2. The attacker supplies an arithmetic expression instead of a decimal timeout, such as an expression using an array subscript with command substitution. 3. The value is stored unchanged in `TIMEOUT`. 4. Bash evaluates the value when processing `((i=1; i<=$TIMEOUT; i++))`. 5. The embedded expression executes with the operating-system privileges of the account running the script. Alternatively, an extremely large integer can keep the polling loop active for an excessive duration. ### Impact Assessment Successful exploitation can execute arbitrary shell commands as the user running the Skill. That account already has a ...[truncated 398 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions Validate and normalize the timeout before using it: ```bash TIMEOUT="${2:-30}" if [[ ! "$TIMEOUT" =~ ^[1-9][0-9]*$ ]] || (( 10#$TIMEOUT > 300 )); then log "Error: Timeout must be an integer between 1 and 300 seconds." exit 1 fi TIMEOUT=$((10#$TIMEOUT)) ``` Use the validated numeric variable without another parameter expansion inside the arithmetic expression: ```bash for ((i = 1; i <= TIMEOUT; i++)); do # Health check done ``` Set a conservative upper bound appropriate for gateway startup so that callers cannot cause an unreasonably long execution. ]]>