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

Depositing in the vault not updating internal accounting, putting funds at risk of draining

Summary

The depositIntoVault function performs a high-risk operation and directly sends ETH to be converted to WETH which it then deposits into the vault:

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 then proceeds to emit an event and finish execution without updating the internal staking accounting.

Vulnerability Details

There’s a risk that a user could drain the contract if the usersToStakes and totalAmountStaked are not updated accordingly. A malicious user can repeatedly call depositIntoVault function until it deposits all the funds from Steaking (effectively draining the pool) to the vault.

Impact

The impact is high, enabling a malicious user to drain all funds from the Steaking contract.

Tools Used

Manual Review

Recommendations

Update the usersToStakes and totalAmountStaked before depositing into the vault:

@external
def depositIntoVault() -> uint256:
"""
@notice Allows users who have staked ETH during the staking period to deposit their ETH
into the WETH Steak vault.
@dev Before depositing into the vault, the raw ETH is converted into WETH.
@return The amount of shares received from the WETH Steak vault.
"""
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
+ self.usersToStakes[msg.sender] -= _amount
+ self.totalAmountStaked -= _amount
extcall IWETH(WETH).deposit(value=stakedAmount)
extcall IWETH(WETH).approve(self.vault, stakedAmount)
sharesReceived: uint256 = extcall IWETHSteakVault(self.vault).deposit(stakedAmount, msg.sender)
log DepositedIntoVault(msg.sender, stakedAmount, sharesReceived)
return sharesReceived
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.