Published on

The Ultimate Security Checklist

Authors

external / public Functions

[F-01] - Should it be external/public?

[F-02] - Does this function need to be restricted? (onlyOwner?)

[F-03] - Are the inputs checked?

[F-04] - Are there any front-running opportunities?

  • Beware of sandwich attacks on Vaults and DEXes.
  • Transactions must not be order-dependent.
  • Auctions with a fixed end time have the known vulnerability of being bid on at the last block.
  • Pausing mechanisms can be front-run.

[F-05] - Is this function making a call() or transferring ERC20 tokens?

  • Is the call address whitelisted?
  • Reentrancy possibility?

[F-06] - Is that function payable or transferring funds?

  • Check if msg.value == amount.

[F-07] - Are the code comments coherent with the implementation?

[F-08] - Can edge case inputs (0, max) result in unexpected behavior?

[F-09] - Requirement checks for external call parameters can be too strict and not allow all valid possible inputs.

[F-10] - When using arrays of addresses/IDs in calldata: what happens if there are duplicate addresses in the array?

External Call

[E-01] - Is an external contract call actually needed?

[E-02] - Is the address called whitelisted?

[E-03] - Be cautious when there is a fixed gas amount in a .call() and check if the gas left is enough.

[E-04] - Grief attack is possible when calling an unknown address by passing a huge amount of data into the call ⇒ use inline assembly.

[E-05] - A call to an address that does not exist returns true: is the existence of the address checked?

[E-06] - Use the check-effect-interaction pattern. Only if necessary, use a reentrancyGuard but beware of cross-contract reentrancy.

[E-07] - For sending ETH, don't use transfer() or send() and instead use call().

[E-08] - Unchecked msg.value can result in unexpected behavior.

[E-09] - Delegate calls that do not interact with stateless types of contracts (libraries) should be triple-checked.

[E-10] - Never delegate call to an untrusted contract.

[E-11] - If the recipient of ETH has a fallback function that reverts, could it cause DoS?

[E-12] - Could it cause an out-of-gas in the calling contract if it returns a massive amount of data? Can the external call be manipulated to cause DoS?

[E-13] - Would it be harmful if the call reentered into the current function?

[E-14] - Would it be harmful if the call reentered into another function?

[E-15] - What if it uses all the gas provided?

[E-16] - Could it cause an out-of-gas in the calling contract if it returns a massive amount of data?

[E-17] - Be careful when using msg.value in a multi-call.

Mathematics

[M-01] - Is the calculation correct?

[M-02] - Are the fees correctly calculated?

[M-03] - Is there precision loss? (especially for year/month/day calculation?)

[M-04] - Regular expressions like 1 day are uint24 meaning that operations with these expressions will be cast to uint24 and potentially overflow.

[M-05] - Always multiply before dividing.

[M-06] - Is a library used to round results?

[M-07] - Division by 0?

[M-08] - Even in Solidity >0.8.0, take care that a variable cannot underflow or overflow, which would cause a revert.

[M-09] - Assigning a negative value to a uint reverts.

[M-10] - unchecked{} blocks need to be validated.

[M-11] - When using < or >, check if it should be ≤ or ≥ instead.

[M-12] - Inline assembly math considerations:

  • div(x, 0) == 0.
  • Operations can overflow/underflow. You should add checks if necessary.

For/While Loops

[L-01] - Is the first iteration a problem?

[L-02] - DoS concerns:

  • Is there a call inside the loop?
  • Is the number of iterations limited? ⇒ can an attacker add elements at no cost?

[L-03] - Don't use msg.value in a loop.

Access Control

[A-01] - Centralization risks:

  • Can executors perform token transfers on behalf of users?
  • Is reclaiming/withdrawing any tokens possible?
  • Is there total upgradeability?
  • Are instant parameter changes possible (no timelock)?
  • Can the contract be paused freely?
  • Is it possible to rug users and steal assets?
  • Bugs in restricted functions that allow stealing assets are a centralization risk.

[A-02] - Can a corrupted owner destroy the protocol?

[A-03] - Are any features lacking access controls?

[A-04] - Do some addresses need a whitelist?

[A-05] - Is the owner change a two-step process?

[A-06] - Are critical functions accessible? (like mint())

Vault

[V-01] - Can transferring ERC20 or ETH directly break something?

[V-02] - Is the vault balance tracked internally?

[V-03] - Can the first deposit raise a problem?

[V-04] - Is this vault taking into consideration that some ERC20 tokens are not 18 decimals?

[V-05] - Is the fee calculation correct?

[V-06] - What if only 1 wei remains in the pool?

[V-07] - On vaults with strategies implemented:

  • Are flash deposit-harvest-withdraw attacks possible?
  • How does the vault behave when locked funds are put in a strategy?
  • Are losses handled? (they always should be)
  • What happens in case of a black swan event? (protocol implemented in the strategy gets hacked)
  • Look at token-specific/protocol risks implemented in strategies:
    • For protocols:
      • Pause functionality?
      • Emergency withdrawal?
      • Deprecation handling?
    • For tokens:
      • All weird implementations.

[V-08] - Can we manipulate the conversion rate between shares and underlying? (here)

ERC20 (More edge cases here: weird-erc20)

[FT-01] - Are safe functions being used?

[FT-02] - Is the USDT approve race condition a problem? (especially on DEXes?)

[FT-03] - Is the decimal difference between ERC20 tokens a problem?

[FT-04] - Does the contract implement a whitelist/blacklist or some kind of address check?

[FT-05] - Multiple-address tokens can be a problem.

[FT-06] - Are fees-on-transfer raising an issue?

[FT-07] - When there is a balanceOf(address(this)), check if manually sending tokens can break something.

[FT-08] - ERC777 tokens can hook bad things on transfers (before and after transfers).

[FT-09] - Solmate ERC20.safeTransferLib does not check for contract existence.

[FT-10] - IERC20(address(0)).decimals() will revert and cause DoS.

[FT-11] - Flash minting increases the token supply.

[FT-12] - Some tokens revert on 0 transfers: can cause DoS.

[FT-13] - Is a contract a target for token approvals? Do not make arbitrary calls from user input.

[FT-14] - Is the DOMAIN_SEPARATOR() function in ERC2612 missing?

[FT-15] - Some ERC20 tokens revert when sending tokens to certain addresses (LUSD).

ERC721

[NFT-01] - Safe functions must be used.

[NFT-02] - SafeMint() and SafeTransfers() from the ERC721 OpenZeppelin contract have callbacks that can reenter.

[NFT-03] - OpenZeppelin implementation of ERC721 and ERC1155 are vulnerable to reentrancy attacks, since safeTransferFrom functions perform an external call to the user address (onReceived).

[NFT-04] - Most of the time, the 'from' parameter of nft.transferFrom() should be msg.sender. Otherwise, hackers can take advantage of other users' approvals and rob them.

Proxies

[P-01] - Is there a constructor? (should not have one)

[P-02] - Is the modifier initializer added to the "initialization()" function? The deployer must always call the initialization function (check deployment scripts if any).

[P-03] - If any contract inheritance has a constructor (erc20, reentrancyGuard, Pausable, etc.), use the upgradeable version for initialize.

[P-04] - Check that authorizeUpgrade() is properly secured if using UUPS.

[P-05] - Can the new implementation cause storage collision with the old one? If children inherit from parents ⇒ set a gap.

[P-06] - Was disableInitializers() called in the constructor?

[P-07] - selfdestruct and delegatecall should not be used inside implementation contracts.

[P-08] - The values in immutable variables are not preserved between upgrades.

[P-09] - The order of storage variables declared or their type cannot be changed between upgrades.

[P-10] - Rugs:

  • Beware of function clashing (here).
  • Beware of metamorphic Contract Rug Vulnerability (here).

Signatures (ref)

[S-01] - Are signatures protected against replay with a nonce and block.chainid? Ensure all signatures use EIP-712.

[S-02] - Signature Malleability: do not use ecrecover() but use openzeppelin/ECDSA.sol (The latest version should be used here).

[S-03] - Check if the returned public key matches the expected public key.

[S-04] - Check if the signature is used from the right person (if not everyone should be able to use it).

[S-05] - Check if the deadline is not expired (if it is not needed for the signature to work forever).

Locktime for Staking

[LS-01] - Check if a user can help other users reduce the time lock by staking their tokens for them.

[LS-02] - Check if a contract can wrap the collateral token to sell 100% liquid already staked tokens.

[LS-03] - Can rewards be delayed in payout or claimed too early?

[LS-04] - Can deposited assets get stuck in the protocol (partially or fully) or be improperly delayed in withdrawal?

[LS-05] - If the payout is in a different asset or currency, can its value be manipulated within the scope of the smart contract? This is relevant if the protocol mints its own tokens to reward liquidity providers or stakers.

Account Abstraction (ref)

[AA-01] - DoS is possible when using paymaster because transactions are free.

AMMs

[AMM-01] - Is there slippage protection?

[AMM-02] - Does it work for every token decimal and type of token?

[AMM-03] - The AMM should either support Fee-on-Transfer (FoT) tokens or check if FoT (attack example).

[AMM-04] - Rebasing tokens can break the AMM: build a blacklist.

Lending (ref)

[LEN-01] - Can the position be liquidated if the loan is not paid back or the collateral drops below the threshold?

[LEN-02] - Can a user profit from self-liquidation?

[LEN-03] - If a token/transfer/add collateral is paused/bricked momentarily, can the user get liquidated even if they want to add more money?

[LEN-04] - Will liquidation work in the event of a large price drop?

[LEN-05] - Can the liquidator receive less than expected?

[LEN-06] - Are liquidations suspended? What happens when unpaused? (pause = solvency risk)

[LEN-07] - If a token/transfer/add collateral is paused/bricked momentarily, can the user get liquidated even if they want to add more money?

[LEN-08] - Is griefing possible by front-running through slightly increasing collateral?

[LEN-09] - Are all positions properly incentivized for liquidation, even the small ones?

Multichains (ref)

[MC-01] - Block time is not the same on different chains: Look for hardcoded time values dependent on the block.number that may only be valid on Mainnet.

[MC-02] - Block production may not be constant.

[MC-03] - PUSH0 is not supported on all chains: Solidity >=0.8.20 should be avoided on multichain apps (here).

[MC-04] - Verify that the EVM opcodes and operations used by the protocol are compatible on all chains: Arbitrum, Optimism.

[MC-05] - Verify that the expected behavior of tx.origin and msg.sender holds on all deployed chains (here).

[MC-06] - Analyze attack vectors that require low gas fees or where a considerable number of transactions have to be executed (here).

[MC-07] - ERC20 decimals can change across chains (here).

[MC-08] - Check the upgradeability of contracts on different chains and evaluate their implications: e.g., USDT is upgradeable on Polygon but not on Ethereum.

[MC-09] - Look for cross-chain message implementations and verify the correct permissions and functionality considering all actors involved.

[MC-10] - When a protocol is cross-chain, make sure all compatible chains are whitelisted: being able to send a message from an unsupported chain can lead to unexpected results.

[MC-11] - Check the compatibility of contracts when being deployed to zkSync Era (here).

[MC-12] - Use evm-diff.com and check here for chain comparisons.

Merkle Trees

[MT-01] - Merkle trees are front-runnable.

[MT-02] - Are leaves hashed with the claimable address inside?

[MT-03] - For airdrops: Does the claim() function rely on msg.sender to validate the mint?

[MT-04] - What happens if we pass the zero hash?

[MT-05] - What happens if the exact same proof exists twice in the tree?

General Tips

[G-01] - For logic implemented several times, check for differences and then standardize the logic.

[G-02] - Timelocks should be implemented for every protocol upgrade/change (to let users exit if they don't agree).

[G-03] - Force-feeding a Smart Contract. Beware of misuse of .balanceOf(this):

  • Self-destruct.
  • Deterministic addresses can be force-fed before deployment.
  • Coinbase Transactions: The attacker can start proof-of-work mining and set the target address to receive the reward.

[G-04] - Beware of DoS:

  • Never call() inside a for loop ⇒ use pull payments.
  • Sensitive withdrawal logic should be done externally by the user.
  • Beware of Block Stuffing attacks when the contract needs to perform an action within a certain time period.
  • If there is a timelock, an attacker might be able to brick the logic at no cost.

[G-05] - Beware of Oracle manipulation: Don't use spot price from an AMM as an oracle.

[G-06] - When deleting a structure, you should delete mappings inside first.

[G-07] - Be careful of relying on the raw token balance of a contract to determine earnings.

[G-08] - Take care with if (receiver == caller) as it can have unexpected behavior.

[G-09] - With pause mechanisms:

  • Check if it can pause something that should not be pausable (e.g., liquidation).
  • Check if it can brick the contract.
  • Check if whenNotPaused is properly implemented on every function where it needs to be.

[G-10] - When using selfdestruct, beware of CREATE2 reinitialization tricks (here).

[G-11] - Semantic Overload needs to be avoided or checked intensively.

[G-12] - Check the documentation and the code for inconsistencies.

[G-13] - If available, deployment scripts should be checked.

[G-14] - It's possible to bypass contract size checks by implementing the logic in the constructor.

[G-15] - Hash collisions are possible with abi.encodePacked when using >2 dynamic types (here).

[G-16] - Check if there are problematic bugs in the Solidity version used (here).

[G-17] - NoReentrancy modifier should be placed before every other modifier.

[G-18] - try/catch blocks can always fail by not supplying enough gas (here).

[G-19] - Reorgs can create a loss of funds (use CREATE2 instead of CREATE) (here).

[G-20] - Beware of cross-contract reentrancy (here).

[G-21] - Beware of read-only contract reentrancy.

[G-22] - When there is a multi-agent system: what if all agents are the same person? (Ex: self-liquidation).

[G-23] - DoS with (unexpected) revert (here).

[G-24] - msg.value in multicalls can hide a vulnerability.

[G-25] - Check for correct inheritance; keep it simple and linear.

[G-26] - Check for code asymmetries (withdraw function should usually undo all the state changes of a deposit function ref).

[G-27] - When an EIP is implemented, verify that all EIP recommendations are being followed.

[G-28] - Is block.timestamp only used for long intervals?

[G-29] - Are comparison operators used correctly (>, <, >=, <=)?

[G-30] - Are logical operators used correctly (==, !=, &&, ||, !)?

[G-31] - Watch for unexpected addresses (providing a receiver address pointing to another contract in the system).

DeFi Integrations

Gnosis Safe Integration

[GS-01] - Make sure your modules execute Guard's hooks (checkTransaction(), checkAfterExecution()).

[GS-02] - execTransactionFromModule() does not increment the Safe nonce: if modules are used, don't rely on it for signatures.

LSD Integration (ref)

stETH

[LSD-01] - stETH is a rebasing token: using wstETH is simpler for DeFi integration.

[LSD-02] - Be careful if your app converts tokens from stETH to wstETH: the rebasing has to be handled.

[LSD-03] - Withdrawing stETH/wstETH brings overhead (queue time, receives an NFT, withdrawal amount limits).

rETH

[LSD-04] - The burn() function can revert if there is not enough ether in the RocketDepositPool contract.

[LSD-05] - The rate rETH/ETH can decrease in case of a slashing event.

[LSD-06] - Remain vigilant about the risk of consensus attacks on RPL nodes, where nodes may submit incorrect exchange rate data.

cbETH

[LSD-07] - Blacklisting feature exists on transfers, approvals, mints, and burns.

[LSD-08] - The cbETH/ETH rate can be changed by a few addresses thanks to the onlyOracle modifier.

[LSD-09] - The cbETH/ETH rate can decrease.

sfrxETH

[LSD-10] - sfrxETH may detach from frxETH during reward transfers by the Frax team's multi-sig contract.

[LSD-11] - Currently, the sfrxETH/ETH rate can't decrease, but this may change in the future.

LayerZero Integration (Official LZ integration checklist here)

[LZ-01] - What is used? Blocking or non-blocking transactions? Blocking can result in DoS (here).

[LZ-02] - Gas should be estimated correctly, otherwise the cross-chain message will fail.

[LZ-03] - The _debitFrom function in ONFT must verify whether the specified owner is the owner of the tokenId passed in the parameters and whether the sender is allowed to transfer this token (ref).

[LZ-04] - If LzApp is inherited, remember to use the _lzSend function instead of directly calling the lzEndpoint.send function.

[LZ-05] - The User Application should implement the ILayerZeroUserApplicationConfig interface, including the forceResumeReceive function which, in the worst case, can allow the owner/multisig to unblock the queue of messages if something unexpected happens.

[LZ-06] - If you don't want to outsource your security, it is recommended to configure the applications to not use the default configuration. The default contracts are upgradeable and can be changed by the LayerZero team.

[LZ-07] - Choose the correct confirmation number depending on the chain and past reorgs.

VRF (VRF Security Considerations)

[CL-01] - When Chainlink VRF is called, make sure all parameters passed are verified; otherwise, fulfillRandomWord will not revert and may return a bad value.

[CL-02] - Chainlink VRF requests remain pending if there is not enough LINK in the subscription. This means that when the subscription becomes active again, the transaction can be front-run.

[CL-03] - Choose a high enough request confirmation number for chain reorgs (here).

[CL-04] - VRF calls are front-runnable: the betting phase must be closed before the VRF call.

Pricefeed

[CL-05] - Pricefeeds may not be supported in the future. To prevent wrong prices from being used, check the last update timestamp: require(block.timestamp - supplyUpdatedAt <= MAX_DELAY). Check the heartbeat of each price feed to adjust the MAX_DELAY variable.

[CL-06] - Rollup sequencers can go offline, resulting in outdated responses (here).

[CL-07] - Check that the price feed for the desired pair is supported on all deployed chains.

[CL-08] - Is the pricefeed heartbeat adapted for the use case? Is it too slow?

[CL-09] - Are different price feeds used? Does the code handle different decimal precisions?

[CL-10] - Is the pricefeed address hardcoded?

  • Is this address correct?
  • Be aware that this pricefeed may become deprecated in the future (especially for uncommon pricefeeds).

[CL-11] - Oracle price updates can be front-run (Angle's solution).

[CL-12] - Are oracle revert DoS scenarios handled? (wrap calls to Oracles in try/catch blocks and provide an alternative solution).

[CL-13] - Are ETH pricefeeds used for stETH? Or BTC pricefeeds used for WBTC? The depeg risk must be addressed.

[CL-14] - Oracles may return incorrect prices during flash crashes: To mitigate such attacks on-chain, smart contracts should check that minAnswer < receivedAnswer < maxAnswer.

Uniswap Integration

[U-01] - Hardcoded slippage is forbidden.

[U-02] - Proper slippage strategy should be implemented.

  • No expiration deadline?
  • Incorrect slippage calculation?
  • Mismatched slippage precision?
  • Is slippage calculated on-chain? On-chain slippage calculation can be manipulated?
  • Never hardcode slippage.

[U-03] - Are there refunds after swaps?

[U-04] - AMM pools' token0 and token1 order differs depending on the chain.

[U-05] - Are pools called whitelisted? If not, check if the pool has the right factory address.

[U-06] - Don't rely on pool reserves since they can be manipulated.

[U-07] - Don't rely on pool.swap(): Always use the Router contract.

AAVE/Compound Integration (help to remember concepts)

[AC-01] - What happens if the utilization rate is too high and collateral can't be retrieved?

[AC-02] - What happens if the protocol is paused?

[AC-03] - What happens if the pool becomes deprecated?

[AC-04] - What happens if assets you lend/borrow are within the same eMode category?

[AC-05] - Flashloans on Aave inflate the pool index (a maximum of 180 flashloans can be performed within a block).

[AC-06] - Does the protocol properly implement AAVE/COMP reward claims?

[AC-07] - The cETH token contract has no underlying() function.

[AC-08] - Borrowing an AAVE siloed asset prohibits you from borrowing any other asset. Use getSiloedBorrowing(address asset) to check.

[AC-09] - On AAVE, what happens if you reach the maximum debt on an isolated asset? (DoS potential?).