T09 · Insecure Skill Coding Practices
- Location
references/recipes.md:42- Finding
Approval Transactions Bypass Mandatory Dry-Run and Confirmation Gates
- Content
View full analysis
Vulnerability Details
File Location:
references/recipes.md:42-68
Vulnerability Type: Missing transaction safety controls
Risk Level: MediumThe approval recipes directly broadcast three security-sensitive transactions:
bash Approve GHST (broadcast; do this only when explicitly instructed): ```bash ~/.foundry/bin/cast send "$GHST" 'approve(address,uint256)' "$DIAMOND" "<AMOUNT_GHST_WEI>" \ --private-key "$PRIVATE_KEY" \ --rpc-url "$BASE_MAINNET_RPC"Approve USDC (broadcast; do this only when explicitly instructed):
bash ~/.foundry/bin/cast send "$USDC" 'approve(address,uint256)' "$DIAMOND" "<AMOUNT_USDC_6DP>" \ --private-key "$PRIVATE_KEY" \ --rpc-url "$BASE_MAINNET_RPC"Set approval (broadcast; do this only when explicitly instructed):
bash ~/.foundry/bin/cast send "<NFT_CONTRACT_ADDRESS>" 'setApprovalForAll(address,bool)' "$DIAMOND" true \ --private-key "$PRIVATE_KEY" \ --rpc-url "$BASE_MAINNET_RPC"text This conflicts with the mandatory policy in `SKILL.md:34-38`, which states that every `cast send` must first be simulated and must require both `DRY_RUN=0` and `BROADCAST_CONFIRM=CONFIRM_SEND`. ### Technical Analysis The approval commands call `cast send` directly. They do not: - Simulate the exact transaction with `cast call`. - Enforce the default dry-run setting. - Verify `BROADCAST_CONFIRM=CONFIRM_SEND`. - Ensure the transaction arguments still match what the user confirmed. - Unset the confirmation token after broadcast. - Locally enforce chain-ID, signer-address, or canonical contract-address validation. The surrounding phrase “only when explicitly instructed” is advisory text rather than an executable control. An agent that selects an allowlisted recipe can therefore broadcast it despite `DRY_RUN` retaining its safe default or no confirmation token being present. ERC20 approvals authoriz ...[truncated 1930 chars]- Remediation
View remediation
Remediation Suggestions
-
Add an exact
cast callsimulation before every approval broadcast. -
Display a transaction summary containing the chain ID, signer, token or NFT contract, spender/operator, amount, RPC endpoint, and whether blanket NFT approval is being granted.
-
Add fail-closed checks immediately before each
cast send:bash test "${DRY_RUN:-1}" = "0" || { echo "Refusing broadcast: DRY_RUN must be 0" exit 1 } test "${BROADCAST_CONFIRM:-}" = "CONFIRM_SEND" || { echo "Refusing broadcast: explicit confirmation is required" exit 1 } -
Revalidate that the Base chain ID is
8453and that the private key derives the expectedFROM_ADDRESS. -
Validate every address as exactly
0xfollowed by 40 hexadecimal characters, and verify critical addresses against canonical allowlisted values. -
Bind confirmation to the complete transaction arguments. Invalidate confirmation if the asset, spender, amount, operator flag, sender, chain, or RPC endpoint changes.
-
Unset
BROADCAST_CONFIRMimmediately after the send attempt. -
Prefer exact, minimal ERC20 allowances rather than unlimited approvals.
-
Warn prominently that
setApprovalForAllgrants collection-wide authority, and offer revocation throughsetApprovalForAll(operator, false). -
Consolidate all broadcasts behind one guarded wrapper so recipes cannot omit mandatory controls.
-