Asymmetry contest - adriro's results

Lines of code

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

Vulnerability details

Impact

The stake function present in the SafEth contract accepts user deposits and stakes them into the different derivatives. To calculate the user shares in the vault, the function will first calculate the total ETH value for all the current staked assets and then it will calculate how much ETH resulted after splitting by weight the deposit into the different derivatives. Both of these calculations use the ethPerDerivative function of each derivative.

To calculate the total value in ETH for the current staked assets (underlyingValue), the implementation will loop each derivative and multiply its balance by the result of ethPerDerivative (that returns the current ETH value for one unit, 1e18, of the derivative asset). Note that here we are passing the total balance of the derivative as the argument to the ethPerDerivative function.

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

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;

To calculate the ETH value of the user deposit (totalStakeValueEth), the function also loops each derivative, deposits the corresponding weighted proportion and then multiplies the deposited amount by the return value of ethPerDerivative. Note that here the argument to the ethPerDerivative function is the user deposited amount.

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

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;
}

The implementation finally calculates the shares to mint by taking the proportion of the user staked value over the total staked value. Here preDepositPrice is the underlyingValue divided by the totalSupply of the shares. Working out the math, this calculation ends up being effectively the canonical formula to calculate shares in a pool, depositAmount * totalSupply / totalAssets.

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

uint256 mintAmount = (totalStakeValueEth * 10 ** 18) / preDepositPrice;

Now, this behavior is particularly concerning with the Reth implementation of the ethPerDerivative function as its implementation is not constant. In this derivative, the result will depend on the amount parameter, as it switches between using the reference conversion from the Rocket Pool contracts and the spot price of the Uniswap V3 pool.

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

function ethPerDerivative(uint256 _amount) public view returns (uint256) {
    if (poolCanDeposit(_amount))
        return
            RocketTokenRETHInterface(rethAddress()).getEthValue(10 ** 18);
    else return (poolPrice() * 10 ** 18) / (10 ** 18);
}

The issue here is that the total value in ETH for the current staked assets (underlyingValue) can be calculated with a different conversion rate than the ETH value of the user deposit (totalStakeValueEth). During the calculation of underlyingValue the argument to ethPerDerivative is the entire Reth balance of the protocol, while the calculation of totalStakeValueEth uses just the user deposit (which can be as low as the weighted fraction of the minimum deposit 0.5 ETH). The poolCanDeposit runs some checks based on the current state of the Rocket Pool protocol (for example, check that the amount doesn't exceed the maximum deposit value the protocol allows) and it is completely feasible to return false on a large amount and true for a small amount.

Given this scenario, the user will either be incorrectly minted less or more shares, depending on the current conversion rates from the Rocket Pool protocol and the Uniswap V3 pool. If the rate of Rocket Pool is higher than the Uniswap V3 rate, the user will be minted more shares, as their deposit value will be using a higher rate. If the rate of Rocket Pool is lower than the Uniswap V3 rate, the used will be minted less shares, as their deposit value will be using a lower rate.

Proof of Concept

Example scenario to describe the issue:

• Rocket Pool maximum deposit value is 1000 ETH.
• Rocket Pool current deposit value is 950 ETH.
• SafEth current balance for Reth derivative is 100 ETH.
• SafEth weights are equal for all derivatives (meaning user deposit value will be equally distributed).

Steps:

1. User stakes 1.5 ETH
2. underlyingValue will be calculated using the Uniswap V3 pool rate as poolCanDeposit will return false since 950 + 100 > 1000.
3. totalStakeValueEth will be calculated using the Rocket Pool exchange rate, as the deposit amount for the Reth derivative will be 0.5 ETH (1.5 ETH divided by the 3 implemented derivatives) and poolCanDeposit will return true since 950 + 0.5 <= 1000.
4. Shares calculation will end up using different valuations (user deposit vs total deposits) for the Reth derivative.

Recommendation

The stake function should ensure to use the same value of ethPerDerivative for both calculations, the total value in ETH for the current staked assets (underlyingValue) and the ETH value of the user deposit (totalStakeValueEth). An easy solution would be to use the user deposit value during both calls to ethPerDerivative.

Lines of code

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

Vulnerability details

Impact

The Reth derivative contract implements the poolPrice function to get the spot price of the derivative asset using a Uniswap V3 pool. The function queries the pool to fetch the sqrtPriceX96 and does the following calculation:

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

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 main issue here is that the multiplications in the expression sqrtPriceX96 * (uint(sqrtPriceX96)) * (1e18) may eventually overflow. This case is taken into consideration by the implementation of the OracleLibrary.getQuoteAtTick function which is part of the Uniswap V3 periphery set of contracts.

https://github.com/Uniswap/v3-periphery/blob/main/contracts/libraries/OracleLibrary.sol#L49-L69

49:     function getQuoteAtTick(
50:         int24 tick,
51:         uint128 baseAmount,
52:         address baseToken,
53:         address quoteToken
54:     ) internal pure returns (uint256 quoteAmount) {
55:         uint160 sqrtRatioX96 = TickMath.getSqrtRatioAtTick(tick);
56: 
57:         // Calculate quoteAmount with better precision if it doesn't overflow when multiplied by itself
58:         if (sqrtRatioX96 <= type(uint128).max) {
59:             uint256 ratioX192 = uint256(sqrtRatioX96) * sqrtRatioX96;
60:             quoteAmount = baseToken < quoteToken
61:                 ? FullMath.mulDiv(ratioX192, baseAmount, 1 << 192)
62:                 : FullMath.mulDiv(1 << 192, baseAmount, ratioX192);
63:         } else {
64:             uint256 ratioX128 = FullMath.mulDiv(sqrtRatioX96, sqrtRatioX96, 1 << 64);
65:             quoteAmount = baseToken < quoteToken
66:                 ? FullMath.mulDiv(ratioX128, baseAmount, 1 << 128)
67:                 : FullMath.mulDiv(1 << 128, baseAmount, ratioX128);
68:         }
69:     }

Note that this implementation guards against different numerical issues. In particular, the if in line 58 checks for a potential overflow of sqrtRatioX96 and switches the implementation to avoid the issue.

Recommendation

The poolPrice function can delegate the calculation directly to the OracleLibrary.getQuoteAtTick function of the v3-periphery package:

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)
    );
    (, int24 tick, , , , , ) = pool.slot0();
    return OracleLibrary.getQuoteAtTick(tick, 1e18, rocketTokenRETHAddress, W_ETH_ADDRESS);
}

Lines of code

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

Vulnerability details

Impact

The WstEth contract implements the ETH derivative for the Lido protocol. The stETH token is the liquid representation of the ETH staked in this protocol.

There are two different places in the codebase that indicate that the implementation is assuming a peg of 1 ETH ~= 1 stETH, each with different consequences. Even though both tokens have a tendency to keep the peg, this hasn't been always the case as it can be seen in this charth or this dashboard. There have been many episodes of market volatility that affected the price of stETH, notably the one in last June when stETH traded at ~0.93 ETH.

The first indication of such an assumption is the implementation of ethPerDerivative. This function is intended to work as an estimation of the current value in ETH of one unit (1e18) of the underlying asset. In this implementation, the function simply queries the amount of stETH for one unit (1e18) of wstETH and returns that value, which clearly indicates a conversion rate of 1 stETH = 1 ETH.

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

function ethPerDerivative(uint256 _amount) public view returns (uint256) {
    return IWStETH(WST_ETH).getStETHByWstETH(10 ** 18);
}

The other indication and most critical one is in the withdraw function. This function is used by the SafEth contract to unstake user positions and rebalance weights. In the implementation for the WstEth derivative, the function will unwrap the wstETH for stETH and use the Curve pool to exchange the stETH for ETH:

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

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

The issue is the calculation of the minOut variable that is sent to the Curve exchange function to validate the output amount of the trade. As we can see in line 60, the calculation is simply applying the slippage percentage to stETH balance. This means that for example, given the default slippage value of 1%, trading 1 stETH will succeed only if the rate is above 0.99. Larger amounts will be more concerning as the Curve AMM implements non-linear invariants, the price impact will be bigger. The rebalanceToWeights function withdraws all the balance before rebalancing, which means it will try to swap all the stETH held by the contract.

This could be mitigated by adjusting the maxSlippage variable to allow for lower exchange rates. However this would imply additional issues. First, the setMaxSlippage is an admin function that needs to be manually updated with extreme care. In times of high volatility the owners won't be able to update this variable as frequently as needed to keep up with the exchange rate. This means that users that want to exit their position won't be able to do so since the exchange for this derivative will fail (see PoC for a detailed example). Second, on the contrary, if the owners decide to set a higher slippage value by default to allow for unexpected market conditions, withdrawals and rebalancing (in particular) will be victim of sandwich attacks by MEV bots.

Proof of Concept

The following test replicates the market conditions during last June where stETH was trading at 0.93 ETH (needs to be forked from mainnet at block ~15000000). Here, the user wants to exit their position but the call to unstake will revert since the exchange in the Curve pool will fail as the output amount will be less than the expected minimum.

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

// Run this test forking mainnet at block height 15000000
function test_WstEth_withdraw_AssumesPegToEth() public {
    // Setup derivative
    vm.prank(deployer);
    safEth.addDerivative(address(wstEth), 1e18);

    // Deal balance to user
    uint256 depositValue = 1 ether;
    vm.deal(user, depositValue);

    // user stakes ether
    vm.prank(user);
    safEth.stake{value: depositValue}();

    // user tries to unstake, action will fail due to stETH being prices at around 0.93-0.95 ETH
    uint256 userShares = safEth.balanceOf(user);
    vm.prank(user);
    vm.expectRevert("Exchange resulted in fewer coins than expected");
    safEth.unstake(userShares);
}

Recommendation

The user should be able to decide on the slippage and set the expected minimum output amount to correctly handle different market conditions and user expectations. Similar to how decentralized exchanges work, the user experience can be improved by using a front-end that queries current exchange rates and offers the user a preview of the estimated output amount.

The ethPerDerivative function should also take into account the results of swapping the stETH for ETH using the Curve pool, similar to how the SfrxEth derivative implementation works.

Lines of code

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

Vulnerability details

Impact

Withdrawals in the Reth derivative are handled directly by the Rocket Pool protocol. The withdraw function calls the burn function of the RETH token contract:

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

function withdraw(uint256 amount) external onlyOwner {
    RocketTokenRETHInterface(rethAddress()).burn(amount);
    // solhint-disable-next-line
    (bool sent, ) = address(msg.sender).call{value: address(this).balance}(
        ""
    );
    require(sent, "Failed to send Ether");
}

Expanding on the burn function we can see that its functionality depends on the availability of collateral to handle the requested amount:

https://github.com/rocket-pool/rocketpool/blob/master/contracts/contract/token/RocketTokenRETH.sol#L106-L123

106:     function burn(uint256 _rethAmount) override external {
107:         // Check rETH amount
108:         require(_rethAmount > 0, "Invalid token burn amount");
109:         require(balanceOf(msg.sender) >= _rethAmount, "Insufficient rETH balance");
110:         // Get ETH amount
111:         uint256 ethAmount = getEthValue(_rethAmount);
112:         // Get & check ETH balance
113:         uint256 ethBalance = getTotalCollateral();
114:         require(ethBalance >= ethAmount, "Insufficient ETH balance for exchange");
115:         // Update balance & supply
116:         _burn(msg.sender, _rethAmount);
117:         // Withdraw ETH from deposit pool if required
118:         withdrawDepositCollateral(ethAmount);
119:         // Transfer ETH to sender
120:         msg.sender.transfer(ethAmount);
121:         // Emit tokens burned event
122:         emit TokensBurned(msg.sender, _rethAmount, ethAmount, block.timestamp);
123:     }

Line 113 fetches the current collateral balance and line 114 reverts if the available collateral is not enough to cover the requested amount.

This means that protocol actions that withdraw from the Reth derivative may eventually fail and block the whole operation. These actions include the user unstake from SafEth and the rebalancing of the derivatives, the latter being particularly concerning because its implementation will withdraw all balance from the Reth derivative.

Proof of Concept

User unstake scenario:

1. User tries to unstake their current position.
2. Assume calculated share of user position in Reth is greater than the available collateral present in the Rocket Pool protocol.
3. Call to unstake will revert because withdraw from Reth will fail.

Rebalancing scenario:

1. Protocol owner calls rebalanceToWeights.
2. Assume current total balance in Reth derivative is greater than the available collateral present in the Rocket Pool protocol.
3. Call to rebalanceToWeights will revert because withdraw from Reth will fail.

Recommendation

Similar to how other derivatives work, the Reth implementation can fallback to swapping assets in the Uniswap pool. The withdraw function can query the current available collateral in the Rocket Pool protocol. If the requested withdrawal amount is greater than the available collateral, the function can exchange the remaining portion using the Uniswap pool.

Lines of code

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

Vulnerability details

Impact

The ability to stake ETH in the Rocket Pool protocol depends on certain external conditions related to the state of the protocol's contracts. The Reth derivative deals with this by checking if the deposit amount can be handled by Rocket Pool and switching to swapping ETH for RETH if not.

The conditions to switch between the two paths are implemented in the poolCanDeposit function. This function check if the deposit amount is above the required minimum and if the current pool balance plus the current deposit value doesn't exceed the maximum allowed by the protocol:

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

function poolCanDeposit(uint256 _amount) private view returns (bool) {
    address rocketDepositPoolAddress = RocketStorageInterface(
        ROCKET_STORAGE_ADDRESS
    ).getAddress(
            keccak256(
                abi.encodePacked("contract.address", "rocketDepositPool")
            )
        );
    RocketDepositPoolInterface rocketDepositPool = RocketDepositPoolInterface(
            rocketDepositPoolAddress
        );

    address rocketProtocolSettingsAddress = RocketStorageInterface(
        ROCKET_STORAGE_ADDRESS
    ).getAddress(
            keccak256(
                abi.encodePacked(
                    "contract.address",
                    "rocketDAOProtocolSettingsDeposit"
                )
            )
        );
    RocketDAOProtocolSettingsDepositInterface rocketDAOProtocolSettingsDeposit = RocketDAOProtocolSettingsDepositInterface(
            rocketProtocolSettingsAddress
        );

    return
        rocketDepositPool.getBalance() + _amount <=
        rocketDAOProtocolSettingsDeposit.getMaximumDepositPoolSize() &&
        _amount >= rocketDAOProtocolSettingsDeposit.getMinimumDeposit();
}

These two conditions are correct, however the function is missing another condition. As we can see in the following snippet from the deposit function of the RocketDepositPool contract:

https://github.com/rocket-pool/rocketpool/blob/master/contracts/contract/deposit/RocketDepositPool.sol#L74-L91

function deposit() override external payable onlyThisLatestContract {
    // Check deposit settings
    RocketDAOProtocolSettingsDepositInterface rocketDAOProtocolSettingsDeposit = RocketDAOProtocolSettingsDepositInterface(getContractAddress("rocketDAOProtocolSettingsDeposit"));
    require(rocketDAOProtocolSettingsDeposit.getDepositEnabled(), "Deposits into Rocket Pool are currently disabled");
    require(msg.value >= rocketDAOProtocolSettingsDeposit.getMinimumDeposit(), "The deposited amount is less than the minimum deposit size");
    RocketVaultInterface rocketVault = RocketVaultInterface(getContractAddress("rocketVault"));
    require(rocketVault.balanceOf("rocketDepositPool").add(msg.value) <= rocketDAOProtocolSettingsDeposit.getMaximumDepositPoolSize(), "The deposit pool size after depositing exceeds the maximum size");
    // Calculate deposit fee
    uint256 depositFee = msg.value.mul(rocketDAOProtocolSettingsDeposit.getDepositFee()).div(calcBase);
    uint256 depositNet = msg.value.sub(depositFee);
    // Mint rETH to user account
    RocketTokenRETHInterface rocketTokenRETH = RocketTokenRETHInterface(getContractAddress("rocketTokenRETH"));
    rocketTokenRETH.mint(depositNet, msg.sender);
    // Emit deposit received event
    emit DepositReceived(msg.sender, msg.value, block.timestamp);
    // Process deposit
    processDeposit(rocketVault, rocketDAOProtocolSettingsDeposit);
}

In addition to the two checks implemented by poolCanDeposit, there's an extra condition that checks if deposits are enabled in the protocol settings contracts (rocketDAOProtocolSettingsDeposit.getDepositEnabled()). This condition should also be checked in poolCanDeposit as it will revert deposits in the derivative if deposits are not enabled in the protocol settings.

Proof of Concept

In the following test we simulate the conditions by impersonating the protocol's DAO account. First we make sure the maximum deposit settings can handle the current deposit value. Next we disable the deposits in the protocol settings (setSettingBool("deposit.enabled", false)) to replicate the described scenario. Then, when the user calls stake the deposit action will be routed to the Rocket Pool deposit function as poolCanDeposit will return true. The transaction will be reverted since the call to rocketDepositPool.deposit{value: msg.value}() will fail.

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

// Run this test forking mainnet at block height 16906254
function test_Reth_deposit_MissingConditionInCanDeposit() public {
    // Setup derivative
    vm.prank(deployer);
    safEth.addDerivative(address(reth), 1e18);

    uint256 depositValue = 1 ether;

    RocketStorageInterface rocketStorage = RocketStorageInterface(reth.ROCKET_STORAGE_ADDRESS());
    address dao = rocketStorage.getAddress(keccak256(abi.encodePacked("contract.address", "rocketDAOProtocolProposals")));
    address daoSettings = rocketStorage.getAddress(keccak256(abi.encodePacked("contract.address", "rocketDAOProtocolSettingsDeposit")));

    uint256 currentMaximumDeposit = RocketDAOProtocolSettingsDepositInterface(daoSettings).getMaximumDepositPoolSize();

    // Ensure there's room to deposit
    vm.prank(dao);
    (bool success, ) = daoSettings.call(abi.encodeWithSignature("setSettingUint(string,uint256)", "deposit.pool.maximum", currentMaximumDeposit + depositValue));
    require(success);

    // DAO disables deposits
    vm.prank(dao);
    (success, ) = daoSettings.call(abi.encodeWithSignature("setSettingBool(string,bool)", "deposit.enabled", false));
    require(success);

    // Deal balance to user
    vm.deal(user, depositValue);

    // user tries to stake ether, transaction is reverted
    vm.prank(user);
    vm.expectRevert("Deposits into Rocket Pool are currently disabled");
    safEth.stake{value: depositValue}();
}

Recommendation

The poolCanDeposit function should also check that deposits are enabled in the protocol settings, and fallback to swapping if deposits are currently disabled.

    function poolCanDeposit(uint256 _amount) private view returns (bool) {
        address rocketDepositPoolAddress = RocketStorageInterface(
            ROCKET_STORAGE_ADDRESS
        ).getAddress(
                keccak256(
                    abi.encodePacked("contract.address", "rocketDepositPool")
                )
            );
        RocketDepositPoolInterface rocketDepositPool = RocketDepositPoolInterface(
                rocketDepositPoolAddress
            );

        address rocketProtocolSettingsAddress = RocketStorageInterface(
            ROCKET_STORAGE_ADDRESS
        ).getAddress(
                keccak256(
                    abi.encodePacked(
                        "contract.address",
                        "rocketDAOProtocolSettingsDeposit"
                    )
                )
            );
        RocketDAOProtocolSettingsDepositInterface rocketDAOProtocolSettingsDeposit = RocketDAOProtocolSettingsDepositInterface(
                rocketProtocolSettingsAddress
            );

        return
+           rocketDAOProtocolSettingsDeposit.getDepositEnabled() &&
            rocketDepositPool.getBalance() + _amount <=
            rocketDAOProtocolSettingsDeposit.getMaximumDepositPoolSize() &&
            _amount >= rocketDAOProtocolSettingsDeposit.getMinimumDeposit();
    }

Lines of code

https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/derivatives/WstEth.sol#L73-L81 https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/derivatives/SfrxEth.sol#L94-L106

Vulnerability details

Impact

The WstEth derivative is used to stake ETH in the Lido protocol. The deposit function takes an ETH transfer from the SafEth contract and forwards it to the Lido wstETH contract:

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

function deposit() external payable onlyOwner returns (uint256) {
    uint256 wstEthBalancePre = IWStETH(WST_ETH).balanceOf(address(this));
    // solhint-disable-next-line
    (bool sent, ) = WST_ETH.call{value: msg.value}("");
    require(sent, "Failed to send Ether");
    uint256 wstEthBalancePost = IWStETH(WST_ETH).balanceOf(address(this));
    uint256 wstEthAmount = wstEthBalancePost - wstEthBalancePre;
    return (wstEthAmount);
}

In wstETH, the staking and minting of stETH will be handled by the submit function in the Lido contract (which inherits from stETH):

https://github.com/lidofinance/lido-dao/blob/master/contracts/0.6.12/WstETH.sol#L80-L83

receive() external payable {
    uint256 shares = stETH.submit{value: msg.value}(address(0));
    _mint(msg.sender, shares);
}

Expanding on the _submit function:

https://github.com/lidofinance/lido-dao/blob/master/contracts/0.4.24/Lido.sol#L675-L705

675:     function _submit(address _referral) internal returns (uint256) {
676:         require(msg.value != 0, "ZERO_DEPOSIT");
677: 
678:         StakeLimitState.Data memory stakeLimitData = STAKING_STATE_POSITION.getStorageStakeLimitStruct();
679:         require(!stakeLimitData.isStakingPaused(), "STAKING_PAUSED");
680: 
681:         if (stakeLimitData.isStakingLimitSet()) {
682:             uint256 currentStakeLimit = stakeLimitData.calculateCurrentStakeLimit();
683: 
684:             require(msg.value <= currentStakeLimit, "STAKE_LIMIT");
685: 
686:             STAKING_STATE_POSITION.setStorageStakeLimitStruct(
687:                 stakeLimitData.updatePrevStakeLimit(currentStakeLimit - msg.value)
688:             );
689:         }
690: 
691:         uint256 sharesAmount = getSharesByPooledEth(msg.value);
692:         if (sharesAmount == 0) {
693:             // totalControlledEther is 0: either the first-ever deposit or complete slashing
694:             // assume that shares correspond to Ether 1-to-1
695:             sharesAmount = msg.value;
696:         }
697: 
698:         _mintShares(msg.sender, sharesAmount);
699: 
700:         BUFFERED_ETHER_POSITION.setStorageUint256(_getBufferedEther().add(msg.value));
701:         emit Submitted(msg.sender, msg.value, _referral);
702: 
703:         _emitTransferAfterMintingShares(msg.sender, sharesAmount);
704:         return sharesAmount;
705:     }

As we can in the previous snippet, there are several conditions and requirements that will make the deposit fail:

1. Lines 678-679 check the settings for the isStakingPaused flag. If staking is currently paused, the operation will fail.
2. Lines 681-689 check that the requested amount is within the stake limits. If there's a limit set and the requested amount is beyond the limit, the operation will fail.
3. An edge case but technically valid is the check in line 676 for msg.value != 0. Staking in SafEth divides the deposit amount by weight. If the amount and weighted proportion for WstEth are low, the division may be rounded to 0.

Similarly, in the SfrxEth derivative, the deposit action is handled by the submitAndDeposit of the FRAX contract frxETHMinter:

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

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;
}

As we can see in the following snippet, the staking in the FRAX protocol has also some requirements:

https://github.com/FraxFinance/frxETH-public/blob/master/src/frxETHMinter.sol#L70-L101

070:     function submitAndDeposit(address recipient) external payable returns (uint256 shares) {
071:         // Give the frxETH to this contract after it is generated
072:         _submit(address(this));
073: 
074:         // Approve frxETH to sfrxETH for staking
075:         frxETHToken.approve(address(sfrxETHToken), msg.value);
076: 
077:         // Deposit the frxETH and give the generated sfrxETH to the final recipient
078:         uint256 sfrxeth_recieved = sfrxETHToken.deposit(msg.value, recipient);
079:         require(sfrxeth_recieved > 0, 'No sfrxETH was returned');
080: 
081:         return sfrxeth_recieved;
082:     }
083: 
084:     /// @notice Mint frxETH to the recipient using sender's funds. Internal portion
085:     function _submit(address recipient) internal nonReentrant {
086:         // Initial pause and value checks
087:         require(!submitPaused, "Submit is paused");
088:         require(msg.value != 0, "Cannot submit 0");
089: 
090:         // Give the sender frxETH
091:         frxETHToken.minter_mint(recipient, msg.value);
092: 
093:         // Track the amount of ETH that we are keeping
094:         uint256 withheld_amt = 0;
095:         if (withholdRatio != 0) {
096:             withheld_amt = (msg.value * withholdRatio) / RATIO_PRECISION;
097:             currentWithheldETH += withheld_amt;
098:         }
099: 
100:         emit ETHSubmitted(msg.sender, recipient, msg.value, withheld_amt);
101:     }

1. In line 87 there's a check for a pause setting. If staking is currently paused, the operation will fail.
2. Similar to the edge case described above, line 88 verifies that msg.value != 0. The division in the weighted proportion may round down the amount to 0.

In summary, this means that if any of these conditions during the staking process of the WstEth or SfrxEth derivative isn't met, then the deposit will fail and will block the whole staking and rebalancing processes in the SafEth contract.

Recommendation

Similar to how it is implemented in the Reth derivative, the deposit function of the WstEth and SfrxEth derivative can check the current state of the different staking requirements and decide between staking in the respective protocol (Lido or Frax) or taking an alternative path, such as swapping ETH for stETH using the Curve pool in the case of WstEth or swapping ETH for frxETH in the case of SfrxEth.

Judge has assessed an item in Issue #763 as 2 risk. The relevant finding follows:

In line 87 there's a check for a pause setting. If staking is currently paused, the operation will fail. Similar to the edge case described above, line 88 verifies that msg.value != 0. The division in the weighted proportion may round down the amount to 0. In summary, this means that if any of these conditions during the staking process of the WstEth or SfrxEth derivative isn't met, then the deposit will fail and will block the whole staking and rebalancing processes in the SafEth contract.

Lines of code

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

Vulnerability details

Impact

The SafEth contract allows protocol owners to adjust weights assigned to each supported derivative. The implementation also includes a function to rebalance all the derivatives to accommodate current balances to new weights:

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

function rebalanceToWeights() external onlyOwner {
    uint256 ethAmountBefore = address(this).balance;
    for (uint i = 0; i < derivativeCount; i++) {
        if (derivatives[i].balance() > 0)
            derivatives[i].withdraw(derivatives[i].balance());
    }
    uint256 ethAmountAfter = address(this).balance;
    uint256 ethAmountToRebalance = ethAmountAfter - ethAmountBefore;

    for (uint i = 0; i < derivativeCount; i++) {
        if (weights[i] == 0 || ethAmountToRebalance == 0) continue;
        uint256 ethAmount = (ethAmountToRebalance * weights[i]) /
            totalWeight;
        // Price will change due to slippage
        derivatives[i].deposit{value: ethAmount}();
    }
    emit Rebalanced();
}

As we can see in the snippet, the rebalanceToWeights function will withdraw all balance from each derivative to exchange them for ETH and then re-deposit everything again. This is highly inefficient and will result in losses, since swap fees and protocol fees will be paid for all the balance that each derivative holds.

Proof of Concept

In the following test we simulate a rebalancing of 200 ETH. After the unstake operation the user is left with 198.90 ETH, which implies a difference of a bit more than 0.5% (the numbers are taken by forking mainnet at block height 16906254).

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

// Run this test forking mainnet at block height 16906254
function test_SafEth_rebalanceToWeights_InefficientImplementation() public {
    // Setup derivatives
    vm.prank(deployer);
    safEth.addDerivative(address(wstEth), 1e18);

    vm.prank(deployer);
    safEth.addDerivative(address(reth), 1e18);

    vm.prank(deployer);
    safEth.addDerivative(address(sfrxEth), 1e18);

    // user has 200 ether
    uint256 initialAmount = 200 ether;
    vm.deal(user, initialAmount);

    // user stakes ether
    vm.prank(user);
    safEth.stake{value: initialAmount}();

    // protocol owner changes weight and rebalances
    vm.prank(deployer);
    safEth.adjustWeight(0, 2e18);
    vm.prank(deployer);
    safEth.rebalanceToWeights();

    // user unstakes
    uint256 userShares = safEth.balanceOf(user);
    vm.prank(user);
    safEth.unstake(userShares);

    // Balance is 198.902140399883614284 - A bit more than 0.5% is lost
    uint256 balanceAfter = user.balance;
    console.log(balanceAfter);
}

Recommendation

The rebalance operation doesn't need to withdraw all balance to ETH and convert it again. New weights can be accommodated by taking the difference of the current position and the expected position dictated by the new weights and just rebalancing that portion. More elaborate strategies can also be devised, since underlying assets from derivatives can be directly swapped for other assets instead of swapping them for ETH only to restake them in the other protocol.

Report

• Non Critical Issues (5)
• Low Issues (5)

Non Critical Issues

<a name="NC-1"></a>[NC-1] Bool expression compared to literal value

Bool expressions do not need to be compared against a literal value. For example, aBoolExpression == true can be directly stated as aBoolExpression, or aBoolExpression == false as !aBoolExpression.

Instances (2):

File: contracts/SafEth/SafEth.sol

65:         require(pauseStaking == false, "staking is paused");

117:         require(pauseUnstaking == false, "unstaking is paused");

<a name="NC-2"></a>[NC-2] Import declarations should import specific symbols

Prefer import declarations that specify the symbol(s) using the form import {SYMBOL} from "SomeContract.sol" rather than importing the whole file

Instances (31):

File: contracts/SafEth/SafEth.sol

4: import "@openzeppelin/contracts/token/ERC20/IERC20.sol";

5: import "../interfaces/IWETH.sol";

6: import "../interfaces/uniswap/ISwapRouter.sol";

7: import "../interfaces/lido/IWStETH.sol";

8: import "../interfaces/lido/IstETH.sol";

9: import "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";

10: import "./SafEthStorage.sol";

11: import "@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol";

File: contracts/SafEth/derivatives/Reth.sol

4: import "../../interfaces/IDerivative.sol";

5: import "../../interfaces/frax/IsFrxEth.sol";

6: import "@openzeppelin/contracts/token/ERC20/IERC20.sol";

7: import "../../interfaces/rocketpool/RocketStorageInterface.sol";

8: import "../../interfaces/rocketpool/RocketTokenRETHInterface.sol";

9: import "../../interfaces/rocketpool/RocketDepositPoolInterface.sol";

10: import "../../interfaces/rocketpool/RocketDAOProtocolSettingsDepositInterface.sol";

11: import "../../interfaces/IWETH.sol";

12: import "../../interfaces/uniswap/ISwapRouter.sol";

13: import "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";

14: import "../../interfaces/uniswap/IUniswapV3Factory.sol";

15: import "../../interfaces/uniswap/IUniswapV3Pool.sol";

File: contracts/SafEth/derivatives/SfrxEth.sol

4: import "../../interfaces/IDerivative.sol";

5: import "../../interfaces/frax/IsFrxEth.sol";

6: import "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";

7: import "@openzeppelin/contracts/token/ERC20/IERC20.sol";

8: import "../../interfaces/curve/IFrxEthEthPool.sol";

9: import "../../interfaces/frax/IFrxETHMinter.sol";

File: contracts/SafEth/derivatives/WstEth.sol

4: import "../../interfaces/IDerivative.sol";

5: import "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";

6: import "@openzeppelin/contracts/token/ERC20/IERC20.sol";

7: import "../../interfaces/curve/IStEthEthPool.sol";

8: import "../../interfaces/lido/IWStETH.sol";

<a name="NC-3"></a>[NC-3] Use uint256 instead of the uint alias

Prefer using the uint256 type definition over its uint alias.

Instances (16):

File: contracts/SafEth/SafEth.sol

26:     event Staked(address indexed recipient, uint ethIn, uint safEthOut);

26:     event Staked(address indexed recipient, uint ethIn, uint safEthOut);

27:     event Unstaked(address indexed recipient, uint ethOut, uint safEthIn);

27:     event Unstaked(address indexed recipient, uint ethOut, uint safEthIn);

28:     event WeightChange(uint indexed index, uint weight);

28:     event WeightChange(uint indexed index, uint weight);

31:         uint weight,

32:         uint index

72:         for (uint i = 0; i < derivativeCount; i++)

89:         for (uint i = 0; i < derivativeCount; i++) {

99:             uint derivativeReceivedEthValue = (derivative.ethPerDerivative(

149:         for (uint i = 0; i < derivativeCount; i++) {

158:         for (uint i = 0; i < derivativeCount; i++) {

219:         uint _derivativeIndex,

220:         uint _slippage

File: contracts/SafEth/derivatives/Reth.sol

183:             uint rethPerEth = (10 ** 36) / poolPrice();

<a name="NC-4"></a>[NC-4] Use constants for literal or magic values

Consider defining constants for literal or magic values as it improves readability and prevents duplication of config values.

Instances (5):

• https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/SafEth.sol#L54
• https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/SafEth.sol#L55
• https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/derivatives/Reth.sol#L44
• https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/derivatives/SfrxEth.sol#L38
• https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/derivatives/WstEth.sol#L35

<a name="NC-5"></a>[NC-5] Use rethAddress function instead of duplicating functionality to retrieve the RETH token address

Instances (2):

• https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/derivatives/Reth.sol#L187-L193
• https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/derivatives/Reth.sol#L229-L235

Low Issues

<a name="L-1"></a>[L-1] Contract files should define a locked compiler version

Contracts should be deployed with the same compiler version and flags that they have been tested with thoroughly. Locking the pragma helps to ensure that contracts do not accidentally get deployed using, for example, an outdated compiler version that might introduce bugs that affect the contract system negatively.

Instances (4):

File: contracts/SafEth/SafEth.sol

2: pragma solidity ^0.8.13;

File: contracts/SafEth/derivatives/Reth.sol

2: pragma solidity ^0.8.13;

File: contracts/SafEth/derivatives/SfrxEth.sol

2: pragma solidity ^0.8.13;

File: contracts/SafEth/derivatives/WstEth.sol

2: pragma solidity ^0.8.13;

<a name="L-2"></a>[L-2] Rounding down when dividing by weight will leave some ETH leftovers

When splitting ETH amounts by weight in order to determine how much corresponds to each derivative, there will be some leftovers that won't be considered due to the rounding. Consider assigning these leftovers to any arbitrary derivative, an easy strategy would be to keep track of how much balance has been already deposited in the derivatives and assign the remaining balance from the deposit to the last derivative.

This happens in the stake and rebalanceToWeights functions:

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

<a name="L-3"></a>[L-3] adjustWeight function should validate derivative index is within range

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

As the underlying data structure is a mapping instead of an array, it is possible to adjust the weight of an out-of-bounds index that will correspond to a invalid or non-existent derivative. The function should validate that the given index is within the valid range.

  function adjustWeight(
      uint256 _derivativeIndex,
      uint256 _weight
  ) external onlyOwner {
+     require(_derivativeIndex < derivativeCount);
      weights[_derivativeIndex] = _weight;
      uint256 localTotalWeight = 0;
      for (uint256 i = 0; i < derivativeCount; i++)
          localTotalWeight += weights[i];
      totalWeight = localTotalWeight;
      emit WeightChange(_derivativeIndex, _weight);
  }

<a name="L-4"></a>[L-4] ethPerDerivative function has confusing semantics

https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/interfaces/IDerivative.sol#L15

The ethPerDerivative function implemented by the different derivatives takes an amount as the argument which hints that this function should return the ETH value corresponding to the amount sent as a parameter of the derivate. This is not the case in neither of the 3 derivatives implemented, all of these return the ETH value for one unit (1e18, due to all tokens having 18 decimals) of the underlying derivative asset (wstETH, sfrxETH and RETH).

The amount paramater is mostly ignored by the implementations, except for the Reth derivative which uses it to switch between using the Rocker Pool protocol or the Uniswap pool.

Consider renaming the function to something more appropiate that reflects the semantics of the current implementation and less confusing in terms of its interface.

<a name="L-5"></a>[L-5] Protect access to receive() function

All major contracts implement the receive function to allow incoming ETH payments: derivatives need to accept ETH when unstaked or swapped, and SafEth needs to accept ETH coming from derivatives when unstaking or rebalancing.

As a precautionary measure, the different instances of this function can be protected to allow only transfers coming from the intended callers. In the case of SafEth these are the configured derivatives, and in the case of each derivative it should be the address of the corresponding protocol or exchange pool.

SafEth contract:

• derivatives[i].balance() is called twice in the stake function. Consider caching the first result to avoid an extra call and read from storage.
  https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/SafEth.sol#L73-L74

• derivatives[i].balance() is called twice in the rebalanceToWeights function. Consider caching the first result to avoid an extra call and read from storage.
  https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/SafEth.sol#L141-L142

• In adjustWeight, instead of looping all derivatives to recalculate the totalWeight, the function can just subtract the current weight for the derivative being modified and add the new weight.
  https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/SafEth.sol#L171-L173

  function adjustWeight(
      uint256 _derivativeIndex,
      uint256 _weight
  ) external onlyOwner {
      uint256 currentWeight = weights[_derivativeIndex];
      weights[_derivativeIndex] = _weight;
      totalWeight = totalWeight - currentWeight + _weight;
      emit WeightChange(_derivativeIndex, _weight);
  }

• In the addDerivative function, instead of recalculating the totalWeight by looping all derivatives, the implementations can just add the weight value for the new derivative to the totalWeight variable.
  https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/SafEth.sol#L190-L193

  function addDerivative(
      address _contractAddress,
      uint256 _weight
  ) external onlyOwner {
      derivatives[derivativeCount] = IDerivative(_contractAddress);
      weights[derivativeCount] = _weight;
      derivativeCount++;
      totalWeight += _weight;
      emit DerivativeAdded(_contractAddress, _weight, derivativeCount);
  }

• derivativeCount variable is read from storage 4 times in the addDerivative function. Consider caching this value in a local variable.
  https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/SafEth.sol#L186
  https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/SafEth.sol#L187
  https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/SafEth.sol#L191
  https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/SafEth.sol#L194

• minAmount amount is read from storage while emitting the event in the setMinAmount function. Consider using the value from the argument of the function to save an sload.
  https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/SafEth.sol#L216

• maxAmount amount is read from storage while emitting the event in the setMaxAmount function. Consider using the value from the argument of the function to save an sload.
  https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/SafEth.sol#L225

• pauseStaking value is read from storage while emitting the event in the setPauseStaking function. Consider using the value from the argument of the function to save an sload.
  https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/SafEth.sol#L234

• pauseUnstaking value is read from storage while emitting the event in the setPauseUnstaking function. Consider using the value from the argument of the function to save an sload.
  https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/SafEth.sol#L243

Reth contract

• Consider using an infinite token approval to the Uniswap pool instead of approving each individual token transfer for each swap involved in the calls to the deposit function.
  https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/derivatives/Reth.sol#L90

• The ethPerDerivative function multiplies and divides the result from poolPrice by the same amount (10 ** 18). Consider returning just the value from poolPrice to save gas.
  https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/derivatives/Reth.sol#L215

• Uniswap V3 pool addresses are deterministic and can be computed locally instead of making an external call to the factory. See PoolAddress.computeAddress.
  https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/derivatives/Reth.sol#L238

SfrxEth contract

• In the withdraw function, use the return value of the call to redeem to know how much frxETH was redeemed instead of making a call to fetch the contract balance.
  https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/derivatives/SfrxEth.sol#L61-L68

• Consider using an infinite token approval to the Curve pool instead of approving each individual token transfer for each swap involved in the calls to the withdraw function.
  https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/derivatives/SfrxEth.sol#L69-L72

• In the deposit function, use the return value of the call to submitAndDeposit to know how much sfrxETH was deposited instead of calculating it as the difference of balances pre/post execution.
  https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/derivatives/SfrxEth.sol#L98-L105

  function deposit() external payable onlyOwner returns (uint256) {
      IFrxETHMinter frxETHMinterContract = IFrxETHMinter(
          FRX_ETH_MINTER_ADDRESS
      );
      return frxETHMinterContract.submitAndDeposit{value: msg.value}(address(this));
  }

• In the withdraw function, use the return value of the call to exchange to know how much ETH was swapped.
  https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/derivatives/SfrxEth.sol#L77

WstEth contract

• Consider using an infinite token approval to the Curve pool instead of approving each individual token transfer for each swap involved in the calls to the withdraw function.
  https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/derivatives/WstEth.sol#L59

• In the withdraw function, use the return value of the call to unwrap to know how much stETH was unwrapped instead of making a call to fetch the contract balance.
  https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/derivatives/WstEth.sol#L57

  function withdraw(uint256 _amount) external onlyOwner {
      uint256 stEthBal = IWStETH(WST_ETH).unwrap(_amount);
      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");
  }

General

• There are several places throughout the code that could benefit from using a gas efficient math library such as solmate FixedPointMathLib.
  https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/SafEth.sol#L81
  https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/SafEth.sol#L92
  https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/SafEth.sol#L98
  https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/SafEth.sol#L115
  https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/SafEth.sol#L149
  https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/derivatives/Reth.sol#L173
  https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/derivatives/Reth.sol#L241
  https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/derivatives/SfrxEth.sol#L74
  https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/derivatives/SfrxEth.sol#L115
  https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/derivatives/WstEth.sol#L60