c4-bot-8 commented 6 months ago

Lines of code

https://github.com/code-423n4/2024-04-dyad/blob/cd48c684a58158de444b24854ffd8f07d046c31b/src/core/Vault.kerosine.unbounded.sol#L65-L67 https://github.com/code-423n4/2024-04-dyad/blob/cd48c684a58158de444b24854ffd8f07d046c31b/src/staking/KerosineDenominator.sol#L21

Vulnerability details

Impact

Solidity’s floor rounding may cause rounding errors to affect functions in the UnboundedKerosineVault (UKV) contract.

Proof of Concept

Specifically, when calling UKV::assetPrice externally (e.g., for liquidation, minting, or withdrawal), if the numerator of tvl - dyad.totalSupply() is sufficiently small than the denominator of kerosineDenominator.denominator() then the assetPrice() return value may round down to 0.

UKV::assetPrice#L65-67

  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();
     // @audit the assetPrice() return value may round down to 0
      return numerator * 1e8 / denominator;
  }

Calling the UKV::assetPrice functions, which call the external KerosineDenominator::denominator function with this rounded denominator value, may result in depositors receiving little or no Kerosene ERC20. Additionally, calls to withdraw or liquidate may proceed without burning any Kerosene ERC20, which is not desirable, as it could lowers the cost for individual to mint DYAD.

KerosineDenominator::denominator

  function denominator() external view returns (uint) {
    // @dev: We subtract all the Kerosene in the multi-sig.
    //       We are aware that this is not a great solution. That is
    //       why we can switch out Denominator contracts.
    return kerosine.totalSupply() - kerosine.balanceOf(MAINNET_OWNER);
  }

Even if the Kerosene ERC20 supply is small enough to avoid this issue, an attacker could still use a flash loan to artificially inflate the total supply of Kerosene ERC20, causing rounding down and potentially front-running a victim’s deposit and then withdrawing the inflated Kerosene ERC20 supply to repay the flash loan.

Recommended Mitigation Steps

Consider reverting if UKV::assetPrice return a zero value. Additionally, consider multiplication by some scalar precision amount. Implementing protections against manipulation of the Kerosene ERC20 total supply within a single block would also be advisable, perhaps through implementing a multi-block delay.

Assessed type

Math

c4-pre-sort commented 5 months ago

JustDravee marked the issue as insufficient quality report

c4-judge commented 5 months ago

koolexcrypto marked the issue as unsatisfactory: Invalid

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