T09 · Insecure Skill Coding Practices
Warning
- Location
- auto-rollback.sh:128
- Finding
- Environment-Controlled Paths Are Embedded into an Executable Rollback Script Without Shell Escaping<![CDATA[ ## Vulnerability Details **File Location**: `auto-rollback.sh`, lines 3-7 and 128-190 **Vulnerability Type**: Shell command injection through generated source code **Risk Level**: Medium ### Vulnerable Code ```bash OPENCLAW_HOME_DIR="${OPENCLAW_HOME_DIR:-$HOME/.openclaw}" STATE_FILE="${STATE_FILE:-$OPENCLAW_HOME_DIR/state/rollback-pending.json}" CONFIG_FILE="${CONFIG_FILE:-$OPENCLAW_HOME_DIR/openclaw.json}" BACKUP_DIR="${BACKUP_DIR:-$OPENCLAW_HOME_DIR}" LOG_FILE="${LOG_FILE:-$OPENCLAW_HOME_DIR/logs/rollback.log}" ``` These environment-controlled values are subsequently inserted into a generated executable script: ```bash write_rollback_script() { local rollback_script="$1" local backup_file="$2" cat > "$rollback_script" <<EOF #!/bin/bash OPENCLAW_HOME_DIR="$OPENCLAW_HOME_DIR" STATE_FILE="$STATE_FILE" LOG_FILE="$LOG_FILE" GATEWAY_PORT="$GATEWAY_PORT" LAUNCHD_LABEL="$LAUNCHD_LABEL" OPENCLAW_CMD="$OPENCLAW_CMD" BACKUP_FILE="$backup_file" log() { local msg="[\$(date -Iseconds)] \$1" echo "\$msg" >> "\$LOG_FILE" echo "\$1" } check_gateway_health() { if ! pgrep -f "openclaw.*gateway" >/dev/null 2>&1; then return 1 fi if curl -s -o /dev/null -w "%{http_code}" "http://127.0.0.1:\$GATEWAY_PORT/health" 2>/dev/null | grep -q "200"; then return 0 fi return 1 } log "🚨 rollback task started" if check_gateway_health; then log "✅ Gateway is already healthy, cancelling rollback" rm -f "\$STATE_FILE" "\$OPENCLAW_HOME_DIR/\$LAUNCHD_LABEL.plist" "\$OPENCLAW_HOME_DIR/.rollback_execute.sh" exit 0 fi log "❌ Gateway still unhealthy, restoring backup: \$BACKUP_FILE" cp "\$BACKUP_FILE" "$CONFIG_FILE" || { log "❌ Failed to restore backup" exit 1 } log "🔄 Restarting Gateway" "\$OPENCLAW_CMD" gateway restart || log "❌ Gateway restart command failed" sleep 5 if check_gateway_health; then log "🎉 Rollback completed and Gateway is healthy" else log "⚠️ Rollback completed but Gat ...[truncated 2364 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Do not generate executable shell source containing dynamic path values. Use a static rollback script and pass values as positional arguments: ```bash /bin/bash rollback-static.sh "$STATE_FILE" "$CONFIG_FILE" "$backup_file" ``` 2. Prefer storing rollback metadata in a permission-restricted JSON file and reading it with `jq` from the static script. 3. If source generation cannot be removed, serialize every inserted value using a shell-safe mechanism such as: ```bash printf 'CONFIG_FILE=%q\n' "$CONFIG_FILE" ``` 4. Validate all environment-overridable paths before use. Reject control characters, newlines, null-equivalent input, and values outside approved directories. 5. Consider removing environment overrides for security-critical paths unless custom locations are an explicit requirement. 6. Create generated files with restrictive permissions and safe creation semantics, such as a restrictive `umask` and atomic file replacement. 7. Verify that the generated script and its parent directory are owned by the expected user and are not writable by other users before registering the launchd job. ]]>