Description:
In Steaking::stake
it can be called by any user on behalf of anyone and the balance of the person on whose behalf the stake was made can be changed!
@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)
Impact:
This is a critical vulnerability that can lead to loss of funds of users as their entire balance can be reset to the MIN_STAKE_AMOUNT
that is expected by the contract.
Proof of Concept:
Add the following test to Steaking.t.sol
:
function testAnyoneCanResetStakingAmount() public {
address user2 = makeAddr("user2");
_stake(user1, 10 ether, user1);
console.log("amount staked by user1 before", steaking.usersToStakes(user1));
_stake(user2, 0.5 ether, user1);
console.log("amount staked by user1 after ", steaking.usersToStakes(user1));
}
To run the test:
forge test --mt testAnyoneCanResetStakingAmount -vvv
The output of the balance can clearly be seen in the log:
amount staked by user1 before 10000000000000000000
amount staked by user1 after 500000000000000000
So even though user1
staked 10 ETH on his behalf, user2
was able to change his balance to the MIN_STAKE_AMOUNT
i.e 0.5 ETH
Moreover, there is no sweep
function in the contract, so even the owner
can not help the user and the remaining 9.5 eth is locked in the contract forever!
Recommended Mitigation:
Change the stake
function to following:
@external
@payable
def stake(_onBehalfOf: address):
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)