Unchecked Send

Overview

• Severity: Medium
• Confidence: Medium
• Affected Versions: All

What is the Unchecked send vulnerability?

In Solidity, the builtin function send() can be used to transfer ether to another address. However, this function will not automatically revert on failure; rather, it will return false to indicate some problem with the send. If the developer does not check this return value, it is possible that

Further reading: Solidity Documentation: Members of Addresses

Technical example of vulnerable code

  // SPDX-License-Identifier: Unlicense
  pragma solidity ^0.8.0;

  contract EtherVault {
      address public owner;
      mapping(address => uint256) public balances;

      event Deposit(address indexed sender, uint256 amount);
      event Withdrawal(address indexed receiver, uint256 amount);

      constructor() {
          owner = msg.sender;
      }

      // Allow users to deposit Ether into the vault
      function deposit() external payable {
          require(msg.value > 0, "Deposit amount must be greater than 0");
          balances[msg.sender] += msg.value;
          emit Deposit(msg.sender, msg.value);
      }

      // Attempt to withdraw Ether from the vault
      function withdraw(uint256 amount) public {
          require(amount <= balances[msg.sender], "Insufficient balance");

          // Unsafely update the balance before the transfer is guaranteed to succeed
          balances[msg.sender] -= amount;

          // send() is used without checking its return value, potentially leading to silent failures
          payable(msg.sender).send(amount);

          emit Withdrawal(msg.sender, amount);
      }

      // Function to check the contract's Ether balance
      function getContractBalance() public view returns (uint256) {
          return address(this).balance;
      }
  }

In the example above, contract EtherVault allows users to deposit and withdraw ether, tracking their balances in the balances mapping. The withdraw() function uses send() to transfer ether to a user when they withdraw it, updating their balance accordingly; however, it does not check the return value of this function, meaning the send may fail while still decreasing the user's accounting balance.

Technical example of how to fix the vulnerability

  // SPDX-License-Identifier: Unlicense
  pragma solidity ^0.8.0;

  contract EtherVaultUpdated {
      address public owner;
      mapping(address => uint256) public balances;

      event Deposit(address indexed sender, uint256 amount);
      event Withdrawal(address indexed receiver, uint256 amount);

      constructor() {
          owner = msg.sender;
      }

      // Allow users to deposit Ether into the vault
      function deposit() external payable {
          require(msg.value > 0, "Deposit amount must be greater than 0");
          balances[msg.sender] += msg.value;
          emit Deposit(msg.sender, msg.value);
      }

      // Attempt to withdraw Ether from the vault
      function withdraw(uint256 amount) public {
          require(amount <= balances[msg.sender], "Insufficient balance");

          // Require that the send succeeds before proceeding
          bool sent = payable(msg.sender).send(amount);
          require(sent, "Failed to send Ether");

          // Only update the balances after the send succeeds
          balances[msg.sender] -= amount;

          emit Withdrawal(msg.sender, amount);
      }

      // Function to check the contract's Ether balance
      function getContractBalance() public view returns (uint256) {
          return address(this).balance;
      }
  }

In the revised example above, contract EtherVaultUpdated now uses a require() check on the result of the send() call within the withdraw() function to ensure that the ether was in fact transferred, and only after this does it update the balances mapping accordingly.