T09 · Insecure Skill Coding Practices
Error
- Location
- SKILL.md:89
- Finding
- Transaction verification can miss dangerous token approvals<![CDATA[ ## Vulnerability Details **File Location**: `SKILL.md`, lines 89-103 **Vulnerability Type**: Incomplete authorization-state verification **Risk Level**: High ### Vulnerable Code Snippet ```markdown ### Response fields | Field | Description | | --- | --- | | `result.assetChanges` | ERC-20 token transfers: token address, from, to, amount | | `result.balanceChanges` | Native token (ETH) balance changes per address | ### Verification logic After Tenderly simulation, check the following before approving execution: 1. **Token destination** — do output tokens land in the expected recipient address? Flag any tokens going to an unexpected address. 2. **Token identity** — is the output token what was requested? Flag substitutions. 3. **Output amount** — is the output within the expected range (accounting for slippage)? Flag if materially lower than quoted. 4. **Input drain** — does the simulation drain more input token than authorized? Flag any excess. 5. **Unexpected approvals** — does the calldata grant approvals beyond what was declared? Flag unlimited or unexpected approvals. ``` ### Technical Analysis The documented verification process relies on `result.assetChanges` and `result.balanceChanges`. These fields describe immediate ERC-20 transfers and native-token balance changes, but the skill does not specify any mechanism for examining authorization-state changes. Calls such as ERC-20 `approve`, ERC-721 or ERC-1155 operator approvals, Permit/Permit2 authorizations, and approvals embedded in nested contract calls can grant future asset-transfer authority without causing an immediate balance change. Consequently, a simulation may show no unexpected asset movement even though the transaction grants an attacker or malicious contract permission to transfer assets later. Although the fifth verification rule requires the skill to detect unexpected approvals, the documented implementation provides no corresponding calldata decoder, state-difference analys ...[truncated 1596 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Decode the transaction's top-level calldata before approving execution, including known authorization methods such as: - ERC-20 `approve` - ERC-721 `approve` and `setApprovalForAll` - ERC-1155 `setApprovalForAll` - EIP-2612 permits - Permit2 permit and transfer authorization methods 2. Inspect the full simulation call trace for nested approvals, multicall operations, proxy calls, and `delegatecall` behavior rather than checking only the top-level method. 3. Compare pre-simulation and post-simulation authorization state, including: - Token allowance - Authorized spender - Approval amount - Operator status - Permit expiration - Nonce and signature scope 4. Compare every authorization change against explicit user intent. Require the expected token, spender, amount, authorization type, and expiration to be supplied as verification inputs. 5. Treat unlimited approvals, undeclared spenders, unexpected operator grants, and approvals broader than the requested transaction as verification failures. 6. Fail closed when the transaction cannot be decoded or when Tenderly does not return sufficient state-difference or call-trace information. Do not report that all checks passed when approval effects cannot be conclusively analyzed. 7. Present authorization changes separately from immediate asset transfers so the user can explicitly review any future spending authority being granted. ]]>