First Flight #22: Steaking

Root cause

The unstake function uses the send function to transfer ETH to the specified address, without properly handling the return value. This can lead to a reentrancy attack, where an attacker can repeatedly call the unstake function and drain the contract's ETH balance.

Impact

The reentrancy vulnerability can allow an attacker to steal all the ETH from the contract, causing significant financial losses for the contract's users.

Proof of Concept

1. An attacker calls the unstakefunction to withdraw a small amount of ETH.

2. Before the contract can update the user's staked amount, the attacker repeatedly calls the unstakefunction, causing the contract to send more ETH to the attacker's address.

3. The attacker can continue this process until the contract's ETH balance is depleted.

Tools Used

Manual code review, Visual Studio Code.

Recommendations

1. Implement the "Checks-Effects-Interactions" pattern in the unstakefunction:

   1. Perform all necessary checks (e.g. staking period ended, amount to be withdrawn is valid).

   2. Update the state variables(e.g. user's staked amount, total amount staked),

   3. Interact with external contracts or addresses (i.e. trasnfer the ETH to the specified address).

      Here's an example of the modified unstake

      @external

      def unstake(_amount: uint256, _to: address):

      """

      @notice Allows users to unstake their staked ETH before the staking period ends. Users

      can adjust their staking amounts to their liking.

      @param _amount The amount of staked ETH to withdraw.

      @param _to The address to send the withdrawn ETH to.

      """

      assert not self._hasStakingPeriodEnded(), STEAK__STAKING_PERIOD_ENDED

      assert _to != ADDRESS_ZERO, STEAK__ADDRESS_ZERO

      ​

      stakedAmount: uint256 = self.usersToStakes[msg.sender]

      assert stakedAmount > 0 and _amount > 0, STEAK__AMOUNT_ZERO

      assert _amount <= stakedAmount, STEAK__INSUFFICIENT_STAKE_AMOUNT

      ​

      self.usersToStakes[msg.sender] -= _amount

      self.totalAmountStaked -= _amount

      ​

      # Use a temporary variable to store the amount to be sent

      amountToSend: uint256 = _amount

      ​

      # Perform the transfer using the "low-level" call function

      # This allows us to handle the return value and ensure the transfer is successful

      success: bool = call(_to, amountToSend, 0, output_size=0)

      assert success, STEAK__TRANSFER_FAILED

      ​

      log Unstaked(msg.sender, _amount, _to)

2. Use the low-level call function to transfer the ETH, and handle the return value to ensure the transfer is successful.

3. Implement a mutex or a re-entrancy guard to prevent multiple concurrent calls to the unstake function.