Summary

The unstake function in the Steaking contract allows users to unstake to any address, including the contract itself. When unstaking to the contract address, it creates an inconsistency between the totalAmountStaked variable and the actual ETH balance of the contract. This discrepancy can lead to accounting errors and vulnerabilities in dependent functions, potentially compromising the integrity of the staking system.

Vulnerability Details

The vulnerability exists in the unstake function in Steaking.vy:

When a user unstakes to the contract's address:

1. usersToStakes[msg.sender] and totalAmountStaked are reduced by _amount.

2. send(_to, _amount) transfers ETH to the contract itself.

3. The contract's ETH balance remains unchanged, while totalAmountStaked is reduced.

This creates a discrepancy between totalAmountStaked and the actual ETH balance of the contract.

Proof of Concept:

1. User A stakes 10 ETH.

2. totalAmountStaked is now 10 ETH.

3. User A unstakes 5 ETH to the contract's address.

4. totalAmountStaked is reduced to 5 ETH.

5. The contract's actual ETH balance remains 10 ETH.

Impact

1. Accounting Discrepancy: totalAmountStaked becomes inconsistent with the contract's actual ETH balance.

2. Potential for Exploitation: An attacker could repeatedly unstake to the contract address, artificially reducing totalAmountStaked without removing ETH from the contract.

3. Interference with Other Functions: Functions relying on totalAmountStaked (e.g., depositIntoVault) may behave incorrectly or be vulnerable to manipulation.

4. Misleading Contract State: The discrepancy could lead to incorrect reporting of the total staked amount, misleading users and external systems.

5. Potential for Further Attacks: This state inconsistency could be leveraged for more complex attacks on the staking system or connected protocols.

Risk Rating:

• Likelihood: Medium (requires intentional action but is easily executable)

• Impact: High (can lead to significant contract state manipulation)

• Overall: High

Tools Used

• Manual code review

Recommendations

1. Disallow Unstaking to Contract:
   Add a check to prevent unstaking to the contract's own address:

   assert _to != self, "Cannot unstake to contract itself"

2. Implement Pull Pattern:
   Replace the direct send with a two-step withdrawal process:

   @external

   def unstake(_amount: uint256):

   # ... existing checks ...

   self.usersToStakes[msg.sender] -= _amount

   self.totalAmountStaked -= _amount

   self.pendingWithdrawals[msg.sender] += _amount

   ​

   @external

   def withdraw():

   amount: uint256 = self.pendingWithdrawals[msg.sender]

   self.pendingWithdrawals[msg.sender] = 0

   send(msg.sender, amount)

By implementing these recommendations, particularly disallowing unstaking to the contract itself and adopting a pull pattern for withdrawals, the contract can maintain consistency between totalAmountStaked and its actual ETH balance, significantly improving its security and reliability.