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 gogo’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:
/// @notice Complete the 2-step node transfer process. Can only be called by
/// by the new owner
// @audit never checks if the first step to transfer node ownership was actually started.
// Attack can set node ownership to 0
function completeNodeOwnerTransfer(uint64 id) external {
// @audit newOwner = 0 if first step was never started
uint64 newOwner = pendingNodeOwnerTransfers[id];
// @audit attacker can launch attack from address which is not registered
// in the AccountRegistry and hence accountId = 0
uint64 accountId = accounts.resolveId(msg.sender);
// @audit attacker can make this check pass as 0 == 0
if (newOwner != accountId) revert NotAuthorizedForNode();
// @audit attacker has now set node ownership to 0
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]
Unexpected Matching Inputs
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;
// @audit will never execute if t1==t2, returns (i, 0)
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:
- adding input validation to revert if t1 == t2
- adding a sanity check of the return values which reverts if i == 0 || j == 0
Unexpected Empty Inputs
Consider this simplified code from Akshay Srivastav‘s audit:
function verifyAndSend(SigData[] calldata signatures) external {
for (uint i; i<signatures.length; i++) {
// verify every signature, revert if one fails to verify
}
// @audit attacker can pass empty signatures array so loop
// never executes, allowing funds to be sent without
// verifying the signatures
(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 {
// @audit collateralToken is never validated, could be empty object corresponding
// to address(0) or a different address not linked to position's collateral
(uint256 price,) = priceFeeds[collateralToken].fetchPrice();
// @audit with empty/non-existent collateral, the value of the collateral will be 0
// with another address, the value will be whatever that value is, not the value
// of the Borrower's actual collateral. This allows Borrower to be Liquidated
// before they are in default, since the value of Borrower's actual collateral is
// never calculated.
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]
Unchecked Return Values
Consider this simplified example from Sherlock’s TellerV2 contest(lending/borrowing):
// AddressSet from https://github.com/OpenZeppelin/openzeppelin-contracts-upgradeable
// a loan must have at least one collateral
// & only one amount per token is permitted
struct CollateralInfo {
EnumerableSetUpgradeable.AddressSet collateralAddresses;
// token => amount
mapping(address => uint) collateralInfo;
}
// loanId -> validated collateral info
mapping(uint => CollateralInfo) internal _loanCollaterals;
function commitCollateral(uint loanId, address token, uint amount) external {
CollateralInfo storage collateral = _loanCollaterals[loanId];
// @audit doesn't check return value of AddressSet.add()
// returns false if not added because already exists in set
collateral.collateralAddresses.add(token);
// @audit after loan offer has been created & validated, borrower can call
// commitCollateral(loanId, token, 0) to overwrite collateral record
// with 0 amount for the same token. Any lender who accepts the loan offer
// won't be protected if the borrower defaults since there's no collateral
// to lose
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]
原文始发于Dacian：Exploiting Developer Assumptions
相关文章
