Summary

Reference: https://github.com/Cyfrin/2024-08-steaking/blob/87e81eb617f3bca3dfbd7c6d4c90bdf1c988ebb0/steaking-contracts/src/Steaking.vy#L73

The stake() function in the Steaking contract incorrectly manages user stakes by overwriting the usersToStakes[_onBehalfOf] value instead of incrementing it. This results in each new stake replacing the user's total staked amount, effectively erasing previous contributions.

Vulnerability Details

In the Steaking.vy::stake() function, the following line incorrectly assigns the new stake amount:

This assignment replaces any existing stake value, rather than adding to it. Consequently, each new stake transaction overwrites the user's total staked amount, discarding any prior stakes.

Impact

This flaw in the staking mechanism leads to several serious issues:

Users staking multiple times will only be able to unstake their most recent contribution, not their total staked amount.
Earlier stakes become inaccessible, effectively locking up those funds in the contract.
When converting stakes to WETH for vault deposits, users can only convert their most recent stake, not their true total staked amount.
This discrepancy between actual ETH staked and WETH deposited in the vault results in permanent loss of user funds.

To demonstrate this issue, add the following test to steaking-contracts/test/Steaking.t.sol:

Tools Used

Manual Code Review

Recommendations

Modify the stake() function to accumulate user stakes rather than overwriting them. Replace the problematic line with:

This change ensures that each new stake is added to the user's existing stake, correctly tracking the total staked amount per user