ctf_sec - Vault_Synths.sol code does not consider protocol exchange fee when evaluating the Collateral worth

[github-actions[bot]]

ctf_sec

medium

Vault_Synths.sol code does not consider protocol exchange fee when evaluating the Collateral worth

Summary

Vault_Synths.sol code does not consider protocol fee.

Vulnerability Detail

If we look into the good-written documentation:

https://github.com/kree-dotcom/isomorph/blob/789338c8979ab75b8187781a2500908bb26dcdea/docs/Vault_Lyra.md#getwithdrawalfee

I want to quote:

Because the withdrawalFee of a lyra LP pool can vary we must fetch it each time it is needed to ensure we use an accurate value. LP tokens are devalued by this as a safety measure as any liquidation would include selling the collateral and so should factor in that cost to ensure it is profitable.

In Vault_Lyra.sol, when calculating the collateral of the LP token, the fee is taken into consideration.

function priceCollateralToUSD(bytes32 _currencyKey, uint256 _amount) public view override returns(uint256){
     //The LiquidityPool associated with the LP Token is used for pricing
    ILiquidityPoolAvalon LiquidityPool = ILiquidityPoolAvalon(collateralBook.liquidityPoolOf(_currencyKey));
    //we have already checked for stale greeks so here we call the basic price function.
    uint256 tokenPrice = LiquidityPool.getTokenPrice();          
    uint256 withdrawalFee = _getWithdrawalFee(LiquidityPool);
    uint256 USDValue  = (_amount * tokenPrice) / LOAN_SCALE;
    //we remove the Liquidity Pool withdrawalFee 
    //as there's no way to remove the LP position without paying this.
    uint256 USDValueAfterFee = USDValue * (LOAN_SCALE- withdrawalFee)/LOAN_SCALE;
    return(USDValueAfterFee);
}

This is not the case for Vault_Synths.sol, the underlying token also charge exchange fee, but this fee is not reflected when evaluating the Collateral worth.

https://docs.synthetix.io/incentives/#exchange-fees

Exchange fees are generated whenever a user exchanges one synthetic asset (Synth) for another through Synthetix.Exchange. Fees are typically between 10-100 bps (0.1%-1%), though usually 30 bps, and when generated are sent to the fee pool, where it is available to be claimed proportionally by SNX stakers each week.

If we go to https://synthetix.io/synths,

we can see that the sETH token charges 0.25%, the sBTC token charges 0.25%, the sUSD charges 0% fee, but this does not ensure this fee rate will not change in the future.

Impact

The collateral may be overvalued because the exchange does not count when evaluating the Collateral worth and result in bad debt which makes the project insolvent.

Code Snippet

Tool used

Manual Review

Recommendation

We recommend the project consider protocol exchange fee when evaluating the Collateral worth in Vault_Synths.sol

https://github.com/Synthetixio/synthetix/blob/develop/contracts/SystemSettings.sol#L362

Precisely when the exchange fee is updated, the fee is reflected in the collateral worth.

    function setExchangeFeeRateForSynths(bytes32[] calldata synthKeys, uint256[] calldata exchangeFeeRates)
        external
        onlyOwner
    {
        flexibleStorage().setExchangeFeeRateForSynths(SETTING_EXCHANGE_FEE_RATE, synthKeys, exchangeFeeRates);
        for (uint i = 0; i < synthKeys.length; i++) {
            emit ExchangeFeeUpdated(synthKeys[i], exchangeFeeRates[i]);
        }
    }

    /// @notice Set exchange dynamic fee threshold constant in decimal ratio
    /// @param threshold The exchange dynamic fee threshold
    /// @return uint threshold constant
    function setExchangeDynamicFeeThreshold(uint threshold) external onlyOwner {
        require(threshold != 0, "Threshold cannot be 0");

        flexibleStorage().setUIntValue(SETTING_CONTRACT_NAME, SETTING_EXCHANGE_DYNAMIC_FEE_THRESHOLD, threshold);

        emit ExchangeDynamicFeeThresholdUpdated(threshold);
    }

[kree-dotcom]

Fixed, https://github.com/kree-dotcom/isomorph/commit/f8ddef0671d8e33bd0a019aaa26834aab6688306 . The exchange rate of the given Synth collateral to sUSD is calculated on each valuation call and removed from the collateral value.

[IAm0x52]

Fixes look good. Exchange fee is now queried and removed from the valuation of the collateral