First Flight #22: Steaking
First Flight #22
Beginner FriendlyFoundryDeFi
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

Overwritten `usersToStakes` Causes Loss of Stake

evmninja

Summary

The users' stake information stored in self.usersToStakes can be overwritten which causes the users to lose their stake.

Vulnerability Details

The function stake() is used to stake Ether for the users themselves or on behalf of other users by determining the _onBehalfOf address. The code in line 73 has a bug such that a user's existing stake is overwritten by the new msg.value.

self.usersToStakes[_onBehalfOf] = msg.value

This can be weaponised by an attacker to overwrite a user's existing stake with a much less value defined as MIN_STAKE_AMOUNT.

Impact

The user could lose their stake without a way to recover it.

Tools Used

Manual review and test

Recommendations

Change the code in line 73 with the following:

self.usersToStakes[_onBehalfOf] += msg.value

Proof of Concept

function test_evmn_stake_stake() public {
// Deal 10 eth to ALICE
deal(ALICE, 10 ether);

// Deposit
vm.startPrank(ALICE);
​
steaking.stake{value: 10 ether}(ALICE);
​
vm.stopPrank();
​
// validation
assertEq(steaking.totalAmountStaked(), 10 ether);
assertEq(steaking.usersToStakes(ALICE), 10 ether);
​
// Second stake
​
// Deal MIN_STAKE_AMOUNT to BOB
deal(BOB, MIN_STAKE_AMOUNT);
​
// Deposit
vm.startPrank(BOB);
​
steaking.stake{value: MIN_STAKE_AMOUNT}(ALICE);
​
vm.stopPrank();
​
// validation
assertEq(steaking.totalAmountStaked(), 10 ether + MIN_STAKE_AMOUNT);
// This should be 10 ether
assertEq(steaking.usersToStakes(ALICE), MIN_STAKE_AMOUNT);
​
​
}
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.