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

Uncontrolled Deposits in Steaking.vy::depositIntoVault(): Risk of Contract Drainage Due to Unreset Staked Amount

afdholudin

Summary

The function Steaking.vy::depositIntoVault() doesn't reset the user's stake after depositing into the WETHSteak Vault. As a result, a user can deposit more than their staked amount and potentially drain all the funds in the contract.

Vulnerability Details

The function Steaking.vy::depositIntoVault() contains the following code:

@external
def depositIntoVault() -> uint256:
...
assert self._hasStakingPeriodEndedAndVaultAddressSet(), STEAK__STAKING_PERIOD_NOT_ENDED_OR_VAULT_ADDRESS_NOT_SET
stakedAmount: uint256 = self.usersToStakes[msg.sender]
assert stakedAmount > 0, STEAK__AMOUNT_ZERO
​
extcall IWETH(WETH).deposit(value=stakedAmount)
extcall IWETH(WETH).approve(self.vault, stakedAmount)
sharesReceived: uint256 = extcall IWETHSteakVault(self.vault).deposit(stakedAmount, msg.sender)
...

The function doesn't reset the user's staked amount. As a result, a user can deposit multiple times, potentially draining all the ETH from the contract.

PoC

Add the following code to the end of Steaking.t.sol in the test folder:

function testDepositIntoVaultMoreThanStakedAmount() public {
uint256 dealAmount = steaking.getMinimumStakingAmount();
​
// Alice stakes 0.5 ETH
address alice = makeAddr("alice");
_stake(alice, dealAmount, alice);
​
// Bob stakes 0.5 ETH
address bob = makeAddr("bob");
_startVaultDepositPhase(bob, dealAmount, bob);
​
vm.startPrank(bob);
// Bob call depositIntoVault() twice, 0.5 ether for each call
steaking.depositIntoVault();
steaking.depositIntoVault();
vm.stopPrank();
​
uint256 bobDeposit = 1 ether;
​
uint256 steakingBalance = address(steaking).balance;
uint256 expectedSteakingBalance = 0;
uint256 wethSteakVaultShares = wethSteakVault.balanceOf(bob);
​
// Check if Bob drained all ETH from the Steaking contract
assertEq(steakingBalance, expectedSteakingBalance);
// Check if Bob's deposit equals 1 ETH, 0.5 ETH more than his staked amount
assertEq(wethSteakVaultShares, bobDeposit);
}

Impact

A user can drain all the funds in the contract.

Tools Used

Recommendations

Reset the user's staked amount before calling the external function. Add the following code in Steaking.vy::depositIntoVault():

@external
def depositIntoVault() -> uint256:
....
assert self._hasStakingPeriodEndedAndVaultAddressSet(), STEAK__STAKING_PERIOD_NOT_ENDED_OR_VAULT_ADDRESS_NOT_SET
stakedAmount: uint256 = self.usersToStakes[msg.sender]
assert stakedAmount > 0, STEAK__AMOUNT_ZERO
​
// Reset staked amount to 0
+ self.usersToStakes[msg.sender] = 0;
​
extcall IWETH(WETH).deposit(value=stakedAmount)
extcall IWETH(WETH).approve(self.vault, stakedAmount)
sharesReceived: uint256 = extcall IWETHSteakVault(self.vault).deposit(stakedAmount, msg.sender)
....
Updates

Lead Judging Commences

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

`Steaking:depositIntoVault` fails to update the users balance allowing contract draining to repeat call

Support

FAQs

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