/CommonWeb3SecurityIssues

Common security findings in smart contracts

Apache License 2.0Apache-2.0

Common Web3 Security Issues

External calls

  • 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).

Swap is performed

  • 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.

transferFrom is used

  • If tokens are transferred from a contract address, that contract must approve the tokens before the transferFrom is called, otherwise it will revert

Overflows

  • Safe math in Solidity 0.8.X does not protect against integer casting overflows and bitshift overflows

Gas griefing / denial of service

  • Check all require and revert logic in case of unintentional reverts causing the user loss of gas

ecrecover

  • 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.

ERC20

  • 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

ERC721

  • 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

Access controls

  • 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.

External price data is used

  • If spot price of a liquidity pool is used... just no
  • Chainlink oracle data may be stale. The roundId and timestamp should be checked.

AMM/DEX

  • 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 forks

  • 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.

Proxies

OpenZeppelin Upgradeable imports

  • Many OpenZeppelin upgradeable contracts need to be initialized in the importing contract's constructor.

Voting and rewards/yield

  • 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?

Loan-related protocol

  • Is there a way that a user can become undercollateralized, reducing the incentive to pay their debt?

NFT projects