This repository contains some heuristics that can be used when auditing smart contracts to get ideas for vulnerabilities/common pitfalls. The goal of this repository is not to list concrete vulnerabilities (there are already a lot of great resources for that, e.g. prior audit reports or the SWC Registry), but rather general heuristics for pitfalls that I encounter often during audits and that can be applied to many projects.
Note: These are general heuristics/common pitfalls and cannot be blindly applied to a codebase to find vulnerabilities. You should always understand a codebase well to judge if something is truly a vulnerability in this particular context. However, these heuristics can help to get ideas for things to explore while you are auditing the code.
In many projects, there should be some symmetries for different functions. For instance, a withdraw function should (usually) undo all the state changes of a deposit function and a delete function should undo all the state changes of the corresponding add function. Asymmetries in these function pairs (e.g., forgetting to unset a field or to subtract from a value) can often lead to undesired behavior.
For some functions (e.g., a deposit function), the resulting state should be (almost) the same when the function is called X times with a value Y or when it is only called once with a value XY. While some small differences can be normal (e.g., because of rounding), large differences can indicate an error in the implementation. Note that this can also be a good fuzzing target, as it is very easy to split up a provided test case and compare the resulting state.
While some small rounding errors are generally unavoidable and not a bad thing, they can become a large problem when it is possible to amplify them. One way to amplify can be to call a function repeatedly at predetermined moments in time (see here for an example). The significance of the rounding error may also depend on the values (or rather their order of magnitude), so thinking about different scales can also help.
Off-by-one errors are very common (not only in smart contracts), so it always makes sense to think about the boundaries. Is <= correct in this context or should < be used? Should a variable be set to the length of a list or the length - 1? Should an iteration start at 1 or 0?
Some programmers have the implicit invariant that a list will not contain duplicates, but do not check so explicitly. This can be especially harmful when addresses are passed by a user (and they are used for some queries, e.g. to query balances), because some values may be double-counted. Thinking about what happens when there are duplicates (and if it is possible) can be very helpful
Ethereum Improvement Proposals and in general standards (like RFCs) often have very detailed instructions on how different functions/implementations should behave in different scenarios. Not following these is very bad for composability (after all, the reason for these standards is that you can rely on a certain behavior) and it can also result in vulnerabilities. In my experience, there are often slight details (e.g., not reverting when one should revert or not following the specified rounding behavior) that are ignored by implementers.
In a lot of smart contracts, there are functions where you have to specify a source (or sender) and a destination (or recipient). Sometimes, the programmer did not think about what happens when these (user-specified) parameters are equal, which can result in undesired behavior. For instance, the balance of the source and destination may be cached in the beginning, in which case you can inflate your balance by specifying yourself as the source and the destination.
It is very common to check if a variable is equal to the default value (0, address(0), empty list, empty string, ...) when checking if it is already initialized. However, these values may also be valid values for this variable. This can result in situations where it is possible to trigger the initialization logic multiple times, which can have negative side effects.
An implicit assumption that some developers make is that the contract balance (ERC20 or ETH balance) is equal to some state variable that is increased/decreased on deposits/withdrawals. This will not be true in practice (you can always manually transfer tokens or ETH to a contract, the latter even if it does not have a receive function), so it can be very dangerous to use this state variable and the balance interchangeably.
Some functions that should only be callable once with the same set of parameters (or slightly modified parameters) and fail afterward. A very common example is signatures that should only be used once (and where malleability can be an issue), but it is in general a good idea to think about if this function should be callable multiple times with the same parameters and if not, validate that there are measures against it.
For optimization purposes, contracts often work on a copy of some data (e.g., a struct that is declared as memory) to avoid accessing the global state repeatedly. Sometimes, they forget to update the global state after doing so and only update the copy. Although it is a simple mistake, it can be relatively hard to spot in a large codebase (as the only difference can be a memory instead of a storage keyword) and an extension that highlights storage variables (e.g., Solidity Visual Developer) can help a lot to catch such mistakes.
When there is an arbitrarily long loop (where the length is for instance controllable by the user) and there is no way to specify the start/end index, there is the risk that the list grows too large at some point and looping over it will no longer be possible because of the gas limit. Thinking about the maximum length (or in general if the list is bounded and if there is a possibility to specify a subset) can help to catch such issues.
To delete an item from a list in Solidity, you usually replace it with the last item in the list and reduce the length of the list by one (using pop). However, this actually changes the state of two items: The item you want to delete (which is usually handled correctly) and the item that was previously the last. This item now has a different list index. Because the index is often referenced in other places, this can lead to wrong references.
Many contracts have some special logic to handle ETH. For instance, they may wrap it to WETH when msg.value > 0, but may also accept WETH directly. One problem that can occur there is that a developer implicitly assumes that these cases are mutually exclusive, but does not explicitly check so. When the user then specifies WETH as currency but also sends ETH with the call, some invariants may be broken, which can lead to wrong behavior.