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

One user stake many times during the staking period, only record the last staked number instead of the user's total staked amount

Summary

if one user stake many times during the staking period, only record the last staked number instead of the user's total staked amount. which leads to the user's eth stuck in the protocol

Vulnerability Details

When user stake should record the total stake amount instead of the last stake amount.

@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
// There should record the user's totol stake amount instead of the last stake amount
self.usersToStakes[_onBehalfOf] = msg.value
self.totalAmountStaked += msg.value
log Staked(msg.sender, msg.value, _onBehalfOf)

POC Test

function test_POC_Stake() public {
// First Stake
uint256 dealAmount = steaking.getMinimumStakingAmount();
_stake(user1, dealAmount, user1);
uint256 userStakedAmount = steaking.usersToStakes(user1);
uint256 totalAmountStaked = steaking.totalAmountStaked();
console.log("userStakedAmount: %s", userStakedAmount);
console.log("totalAmountStaked: %s", totalAmountStaked);
// Second Stake, user's stake should be updated
_stake(user1, dealAmount, user1);
userStakedAmount = steaking.usersToStakes(user1);
totalAmountStaked = steaking.totalAmountStaked();
console.log("Second stake userStakedAmount: %s", userStakedAmount);
console.log("Second stake totalAmountStaked: %s", totalAmountStaked);
}

Impact

The user who staked many times during the staking period, all the previous staked amount of eth will stuck in the protocol.

Meanwhile will occur the inconsistant between user's points rewards and the user's staked record beacuse triggering the reward points based on the staked event.

Tools Used

Manual

Recommendations

Should record all the staked amount when user stake each time.

@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
// There should record the user's totol stake amount instead of the last stake amount
//self.usersToStakes[_onBehalfOf] = msg.value
self.usersToStakes[_onBehalfOf] += msg.value
self.totalAmountStaked += msg.value
log Staked(msg.sender, msg.value, _onBehalfOf)
Updates

Lead Judging Commences

inallhonesty Lead Judge 10 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Steaking::stake overwrites the msg.value into storage

Support

FAQs

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