Beginner FriendlyFoundryDeFi
100 EXP
View results
Submission Details
Severity: high
Invalid

Reentrancy Vulnerability in the `unstake` Function

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.

Updates

Lead Judging Commences

inallhonesty Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Lack of quality

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.