- Check for reentrancy risks
- Check whether the external call specifies the 1. the proper function and 2. the proper function arguments in the proper order for that function.
- Does the external address have this function, or any contract at all? A call to an address with no bytecode will not revert, and a contract with a fallback function will call the fallback function when an undeclared function is called (Dedaub Phantom Functions research).
- Check for frontrunning and sandwich attack of the swap. This can happen when interacting with any liquidity pool (Uniswap, Sushi, Curve, Balancer, etc.)
- If a one-sided swap is performed before depositing into the liquidity pool (often named a "zap"), there may be dust left over because the first one-sided swap alters the balance and price of the liquidity pool assets.
- If tokens are transferred from a contract address, that contract must approve the tokens before the transferFrom is called, otherwise it will revert
- Safe math in Solidity 0.8.X does not protect against integer casting overflows and bitshift overflows
- Check all
require
andrevert
logic in case of unintentional reverts causing the user loss of gas
- Standard solidity ecrecover function is used without checking if the result is the zero address. Must check for zero address or use OZ
recover
from ECDSA library.
- If any ERC20 token is supported by the protocol (meaning tokens are not whitelisted), check for weird ERC20 edge cases
- If an ERC20 token list is used it must consider double entry point tokens, with past issues of this type here and here
- The ERC721 tokenId value must be unique. If the tokenId value is not a simple incrementing counter and instead uses a formula to calculate the tokenId, duplicate values may occur and lead to a revert
- If safeMint is used, check for reentrancy on the callback hook
- If any access controls are used (often in the form of modifiers) check functions that do not have any modifier to see if the modifier may have been forgotten.
- If spot price of a liquidity pool is used... just no
- Chainlink oracle data may be stale. The roundId and timestamp should be checked.
- Any purchase or swap function should have slippage protection. This is normally a user-specified function argument.
- If a contract facilitates deposits into a DEX like Uniswap and tracks user positions with shares, the first deposit into this pair of the protocol could be frontrun
- Compound does not strictly follow the checks-effects-interactions pattern to avoid reentrancy. This can lead to reentrancy problems if tokens with callbacks (ERC721, ERC777, etc.) are used.
- Compound whitelists tokens and can avoid ERC20 tokens with unusual behavior. If a fork of compound does not whitelist tokens, issues can exist with fee-on-transfer tokens among others.
- UUPS proxies MUST be initialized after deployment. Forgetting to initialize the proxy led to the first $10 million bug from Immunefi and many other past bugs.
- State variable layout must be followed when delegatecall is used, otherwise this leads to problems
- Full list of common proxy bugs is at proxies.yacademy.dev
- Many OpenZeppelin upgradeable contracts need to be initialized in the importing contract's constructor.
- Does the snapshot process prevent a user from flashloaning tokens to make a large number of votes or receive a lot of yield?
- Does the snapshot process prevent a user from voting (or redeeming yield), transferring tokens, and voting again (or redeeming yield)?
- If the voting happens off-chain, is proper planning and integration with snapshot.org applied? Which voting strategy is used? Will voting delegation be added with the
setDelegate
function?
- Is there a way that a user can become undercollateralized, reducing the incentive to pay their debt?
- If cryptopunks are supported (which requires custom code), it should protect against this frontrun attack vector.