Denial of Service via Asset Price Underflow in Unbounded Kerosine Vault

[c4-bot-4]

Lines of code

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

Vulnerability details

Impact

The potential underflow in the Vault.kerosine.unbounded::assetPrice() function can lead to a denial of service where users are unable to withdraw their assets from the Unbounded kerosine vault. This can be exploited by an attacker to manipulate the TVL, potentially causing financial loss to users.This could lead to loss of confidence in the system and financial loss for users if they are unable to access their assets when needed.

Proof of Concept

In Deployment Script a new WETH or wstETH vault is created without any initial assets, resulting in a TVL of zero. Additionally, the deployment script grants an unbounded vault to the vault licenser, so the unbounded vault can be added to the VaultManagerV2 using the add function. This also satisfies the check of !vaultLicenser.isLicensed(vault).

Case 1: When the TVL < dyad.totalSupply()

user add the Unbouded kerosine vault using `VaultManagerV2::add` A user deposits assets into the `Unbounded kerosine vault`. The user attempts to [withdraw](https://github.com/code-423n4/2024-04-dyad/blob/cd48c684a58158de444b24854ffd8f07d046c31b/src/core/VaultManagerV2.sol#L134), triggering the `vault.assetPrice()` function ```jsx function withdraw( uint id, address vault, uint amount, address to ) public isDNftOwner(id) { ///........ uint value = amount * _vault.assetPrice() * 1e18 / 10**_vault.oracle().decimals() // oracle decimal is 8 / 10**_vault.asset().decimals(); ///...... } ``` Since the user is withdrawing from the `Unbounded Kerosene vault`, the `assetPrice()` function specific to the `Unbounded vault` is called. This function's calculation depends on the TVL of the exogenous vaults (e.g., WETH, wstETH), which can be influenced by external users. ```jsx 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()); } //@audit-> underflow reverts when tvl < totalsupply uint numerator = tvl - dyad.totalSupply(); uint denominator = kerosineDenominator.denominator(); return numerator * 1e8 / denominator; } ``` Since the TVL is zero and the dyad total supply is greater than zero, the subtraction in the [assetPrice calculation](https://github.com/code-423n4/2024-04-dyad/blob/cd48c684a58158de444b24854ffd8f07d046c31b/src/core/Vault.kerosine.unbounded.sol#L65) results in an underflow. Even if a user deposits `WETH` and `wstETH`, for a withdrawal to succeed, it must be ensured that the TVL is greater than `dyad.totalSupply()`. If the TVL is less than `dyad.totalSupply()` after a user's deposit, the transaction will fail, and the user will not be able to `withdraw`.

Case 2: When the TVL > dyad.totalSupply()

User Prepares Withdrawal: A user initiates a withdrawal from a `unbounded vault`with a significant TVL in `exogenous Vault`. Attacker Monitors: An attacker, holding a large portion of the assets in the`exogenous Vault`, monitors the network for pending withdrawal transactions. Attacker Front-Runs: Upon detecting a user's withdrawal transaction, the attacker submits their own withdrawal from the exogenousVault. and submit a transaction with a higher gas fee to ensure it gets processed first. Attacker Withdraws Assets: The attacker's withdrawal transaction is confirmed first due to the higher gas fee, significantly reducing the TVL in the vault. `now TVL < dyad.totalSupply()` User's Withdrawal Transaction Processes: When the user's withdrawal transaction is processed, it triggers the `Vault.assetPrice()` ex(unbounded vault) function. TVL Falls Below Dyad Total Supply: The attacker's withdrawal may have caused the TVL to fall below the dyad total supply. Underflow in Asset Price Calculation: The assetPrice function calculates a negative value due to underflow, as the TVL is now less than the dyad total supply. Transaction Reverts: The smart contract reverts the user's withdrawal transaction because of the `underflow`, preventing the user from retrieving their assets and effectively locking them in the `unboundedVault`. In both scenarios, the attacker exploits the underflow vulnerability in the `assetPrice` function, which assumes that the TVL will always be greater than the dyad total supply. This vulnerability can lead to a denial of service for users trying to withdraw their assets.

Case 3: When the TVL == dyad.totalSupply()

here's a example of how the `TVL` could be manipulated to equal `dyad.totalSupply()`, causing the `assetPrice` to return zero and triggering a failure in the withdrawal process due to the [NotEnoughExoCollat](https://github.com/code-423n4/2024-04-dyad/blob/cd48c684a58158de444b24854ffd8f07d046c31b/src/core/VaultManagerV2.sol#L150) check, we would follow these steps: Initial State: Assume the contract has a certain tvl (total value locked) and `dyad.totalSupply()` that are not equal. The [assetPrice](https://github.com/code-423n4/2024-04-dyad/blob/cd48c684a58158de444b24854ffd8f07d046c31b/src/core/Vault.kerosine.unbounded.sol#L50) is therefore greater than zero. Deposit/Withdraw Manipulation: An actor (could be a user or a group) starts interacting with the exogenous vaults that contribute to the tvl calculation. They could either `deposit` more assets into the exogenous vaults or `withdraw` assets from them. Equalizing TVL and Dyad Supply: The actor continues to adjust their `deposits` and `withdrawals` until the `tvl` exactly equals `dyad.totalSupply()`. This could be done by monitoring the contract state off-chain and performing transactions accordingly. TVL Equals Dyad Supply: Once `tvl` is equal to `dyad.totalSupply()`, the `numerator` in the `assetPrice` calculation becomes zero [tvl - dyad.totalSupply() = 0](https://github.com/code-423n4/2024-04-dyad/blob/cd48c684a58158de444b24854ffd8f07d046c31b/src/core/Vault.kerosine.unbounded.sol#L65). Asset Price Calculation: When the `assetPrice` function is called, it returns zero because the numerator is zero, making the price calculation `0 * 1e8 / denominator` = 0. Withdrawal Attempt: A user tries to [withdraw](https://github.com/code-423n4/2024-04-dyad/blob/cd48c684a58158de444b24854ffd8f07d046c31b/src/core/VaultManagerV2.sol#L134) their funds from the `unbounded vault`. The withdrawal function calls `getNonKeroseneValue(id)`, which relies on `assetPrice` of vault whhich is unbounded vault here and to determine the USD value of the assets that are not part of the Kerosene collateral. Zero Asset Price Impact: Since `assetPrice` is zero, `getNonKeroseneValue(id)` also returns zero. This means that the system believes there is no non-Kerosene collateral value. NotEnoughExoCollat Check: The withdrawal function then performs a check to ensure that the user has enough exogenous collateral to cover the withdrawal after accounting for the Kerosene collateral (dyadMinted). The check is [if (getNonKeroseneValue(id) - value < dyadMinted) revert NotEnoughExoCollat]((https://github.com/code-423n4/2024-04-dyad/blob/cd48c684a58158de444b24854ffd8f07d046c31b/src/core/VaultManagerV2.sol#L150));. Revert Condition Met: Because `getNonKeroseneValue(id)` returns zero, any non-zero value requested for withdrawal will cause the subtraction to `underflow` (which is prevented by Solidity 0.8.x), or it will simply be less than `dyadMinted`, thus triggering the `NotEnoughExoCollat revert`. Withdrawal Failure: The transaction is reverted, and the user is unable to withdraw their funds, effectively locking assets in the vault until the issue is resolved.

NOTE: This could also affect the redeemDyad function in the same way it affects the withdraw function. Users may not be able to redeemDyad because the underflow could cause the transaction to revert.

Tools Used

Manual Review

Recommended Mitigation Steps

Validation Checks: Implement validation checks in the assetPrice function to ensure that the TVL is always greater than or equal to dyad.totalSupply(). If not, the function should return a meaningful error rather than allowing an underflow.

Case 3 solution:

  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();
++    require(tvl > 0 , "tvl should be greater than zero");  
      uint denominator = kerosineDenominator.denominator();
      return numerator * 1e8 / denominator;
  }

Assessed type

DoS

[c4-pre-sort]

JustDravee marked the issue as duplicate of #958

[c4-pre-sort]

JustDravee marked the issue as sufficient quality report

[c4-judge]

koolexcrypto marked the issue as duplicate of #308

[c4-judge]

koolexcrypto marked the issue as satisfactory

[c4-judge]

koolexcrypto changed the severity to 3 (High Risk)

[0xAbhay]

@koolexcrypto i think this finding has more detail than #308

here is the explanation

Finding #308 describes a specific scenario related to the assetPrice() calculation in the Unbounded Kerosene vault, where the numerator calculation (tvl - dyad.totalSupply()) can lead to assets getting temporarily stuck in the Kerosene vaults until the TVL surpasses the Dyad's total supply. However, it focuses only on this particular situation without exploring other potential issues.

On the other hand, Finding #873 elaborates on the vulnerability in the assetPrice() function of the Vault.kerosine.unbounded contract in more detail. It outlines three distinct cases where this vulnerability can cause problems: when the TVL is less than, greater than, or equal to the Dyad's total supply. Each case describes a scenario where the underflow in the numerator calculation (tvl - dyad.totalSupply()) can lead to a denial of service situation, potentially causing financial loss and loss of confidence in the system.

In summary, Finding 308 provides a single scenario, while Finding 873 expands on the issue by presenting three distinct cases where the vulnerability can manifest due to the numerator calculation.

[koolexcrypto]

Thank you for your feedback.

Case 1 and 2 are the same. The root cause is the underflow, the attacker in case 2 used the same bug in case 1.

While I appreciate your thoroughness in this report, I believe #308 is much easier to understand and comprehend.