Skip to content

Latest commit

 

History

History
93 lines (79 loc) · 3.59 KB

M-04.md

File metadata and controls

93 lines (79 loc) · 3.59 KB
Raw

Inaccurate minimum exchange amount out calculation in SfrxEth withdrawal

Impact

The withdraw procedure for the SfrxEth derivative involves first redeeming the sfrxETH for frxETH from the ERC4626 vault and then exchanging the frxETH for ETH using the Curve pool.

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

60:     function withdraw(uint256 _amount) external onlyOwner {
61:         IsFrxEth(SFRX_ETH_ADDRESS).redeem(
62:             _amount,
63:             address(this),
64:             address(this)
65:         );
66:         uint256 frxEthBalance = IERC20(FRX_ETH_ADDRESS).balanceOf(
67:             address(this)
68:         );
69:         IsFrxEth(FRX_ETH_ADDRESS).approve(
70:             FRX_ETH_CRV_POOL_ADDRESS,
71:             frxEthBalance
72:         );
73: 
74:         uint256 minOut = (((ethPerDerivative(_amount) * _amount) / 10 ** 18) *
75:             (10 ** 18 - maxSlippage)) / 10 ** 18;
76: 
77:         IFrxEthEthPool(FRX_ETH_CRV_POOL_ADDRESS).exchange(
78:             1,
79:             0,
80:             frxEthBalance,
81:             minOut
82:         );
83:         // solhint-disable-next-line
84:         (bool sent, ) = address(msg.sender).call{value: address(this).balance}(
85:             ""
86:         );
87:         require(sent, "Failed to send Ether");
88:     }

In line 61 the sfrxETH is redeemed for frxETH and the swap happens in line 77. For the exchange, the expected minimum amount is calculated in line 74 using the ethPerDerivative function. As we can see in the following snippet, this function is dependent on the state of the sfrxETH vault:

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

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

ethPerDerivative uses the function convertToAssets to calculate the amount of assets that equal one unit (1e18) of shares. The main issue is that the ERC4626 vault has been previously altered in the call to redeem: the first action in the withdraw function already burned the shares in exchange for the assets (frxETH), which modified the total supply of assets and shares in the vault.

This means that using ethPerDerivative to calculate the minimum output amount in the swap for the original withdrawal amount is inaccurate, since ethPerDerivative uses the state of the vault which was already altered by the call to redeem.

Recommendation

The withdraw function in the SfrxEth derivative should calculate the minimum output amount before redeeming the shares.

function withdraw(uint256 _amount) external onlyOwner {
    uint256 minOut = (((ethPerDerivative(_amount) * _amount) / 10 ** 18) *
        (10 ** 18 - maxSlippage)) / 10 ** 18;
        
    IsFrxEth(SFRX_ETH_ADDRESS).redeem(
        _amount,
        address(this),
        address(this)
    );
    uint256 frxEthBalance = IERC20(FRX_ETH_ADDRESS).balanceOf(
        address(this)
    );
    IsFrxEth(FRX_ETH_ADDRESS).approve(
        FRX_ETH_CRV_POOL_ADDRESS,
        frxEthBalance
    );

    IFrxEthEthPool(FRX_ETH_CRV_POOL_ADDRESS).exchange(
        1,
        0,
        frxEthBalance,
        minOut
    );
    // solhint-disable-next-line
    (bool sent, ) = address(msg.sender).call{value: address(this).balance}(
        ""
    );
    require(sent, "Failed to send Ether");
}