Skip to content

Latest commit

 

History

History
75 lines (54 loc) · 5.69 KB

H-02.md

File metadata and controls

75 lines (54 loc) · 5.69 KB
Raw

WstEth derivative assumes a ~1=1 peg of stETH to ETH

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.