code-423n4 / 2024-04-dyad-findings

8 stars 6 forks source link

Incorrect Total Value Locked (TVL) Calculation Including Kerosene Assets. #540

Closed c4-bot-1 closed 4 months ago

c4-bot-1 commented 5 months ago

Lines of code

https://github.com/code-423n4/2024-04-dyad/blob/cd48c684a58158de444b24854ffd8f07d046c31b/src/core/Vault.kerosine.unbounded.sol#L56

Vulnerability details

Description

A bug has been identified within the Vault.Kerosene.unbounded:assetPrice() function. This bug inaccurately computes the Total Value Locked (TVL) by considering only Kerosene-containing vaults through kerosineManager.getVaults(), leading to an incorrect value of the collateral backing for DYAD tokens. This misrepresentation affects the protocol's sustainability and could lead to potential arbitrage issues.

Deploy.V2.s.sol

        function assetPrice() 
        public 
        view 
        override
        returns (uint) {
        uint tvl;
   @>   address[] memory vaults = kerosineManager.getVaults(); 
        uint numberOfVaults = vaults.length;
        for (uint i = 0; i < numberOfVaults; i++) {
            Vault vault = Vault(vaults[i]);
            tvl += vault.asset().balanceOf(address(vault)) 
                    * vault.assetPrice() * 1e18
                    / (10**vault.asset().decimals()) 
                    / (10**vault.oracle().decimals());
        }
        uint numerator   = tvl - dyad.totalSupply();
        uint denominator = kerosineDenominator.denominator();
        return numerator * 1e8 / denominator;
    }   

Impact The assetPrice() function is designed to calculate the price of assets by determining the protocol’s TVL. However, the current implementation mistakenly includes only vaults associated with Kerosene assets through the kerosineManager.getVaults() call. This contradicts the protocol's design, where Kerosene should not be counted in the TVL used for determining backing for DYAD tokens. The impact of this bug can lead to incorrect pricing of DYAD, potential manipulations, and systemic issues within the stablecoin framework.

Proof Of Concept

  1. Deploy the protocol with multiple vaults, including both Kerosene and non-Kerosene containing assets.
  2. Invoke assetPrice() function to calculate the asset price based on TVL.
  3. Observe that the returned value of the asset price is calculated using only Kerosene-contain vaults as a result of the kerosineManager.getVaults() call.

Recommended Mitigation:

Assessed type

Other

c4-pre-sort commented 5 months ago

JustDravee marked the issue as insufficient quality report

c4-judge commented 4 months ago

koolexcrypto marked the issue as unsatisfactory: Insufficient quality