Description
Users should be able to increase their total staked amount every time they stake a new amount, but the Steaking::usersToStakes state variable will reflect the last user staked amount and the accumulated.
Impact
User will expect to increase their total staked amount to have more ETH to deposit into the vault but at the end just will able to deposit the last staked amount because Steaking::Stake doesn't add up the value.
@external
@payable
def stake(_onBehalfOf: address):
"""
@notice Allows users to stake ETH for themselves or any other user within the staking period.
@param _onBehalfOf The address to stake on behalf of.
"""
assert not self._hasStakingPeriodEnded(), STEAK__STAKING_PERIOD_ENDED
assert msg.value >= MIN_STAKE_AMOUNT, STEAK__INSUFFICIENT_STAKE_AMOUNT
assert _onBehalfOf != ADDRESS_ZERO, STEAK__ADDRESS_ZERO
@> self.usersToStakes[_onBehalfOf] = msg.value
self.totalAmountStaked += msg.value
log Staked(msg.sender, msg.value, _onBehalfOf)
Proof of Concepts
Put next snippet code into Steaking.t.sol file.
This test proof that the final usersToStake amount is not the total amount staked by the user.
function testStakedAmountDoesNotAccumulative() public {
uint256 dealAmount = steaking.getMinimumStakingAmount();
vm.deal(attacker, dealAmount);
uint16 numberOfStakes = 3;
for (uint16 i = 0; i < numberOfStakes; i++) {
_stake(user1, dealAmount, user1);
}
assertEq(steaking.usersToStakes(user1), dealAmount);
}
Recommended mitigation
@external
@payable
def stake(_onBehalfOf: address):
"""
@notice Allows users to stake ETH for themselves or any other user within the staking period.
@param _onBehalfOf The address to stake on behalf of.
"""
assert not self._hasStakingPeriodEnded(), STEAK__STAKING_PERIOD_ENDED
assert msg.value >= MIN_STAKE_AMOUNT, STEAK__INSUFFICIENT_STAKE_AMOUNT
assert _onBehalfOf != ADDRESS_ZERO, STEAK__ADDRESS_ZERO
+ self.usersToStakes[_onBehalfOf] += msg.value
- self.usersToStakes[_onBehalfOf] = msg.value
self.totalAmountStaked += msg.value
log Staked(msg.sender, msg.value, _onBehalfOf)
