Unchecked CALL Return Values

There are a number of ways of performing external calls in Solidity. Sending ether to external accounts is commonly performed via the transfer method. However, the send function can also be used, and for more versatile external calls the CALL opcode can be directly employed in Solidity. The call and send functions return a Boolean indicating whether the call succeeded or failed. Thus, these functions have a simple caveat, in that the transaction that executes these functions will not revert if the external call (intialized by call or send) fails; rather, the functions will simply return false. A common error is that the developer expects a revert to occur if the external call fails, and does not check the return value.

For further reading, see #4 on the DASP Top 10 of 2018 and “Scanning Live Ethereum Contracts for the ‘Unchecked-Send’ Bug”.

The Vulnerability

Consider the following example:

contract Lotto {
    bool public payedOut = false;
    address public winner;
    uint public winAmount;
    // ... extra functionality here
    function sendToWinner() public {
        require(!payedOut);
        winner.send(winAmount);
        payedOut = true;
    }
    function withdrawLeftOver() public {
        require(payedOut);
        msg.sender.send(this.balance);
    }
}

This represents a Lotto-like contract, where a winner receives winAmount of ether, which typically leaves a little left over for anyone to withdraw.

The vulnerability exists on line 11, where a send is used without checking the response. In this trivial example, a winner whose transaction fails (either by running out of gas or by being a contract that intentionally throws in the fallback function) allows payedOut to be set to true regardless of whether ether was sent or not. In this case, anyone can withdraw the winner’s winnings via the withdrawLeftOver function.

Preventative Techniques

Whenever possible, use the transfer function rather than send, as transfer will revert if the external transaction reverts. If send is required, always check the return value.

A more robust recommendation is to adopt a withdrawal pattern. In this solution, each user must call an isolated withdraw function that handles the sending of ether out of the contract and deals with the consequences of failed send transactions. The idea is to logically isolate the external send functionality from the rest of the codebase, and place the burden of a potentially failed transaction on the end user calling the withdraw function.

Real-World Example: Etherpot and King of the Ether

Etherpot was a smart contract lottery, not too dissimilar to the example contract mentioned earlier. The downfall of this contract was primarily due to incorrect use of block hashes (only the last 256 block hashes are usable; see Aakil Fernandes’s post about how Etherpot failed to take account of this correctly). However, this contract also suffered from an unchecked call value. Consider the function cash in lotto.sol: Code snippet.

Example 9. lotto.sol: Code snippet

...
  function cash(uint roundIndex, uint subpotIndex){
        var subpotsCount = getSubpotsCount(roundIndex);
        if(subpotIndex>=subpotsCount)
            return;
        var decisionBlockNumber = getDecisionBlockNumber(roundIndex,subpotIndex);
        if(decisionBlockNumber>block.number)
            return;
        if(rounds[roundIndex].isCashed[subpotIndex])
            return;
        //Subpots can only be cashed once. This is to prevent double payouts
        var winner = calculateWinner(roundIndex,subpotIndex);
        var subpot = getSubpot(roundIndex);
        winner.send(subpot);
        rounds[roundIndex].isCashed[subpotIndex] = true;
        //Mark the round as cashed
}
...

Notice that on line 21 the send function’s return value is not checked, and the following line then sets a Boolean indicating that the winner has been sent their funds. This bug can allow a state where the winner does not receive their ether, but the state of the contract can indicate that the winner has already been paid.

A more serious version of this bug occurred in the King of the Ether. An excellent post-mortem of this contract has been written that details how an unchecked failed send could be used to attack the contract.