Input handling & state transitions in smart contracts are often overlooked; developers subconsciously make assumptions about the type of inputs their functions will receive and hence don't stringently handle inputs that don't conform to those assumptions.
Unexpected inputs & unchecked state transitions can result in storage corruption and invariant invalidation, leading to catastrophic consequences. Smart contract auditors should consciously ask themselves: What subconscious assumptions has the developer made, leading to unhandled input vulns? Let's look at some common "gotchas" that exploit smart contracts through erroneous developer assumptions.
Unchecked 2-Step Ownership Transfer
This exploit comes from GeorgeHNTR's audit of Metalabel. Here the 2nd step of the 2-step ownership transfer process never checks that the 1st step was started. If a node has an existing owner and no ownership transfer has been started, an attacker can directly invoke the 2nd step by calling NodeRegistry.completeNodeOwnerTransfer() to zero the owner, effectively bricking that node's ownership:
function completeNodeOwnerTransfer(uint64 id) external {
uint64 newOwner = pendingNodeOwnerTransfers[id];
uint64 accountId = accounts.resolveId(msg.sender);
if (newOwner != accountId) revert NotAuthorizedForNode();
nodes[id].owner = newOwner;
delete pendingNodeOwnerTransfers[id];
emit NodeOwnerSet(id, newOwner);
}
To prevent this attack, the 2nd step of the ownership transfer process must check that the 1st step has been initiated. Smart contract developers must carefully consider state transitions and implement checks within their contracts to ensure only valid state transitions are possible. More examples: [1]
For the next example we'll use a simplified version of an unexpected matching inputs vulnerability discovered during Cyfrin's Beanstalk Wells audit. Consider a vault contract that maintains a storage list "_tokens" that have been deposited into the vault, and allows swaps between them through a function, swap(). Assume swap() uses an internal function _getTokenIndexes() to get the indexes of the tokens from its storage map of deposited tokens:
function _getTokenIndexes(IERC20 t1, IERC20 t2) internal pure
returns (uint i, uint j) {
for (uint k; k < _tokens.length; ++k) {
if (t1 == _tokens[k]) i = k;
else if (t2 == _tokens[k]) j = k;
}
}
The subconscious assumption by the developer is that users will swap between different tokens, hence t1 != t2. By calling this function with t1 == t2, it will return (i, 0) since the "else if" statement will never execute. In this case Cyfrin was able to leverage this vulnerability to invalidate the contract's invariant and drain funds. The developer could prevent this by:
More examples: [1, 2, 3]
Consider this simplified code from Akshay Srivastav's audit:
function verifyAndSend(SigData[] calldata signatures) external {
for (uint i; i<signatures.length; i++) {
}
(bool sent, bytes memory data) = payable(msg.sender).call{value: 1 ether}("");
require(sent, "Failed to send Ether");
}
The developer has subconsciously assumed that verifyAndSend() will be passed an array of signatures, but an attacker can simply pass an empty array to bypass the loop & signature verification. Developers should be careful to first validate received inputs; in the case of arrays, validate that array.length > 0 before attempting to loop through it. Auditors should examine how control flow operates if they pass empty values to functions - do the functions continue to execute and can this be leveraged into an exploit?
Another example of this vulnerability comes from Antonio Viggiano's audit of Tempus Finance Raft protocol. Here an attacker can pass a different or zero value for collateral to liquidate a Borrower who should not be subject to liquidation:
function liquidate(IERC20 collateralToken, address position) external override {
(uint256 price,) = priceFeeds[collateralToken].fetchPrice();
uint256 entirePositionCollateral = raftCollateralTokens[collateralToken].token.balanceOf(position);
uint256 entirePositionDebt = raftDebtToken.balanceOf(position);
uint256 icr = MathUtils._computeCR(entirePositionCollateral, entirePositionDebt, price);
This is also an example of the Lending/Borrowing vulnerability class Liquidation Before Default. More examples: [1, 2, 3]
Unchecked Return Values
Consider this simplified example from Sherlock's TellerV2 contest (lending/borrowing):
struct CollateralInfo {
EnumerableSetUpgradeable.AddressSet collateralAddresses;
mapping(address => uint) collateralInfo;
}
mapping(uint => CollateralInfo) internal _loanCollaterals;
function commitCollateral(uint loanId, address token, uint amount) external {
CollateralInfo storage collateral = _loanCollaterals[loanId];
collateral.collateralAddresses.add(token);
collateral.collateralInfo[token] = amount;
}
The return value of AddressSet.add() is never checked; this function will return false if "token" already exists in the set "collateralAddresses". Because the return value is never checked, add() will silently fail when attempting to add the same token, allowing a borrower to overwrite their collateral amount from a large amount initially (to get a loan) to a 0 amount afterward so they can default on the loan if it is accepted and not lose any collateral!
Another common example of unchecked return value errors is when sending eth via call(), as seen in Code4rena's Tessera contest:
payable(msg.sender).call{value: contribution}("");
Here the return value of call() is never checked, so if the eth was sent to a smart contract whose receive() function reverted, this code would assume that the eth was successfully sent and continue executing with this assumption. Developers should always check the return values of function calls that can return false, and auditors should look out for code that doesn't check the return value of such functions. More examples: [1]