Asymmetry contest - Bauer's results

Lines of code

https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/derivatives/SfrxEth.sol#L111-L117

Vulnerability details

Impact

Due to a miscalculation of the price of derivative in terms of ETH, resulting in users getting more share, and the protocol will lose money

Proof of Concept

In the SfrxEth contract, the price of derivative in terms of ETH is calculated as below code.This calculation is wrong.

frxAmount:1031241986383403960 price_oracle:998817873681710532 As below code,the price of derivative in terms of ETH is 10**18*1031241986383403960/998817873681710532 = 1032462487462479700. Actually，the IFrxEthEthPool(FRX_ETH_CRV_POOL_ADDRESS).price_oracle() function returns the value of Eth per FrxETH. Hence,it should be calculated like this: frxAmount *IFrxEthEthPool(FRX_ETH_CRV_POOL_ADDRESS).price_oracle()/1e18 = 1030022928090775200,it will be 2439559371704576 less than the original value.

Hence, the protocol will lose assets and user will get more shares when staking ETH into the SafETH contract.

    function ethPerDerivative(uint256 _amount) public view returns (uint256) {
        uint256 frxAmount = IsFrxEth(SFRX_ETH_ADDRESS).convertToAssets(
            10 ** 18
        );
        return ((10 ** 18 * frxAmount) /
            IFrxEthEthPool(FRX_ETH_CRV_POOL_ADDRESS).price_oracle());
    }

Tools Used

vscode

Recommended Mitigation Steps

    function ethPerDerivative(uint256 _amount) public view returns (uint256) {
        uint256 frxAmount = IsFrxEth(SFRX_ETH_ADDRESS).convertToAssets(
            10 ** 18
        );
        return frxAmount * IFrxEthEthPool(FRX_ETH_CRV_POOL_ADDRESS).price_oracle()/1e18;
    }

Lines of code

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

Vulnerability details

Impact

The the minimum output of ETH is calculated incorrectly and could lead to sandwich attack.

Proof of Concept

The SafETH.unstake() is used to unstake safETH into ETH. When it is called, the protocol will withdraw shares from third-party contracts and finally swap the tokens to ETH. In the WstETH contract, the swap is performed on lido CRV pool. The protocol set max slippage for derivative. However,in the WstETH.withdraw() function, the protocol calculates the minimum output of stETH not ETH.We know that the price of stETH is lowwer then the price of ETH,the the minimum output of ETH is lower which may lead to sandwich attack.

    function withdraw(uint256 _amount) external onlyOwner {
        IWStETH(WST_ETH).unwrap(_amount);
        uint256 stEthBal = IERC20(STETH_TOKEN).balanceOf(address(this));
        IERC20(STETH_TOKEN).approve(LIDO_CRV_POOL, stEthBal);
        uint256 minOut = (stEthBal * (10 ** 18 - maxSlippage)) / 10 ** 18;
        IStEthEthPool(LIDO_CRV_POOL).exchange(1, 0, stEthBal, minOut);
        // solhint-disable-next-line
        (bool sent, ) = address(msg.sender).call{value: address(this).balance}(
            ""
        );
        require(sent, "Failed to send Ether");
    }

Tools Used

vscode

Recommended Mitigation Steps

add code like this uint256 minOut = (stEthBal*EthPricePerstETH * (10 ** 18 - maxSlippage)) / 10 ** 18;

Lines of code

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

Vulnerability details

Impact

Reth.poolPrice() uses the UniV3Pool.slot0 to get the price of derivative in liquidity pool. UniV3Pool.slot0 is the most recent data point and is therefore extremely easy to manipulate.

Proof of Concept

Reth.poolPrice() the UniV3Pool.slot0 to get the price of derivative in liquidity pool. UniV3Pool.slot0 is the most recent data point and can easily be manipulated.

    function poolPrice() private view returns (uint256) {
        address rocketTokenRETHAddress = RocketStorageInterface(
            ROCKET_STORAGE_ADDRESS
        ).getAddress(
                keccak256(
                    abi.encodePacked("contract.address", "rocketTokenRETH")
                )
            );
        IUniswapV3Factory factory = IUniswapV3Factory(UNI_V3_FACTORY);
        IUniswapV3Pool pool = IUniswapV3Pool(
            factory.getPool(rocketTokenRETHAddress, W_ETH_ADDRESS, 500)
        );
        (uint160 sqrtPriceX96, , , , , , ) = pool.slot0();
        return (sqrtPriceX96 * (uint(sqrtPriceX96)) * (1e18)) >> (96 * 2);
    }

The Reth.deposit() and Reth.ethPerDerivative() functions directly uses the token values returned by poolPrice(). This allows a malicious user to manipulate the valuation of the LP. An example of this kind of manipulation would be to use large buys/sells to alter the composition of the LP to make it worth less or more.This allows a malicious user to manipulate the rethPerEth and ethPerDerivative. An example of this kind of manipulation would be to use large buys/sells to make it worth less or more.

Tools Used

vscode

Recommended Mitigation Steps

Use a TWAP instead of slot0.

Lines of code

https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/derivatives/Reth.sol#L171-L174

Vulnerability details

Impact

Calculate the minOut like this can reduce unnecessary precision loss and save gas cost.

Proof of Concept

In the Reth contract,the protocol will swap weth to reth if the condition !poolCanDeposit(msg.value) is met. The swap is performed on uniswap v3, the minOut is calculated according to code below.Actually, poolPrice() is the price of eth per reth. Hence ,the minOut can be calculated like this:uint256 minOut = (((msg.value * 10 ** 18 / poolPrice()) * ((10 ** 18 - maxSlippage))) / 10 ** 18)

This can reduce unnecessary precision loss and save gas cost.

        if (!poolCanDeposit(msg.value)) {
            uint rethPerEth = (10 ** 36) / poolPrice();

            uint256 minOut = ((((rethPerEth * msg.value) / 10 ** 18) *
                ((10 ** 18 - maxSlippage))) / 10 ** 18);

            IWETH(W_ETH_ADDRESS).deposit{value: msg.value}();
            uint256 amountSwapped = swapExactInputSingleHop(
                W_ETH_ADDRESS,
                rethAddress(),
                500,
                msg.value,
                minOut
            );

            return amountSwapped;

Tools Used

vscode

Recommended Mitigation Steps

uint256 minOut = (((msg.value * 10 ** 18/poolPrice()) * ((10 ** 18 - maxSlippage))) / 10 ** 18)

Lines of code

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

Vulnerability details

Impact

The frxETHMinter contract can be paused which block staking

Proof of Concept

When user stakes ETH into the SafEth contract, the protocol will deposit the ETH into the FrxETHMinter contract. However,in FrxETHMinter contract the deposit can be paused.

SfrxEth.sol
    function deposit() external payable onlyOwner returns (uint256) {
        IFrxETHMinter frxETHMinterContract = IFrxETHMinter(
            FRX_ETH_MINTER_ADDRESS
        );
        uint256 sfrxBalancePre = IERC20(SFRX_ETH_ADDRESS).balanceOf(
            address(this)
        );
        frxETHMinterContract.submitAndDeposit{value: msg.value}(address(this));
        uint256 sfrxBalancePost = IERC20(SFRX_ETH_ADDRESS).balanceOf(
            address(this)
        );
        return sfrxBalancePost - sfrxBalancePre;
    }

In frxETHMinter contract: https://etherscan.io/address/0xbafa44efe7901e04e39dad13167d089c559c1138#code#F1#L87

 function submitAndDeposit(address recipient) external payable returns (uint256 shares) {
        // Give the frxETH to this contract after it is generated
        _submit(address(this));

        // Approve frxETH to sfrxETH for staking
        frxETHToken.approve(address(sfrxETHToken), msg.value);

        // Deposit the frxETH and give the generated sfrxETH to the final recipient
        uint256 sfrxeth_recieved = sfrxETHToken.deposit(msg.value, recipient);
        require(sfrxeth_recieved > 0, 'No sfrxETH was returned');

        return sfrxeth_recieved;
    }

    /// @notice Mint frxETH to the recipient using sender's funds. Internal portion
    function _submit(address recipient) internal nonReentrant {
        // Initial pause and value checks
        require(!submitPaused, "Submit is paused");
        require(msg.value != 0, "Cannot submit 0");

        // Give the sender frxETH
        frxETHToken.minter_mint(recipient, msg.value);

Note the require in the _submit() function.

Tools Used

Recommended Mitigation Steps

If paused, swap the ETH to sfrxEth ,just like the reth contract.