T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/lib.sh:43
- Finding
- Operator Override Is Silently Ignored by Fallback Calldata## Vulnerability Details **File Location**: `scripts/lib.sh`, lines 43–67 **Vulnerability Type**: Inconsistent ABI encoding caused by hardcoded fallback calldata **Risk Level**: Medium ### Vulnerable Code ```bash set_pet_operator_calldata() { local approved="$1" case "$approved" in true|false) ;; *) err "approved must be true|false" ;; esac if command -v cast >/dev/null 2>&1; then local data data="$(cast calldata "setPetOperatorForAll(address,bool)" "$AAI_OPERATOR" "$approved" 2>/dev/null || true)" if [[ -n "$data" && "$data" == 0x* ]]; then printf '%s\n' "$data" return 0 fi fi if [[ "$approved" == "true" ]]; then printf '%s\n' "0xcd675d57000000000000000000000000b96b48a6b190a9d509ce9312654f34e9770f21100000000000000000000000000000000000000000000000000000000000000001" else printf '%s\n' "0xcd675d57000000000000000000000000b96b48a6b190a9d509ce9312654f34e9770f21100000000000000000000000000000000000000000000000000000000000000000" fi } ``` ### Technical Analysis The Skill documents `AAI_OPERATOR` as an overridable environment variable. The primary encoding path correctly uses the current value of `$AAI_OPERATOR`. However, if `cast` is unavailable or `cast calldata` fails, the fallback transaction data embeds the default AAI operator address, `0xb96B48a6B190A9d509cE9312654F34E9770F2110`, rather than the configured address. The transaction-generation scripts display `$AAI_OPERATOR` separately from the generated calldata. Consequently, their human-readable output can identify one operator while the actual calldata grants or revokes permission for another operator. A user who reviews only the displayed operator and destination may sign a transaction whose effective authorization differs from the stated authorization. Although the generated transaction grants pet-operator rights rather than NFT transfer rights, it applies through `setPetOperatorForAll` to all of the owner's Aavegotchis. T ...[truncated 1507 chars]
- Remediation
- ## Remediation Suggestions 1. Remove the hardcoded fallback and fail closed if ABI encoding cannot be performed: ```bash require_bin cast cast calldata \ "setPetOperatorForAll(address,bool)" \ "$AAI_OPERATOR" \ "$approved" ``` 2. If an offline fallback is required, dynamically ABI-encode the validated current value of `$AAI_OPERATOR` rather than embedding a fixed address. 3. Validate `AAI_OPERATOR` with `is_eth_address` before generating transaction data. 4. Decode or independently verify the generated calldata before displaying it. Confirm that: - the selector is for `setPetOperatorForAll(address,bool)`; - the encoded operator equals `$AAI_OPERATOR`; - the encoded Boolean equals the requested operation. 5. Abort on any encoding or verification failure instead of suppressing the error with `2>/dev/null || true`. 6. Add automated tests covering default and overridden operator addresses for both approval and revocation, including simulated `cast` failure. 7. Display a decoded summary derived from the final calldata so the human-readable operator cannot diverge from the actual transaction payload.