Asymmetry contest - BPZ's results

Lines of code

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

Vulnerability details

Impact

The purpose of the ethPerDerivative function with-in the WstEth contract is to get the price of the derivative in terms of ETH. However the implementation of this function does not do that; this function will actually return the "amount of stETH tokens corresponding to one wstETH" (cited from the Lido Finance documentation for wstETH). This is problematic because the price of stETH in terms of ETH is not equal to 1, for example the current conversion 1 stETH = 0.992 ETH. The implications of this is that the protocol will misrepresent the underlying value in terms of ETH that has been allocated to the Lido-based derivative (WstEth contract). The direct impact of this misrepresentation of underlying ETH on the users can be seen with-in the stake function of the SafEth contract. This function will miscalculate the current underlying value of the Lido-based derivative and, additionally, miscalculate the amount the user is now staking, ultimately, over-valuing or under-valuing the user's position by minting an incorrect amount of SafEth tokens to the user.

Proof of Concept

The ethPerDerivative function implemented in the WstEth contract will not get the price of the derivative in terms of ETH:

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

This function will, instead, return the amount of stETH tokens representing one wstETH. For additional reference, please see the Lido Finance documentation on the getStETHByWstETH method.

Tools Used

Manual Audit Review

Recommended Mitigation Steps

It is recommended to refactor the ethPerDerivative function with-in the WstEth contract so that the resulting amount of stETH tokens representing one wstETH is then directly converted to ETH. This can be done by utilizing the get_dy method of the implemented stETH/ETH Curve Pool:

    function ethPerDerivative(uint256 _amount) public view returns (uint256) {
        uint256 stETHPerWstETH = IWStETH(WST_ETH).getStETHByWstETH(10 ** 18);
        return IStEthEthPool(LIDO_CRV_POOL).get_dy(1, 0, stETHPerWstETH);
    }

Lines of code

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

Vulnerability details

Not defining deadline in ExactInputSingleParams

To execute the swap function, its needed to define necessary swap data for ExactInputSingleParams struct.

Source : https://docs.uniswap.org/contracts/v3/reference/periphery/interfaces/ISwapRouter

Source : https://docs.uniswap.org/contracts/v3/guides/swaps/single-swaps

Proof of Concept

  177        uint256 amountSwapped = swapExactInputSingleHop(
  178                        W_ETH_ADDRESS,
  179                        rethAddress(),
  180                        500,
  181                        msg.value,
  182                        minOut
  183                    );

https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/derivatives/Reth.sol#[L177,L183]

  91      ISwapRouter.ExactInputSingleParams memory params = ISwapRouter
  92                  .ExactInputSingleParams({
  93                      tokenIn: _tokenIn,
  94                      tokenOut: _tokenOut,
  95                      fee: _poolFee,
  96                      recipient: address(this),
  97                      amountIn: _amountIn,
  98                      amountOutMinimum: _minOut,
  99                      sqrtPriceLimitX96: 0
  100                  });
  101              amountOut = ISwapRouter(UNISWAP_ROUTER).exactInputSingle(params);

https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/derivatives/Reth.sol#[L91,L101]

Here you can see that deadline parameter has not been defined so that it could finally revert the swap.

Tools Used

Manual Auditing

Recommended Mitigation Steps

Add the deadline: block.timestamp as shown below.

  91      ISwapRouter.ExactInputSingleParams memory params = ISwapRouter
  92                  .ExactInputSingleParams({
  93                      tokenIn: _tokenIn,
  94                      tokenOut: _tokenOut,
  95                      fee: _poolFee,
  96                      recipient: address(this),
  +                        deadline: block.timestamp,
  97                      amountIn: _amountIn,
  98                      amountOutMinimum: _minOut,
  99                      sqrtPriceLimitX96: 0
  100                  });
  101              amountOut = ISwapRouter(UNISWAP_ROUTER).exactInputSingle(params);
  

https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/derivatives/Reth.sol#[L91,L101]

Also its needed to change the ISwapRouter interface ExactInputSingleParams struct accordingly. Add the deadline: block.timestamp as shown below.

   5         struct ExactInputSingleParams {
   6             address tokenIn;
   7             address tokenOut;
   8             uint24 fee;
   9             address recipient;
   +              deadline: block.timestamp,
  10              uint amountIn;
  11              uint amountOutMinimum;
  12              uint160 sqrtPriceLimitX96;
  13          }

https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/interfaces/uniswap/ISwapRouter.sol#[L5,L13]

Lines of code

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

Vulnerability details

Function withdraw may be reverted due to hardcoded slippage during market turbulence conditions.

WstEth.sol has a hardcoded maxSlipage as 1%. It could revert the withdraw function during sudden price crashes.

Proof of Concept

35  maxSlippage = (1 * 10 ** 16); // 1%

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

60   uint256 minOut = (stEthBal * (10 ** 18 - maxSlippage)) / 10 ** 18;

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

Due to hard coded maxSlippage owner unable to define minOut so that its a constant (relative to the input parameter amount) , eventually Owner unable to perform the withdraw function during price crash conditions.

Tools Used

Manual Auditing

Recommended Mitigation Steps

Remove the hard coded maxSlippage & let owner to determine the maximum slippage he's willing to take with the current market condition as a input parameter for the withdraw function.