First Flight #22: Steaking

Vulnerability Details

The stake function in the contract is responsible for allowing users to stake ETH on behalf of themselves or others. However, the implementation of this function contains a critical flaw: the user's previous stake amount is overwritten by the new msg.value each time they stake. Specifically, the line:

self.usersToStakes[_onBehalfOf] = msg.value

This line of code replaces the existing staked amount with the new amount,
meaning that only the most recent stake is stored.
As a result, if a user stakes multiple times, only the latest stake amount will be reflected,
leading to an inaccurate total of their staked ETH.

The unstake function also calls the stakedAmount: uint256 = self.usersToStakes[msg.sender] And if a user stakes multiple times, its going to return the most recent stake and not the accumulated stakr amount

Impact

1. Incorrect Point Allocation: If the points are awarded based on the total ETH staked, the user may not receive the correct number of points if they stake multiple times, as earlier stakes are effectively discarded.

2. Misleading Staking Balance: Users might believe their staking balance is cumulative, but in reality, it's being overwritten with each new stake. This can lead to incorrect calculations in functions such as unstake where the balance is checked.

3. User Confusion and Potential Disputes: Users may become confused or dissatisfied if they expect their staking balance to reflect the total of all their stakes but see a lower amount. This could lead to disputes and a loss of trust in the protocol.

Proof of Concept

1. Initial Stake:

   • User stakes 1 ETH.

   • self.usersToStakes[_onBehalfOf] is set to 1 ETH.

2. Subsequent Stake:

   • User stakes another 0.5 ETH.

   • self.usersToStakes[_onBehalfOf] is now set to 0.5 ETH, effectively discarding the initial 1 ETH.

3. Unstake Attempt:

   • When the user tries to unstake, the contract assumes they only have 0.5 ETH staked, not the actual total of 1.5 ETH.

Tools Used

• Manual Review

Recommendations

The `stake` function should be modified to accumulate the staked amount rather than overwrite it. The corrected code should add the new msg.value to the existing staked amount like this:

`self.usersToStakes[_onBehalfOf] += msg.value`

This ensures that each new stake is added to the user's existing stake, accurately reflecting the total amount staked. Additionally, it is recommended to thoroughly test the contract to ensure that the staking balance is correctly tracked across multiple transactions.