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

8 stars 6 forks source link

Collateral Verification Flaw in VaultManagerV2 Allowing Minting of DYAD with Kerosene Only #872

Closed c4-bot-5 closed 6 months ago

c4-bot-5 commented 7 months ago

Lines of code

https://github.com/code-423n4/2024-04-dyad/blob/cd48c684a58158de444b24854ffd8f07d046c31b/src/core/VaultManagerV2.sol#L156-L169 https://github.com/code-423n4/2024-04-dyad/blob/cd48c684a58158de444b24854ffd8f07d046c31b/src/core/Vault.kerosine.unbounded.sol#L50 https://github.com/code-423n4/2024-04-dyad/blob/cd48c684a58158de444b24854ffd8f07d046c31b/src/core/Vault.kerosine.sol#L60

Vulnerability details

Impact

The protocol's vulnerability allows for the minting of dyad tokens using only kerosene as collateral, which contravenes the intended design that prohibits using kerosene alone for this purpose. Additionally, this flaw enables users to bypass the critical 150% collateralization ratio requirement, threatening the protocol's financial stability. Exploitation of this bug could lead to the issuance of dyad tokens that are not sufficiently backed by collateral, potentially causing a devaluation of the dyad currency and jeopardizing the protocol's solvency.

Proof of Concept The [deploymentscript](https://github.com/code-423n4/2024-04-dyad/blob/cd48c684a58158de444b24854ffd8f07d046c31b/script/deploy/Deploy.V2.s.sol#L95) adds the `unboundedKerosineVault` to the `vaultLicenser`, which could be problematic because the intention is that users should not be able to mint dyad solely with `kerosene` as collateral a user adds the `unboundedKerosineVault` using `VaultManagerV2::add` and inside there is check ```jsx function add( uint id, address vault ) external isDNftOwner(id) { ///...... // @audit this check will passed for the unbounded vault because of the deployment script if (!vaultLicenser.isLicensed(vault)) revert VaultNotLicensed(); //... } ``` and the check is passed now the user deposits 1 million kerosene using the `VaultManagerV2::deposit` function. As of now when i am writing this report the cost of `1 KEROSENE = 0.03456 USDC` ($0.0317) 1m kerosene = 34560.3 usdc. also this can be huge if the user is taking a flash loan to exploit. User calls [mintDyad](https://github.com/code-423n4/2024-04-dyad/blob/cd48c684a58158de444b24854ffd8f07d046c31b/src/core/VaultManagerV2.sol#L156) with `36500` dyad as the amount and with their address now inside the `mintdyad` function it calls the `getNonKeroseneValue()` ```jsx function getNonKeroseneValue( uint id ) public view returns (uint) { uint totalUsdValue; uint numberOfVaults = vaults[id].length(); for (uint i = 0; i < numberOfVaults; i++) { Vault vault = Vault(vaults[id].at(i)); uint usdValue; if (vaultLicenser.isLicensed(address(vault))) { usdValue = vault.getUsdValue(id); } totalUsdValue += usdValue; } return totalUsdValue; } ``` since there is only one vault which is the `unboundedkerosine vault` so it will call the [getUsdValue](https://github.com/code-423n4/2024-04-dyad/blob/cd48c684a58158de444b24854ffd8f07d046c31b/src/core/Vault.kerosine.sol#L60-L67) inside this function it will call the `Vault.kerosine.unbounded::assetprice()`: ```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()); } uint numerator = tvl - dyad.totalSupply(); // 1000000000e18 - 950841953.47e18 = 49158046.53e18 uint denominator = kerosineDenominator.denominator(); return numerator * 1e8 / denominator; } } ``` [KerosineManager has only two vault, weth & wstETH](https://github.com/code-423n4/2024-04-dyad/blob/cd48c684a58158de444b24854ffd8f07d046c31b/script/deploy/Deploy.V2.s.sol#L64-L65). so we will only calculating the `assetprice()` of weth ans wstETH now lets take an example , suppose tvl of weth = 500 weth, tvl of wstETH = 500 stEth dyad totalsupply (as of the time of writing this report) = 632967400000000000000000 (in 18 decimal) return value of [assetPrice()](https://github.com/code-423n4/2024-04-dyad/blob/cd48c684a58158de444b24854ffd8f07d046c31b/src/core/Vault.sol#L91) for `Weth` = 318542608000 return value of [assetPrice()](https://github.com/code-423n4/2024-04-dyad/blob/cd48c684a58158de444b24854ffd8f07d046c31b/src/core/Vault.wsteth.sol#L92) for `wstETH` = 368088673998 now calculating the tvl using the [Tvl formula](https://github.com/code-423n4/2024-04-dyad/blob/cd48c684a58158de444b24854ffd8f07d046c31b/src/core/Vault.kerosine.unbounded.sol#L60-L64) $$ \text{tvlforWeth} = \frac{500 \times 10^{18} \times 318542608000 \times 10^{18}}{10^8 \times 10^{18}} = {1592713.04e18} $$ $$ \text{tvlforWstETH} = \frac{500 \times 10^{18} \times 368088673998 \times 10^{18}}{10^8 \times 10^{18}} = 1840443.36999e18 $$ $$ \text{totalTVL} = 1592713.04e18 + 1840443.36999e18 = 3433156.40999e18 $$ [numetor](https://github.com/code-423n4/2024-04-dyad/blob/cd48c684a58158de444b24854ffd8f07d046c31b/src/core/Vault.kerosine.unbounded.sol#L65) = `3433156409990000000000000 - 632967400000000000000000` = `2800189009990000000000000` [denominator](https://github.com/code-423n4/2024-04-dyad/blob/cd48c684a58158de444b24854ffd8f07d046c31b/src/core/Vault.kerosine.unbounded.sol#L66) = `1000000000e18 - 950841953.47e18 = 49158046.53e18` [return](https://github.com/code-423n4/2024-04-dyad/blob/cd48c684a58158de444b24854ffd8f07d046c31b/src/core/Vault.kerosine.unbounded.sol#L67) = `2800189009990000000000000 * 1e8 / 49158046.53e18` = `5696298` (return value of `assetPrice()` inside unbounded kerosine vault contract) now, calculating the [getUsdValue](https://github.com/code-423n4/2024-04-dyad/blob/cd48c684a58158de444b24854ffd8f07d046c31b/src/core/Vault.kerosine.sol#L66) for `Unbounded Vault`, $$ 1000000e18 * 5696298 / 1e18 = 56962.98e18. $$ since there is a only one vault added by the user the `getNonKeroseneValue` will only run for one `vault`,so the [totalUsdValue](https://github.com/code-423n4/2024-04-dyad/blob/cd48c684a58158de444b24854ffd8f07d046c31b/src/core/VaultManagerV2.sol#L266) = `56962.98e18`. now back to `mintdyad` the [check](https://github.com/code-423n4/2024-04-dyad/blob/cd48c684a58158de444b24854ffd8f07d046c31b/src/core/VaultManagerV2.sol#L165) will pass because the `newDyadminted` is 0 + 37000e18 (attacker is minting for the first time). and the `getNonKeroseneValue(id)` is `56962.98e18` now the user minted the `36500` dyad, ```jsx dyad.mint(id, to, amount); if (collatRatio(id) < MIN_COLLATERIZATION_RATIO) revert CrTooLow(); emit MintDyad(id, amount, to); ``` after minting dyad it will call, the [collatRatio(id)](https://github.com/code-423n4/2024-04-dyad/blob/cd48c684a58158de444b24854ffd8f07d046c31b/src/core/VaultManagerV2.sol#L230) inside this function it will return the [getTotalUsdValue](https://github.com/code-423n4/2024-04-dyad/blob/cd48c684a58158de444b24854ffd8f07d046c31b/src/core/VaultManagerV2.sol#L238) because the user `_dyad` is non zero it is `37000e18` now inside the `getTotalUsdValue` it return ```jsx function getTotalUsdValue( uint id ) public view returns (uint) { return getNonKeroseneValue(id) + getKeroseneValue(id); } ``` so, the `getnonkerosene` is `56962.98e18` and `getKeroseneValue` is `zero` becuase user didnt add any exogenous collateral. so the return value is `56962.98e18` returned to collatration for the [ratio calculation](https://github.com/code-423n4/2024-04-dyad/blob/cd48c684a58158de444b24854ffd8f07d046c31b/src/core/VaultManagerV2.sol#L238) = `56962.98e18.divwadDown(36500e18)` = `1.5606e18` (18 decimal) now the check in `mintDyad` :This check will pass, allowing the user to successfully mint dyad. Since the `collatRatio returns 1.5606` is greater than the `MIN_COLLATERIZATION_RATIO`, the check will pass, enabling the user to mint dyad using only kerosene tokens. ```jsx dyad.mint(id, to, amount); if (collatRatio(id) < MIN_COLLATERIZATION_RATIO) revert CrTooLow(); emit MintDyad(id, amount, to); ``` Due to the addition of the `unboundedKerosineVault` to the `vaultLicenser`, an attacker can mint dyad tokens using only `kerosene` as collateral. This attack is profitable as 1 million kerosene costs `$34,560.3`, and the user minted `36,500` dyad, with dyad being pegged 1:1 to stablecoins such as USDC, USDT, or DAI. Therefore, the profit from the attack would be `$36,500 - $34,560.3 = $1,939.7` in stablecoin. The profitability could be significantly amplified if the exploiter utilizes a flash loan to scale the attack.

Tools Used

Manual Review

Recommended Mitigation Steps

Modify the VaultManagerV2::add function to include additional checks that prevent unbounded vaults from being used as collateral sources for minting dyad.

Assessed type

Context

c4-pre-sort commented 7 months ago

JustDravee marked the issue as duplicate of #70

c4-pre-sort commented 7 months ago

JustDravee marked the issue as sufficient quality report

c4-judge commented 6 months ago

koolexcrypto marked the issue as satisfactory

c4-judge commented 6 months ago

koolexcrypto marked the issue as not a duplicate

c4-judge commented 6 months ago

koolexcrypto marked the issue as unsatisfactory: Invalid

0xAbhay commented 6 months ago

@koolexcrypto The identified vulnerability allows an attacker to exploit the minting mechanism of the protocol by bypassing the intended collateralization constraints. This is due to an error in the deployment script, which incorrectly licenses the unboundedKerosineVault, enabling users to use this vault as valid collateral despite it not being intended as such. An attacker can then add this vault, deposit kerosene, and mint dyad tokens solely with kerosene tokens. The flawed script allows the vault to pass the collateral check, leading to the successful minting of dyad tokens with insufficient collateral, undermining the financial stability of the protocol. This exploit can be further amplified using flash loans, significantly increasing the attacker's profit.

0xAbhay commented 6 months ago

@koolexcrypto The main invariant is that the user cannot mint dyad tokens using only kerosene. If they are trying to mint using solely kerosene, then there is a problem. #679 #1098 and #872 has similar issue please relook in to it

Thank you 🙏

koolexcrypto commented 6 months ago

Hi @0xAbhay

Thank you for your feedback.

The issue seems to be in the condition if (vaultLicenser.isLicensed(address(vault))) { in getNonKeroseneValue function, as it should ensure that only non-kerosene vaults only are used. Probably using keroseneManager.isLicensed instead.

Could you please provide a PoC (coded) that demonstrates the issue above?

shafu0x commented 6 months ago

Hi @0xAbhay

Thank you for your feedback.

The issue seems to be in the condition if (vaultLicenser.isLicensed(address(vault))) { in getNonKeroseneValue function, as it should ensure that only non-kerosene vaults only are used. Probably using keroseneManager.isLicensed instead.

Could you please provide a PoC (coded) that demonstrates the issue above?

exactly. this is the problem here.

0xAbhay commented 6 months ago

@koolexcrypto

Sure, let's break down the finding and the potential exploit step-by-step, focusing on how the minting of dyad tokens using only kerosene as collateral happens and why it is problematic for the protocol. We'll use the provided example to illustrate this process.

Step-by-Step Explanation

1. **Deployment Script Issue**: - The deployment script incorrectly adds the [unboundedKerosineVault](https://github.com/code-423n4/2024-04-dyad/blob/cd48c684a58158de444b24854ffd8f07d046c31b/script/deploy/Deploy.V2.s.sol#L95) to the [vaultLicenser](). This setup was not intended because the protocol design prohibits using kerosene alone as collateral for minting dyad tokens. 2. **Adding the Vault**: - A user adds the `unboundedKerosineVault` using the `VaultManagerV2::add` function. - Inside this function, a check ensures the vault is licensed: ```solidity if (!vaultLicenser.isLicensed(vault)) revert VaultNotLicensed(); ``` - Because of the deployment script error, this check passes for the `unboundedKerosineVault`. 3. **Depositing Kerosene**: - The user deposits 1 million kerosene tokens into the `unboundedKerosineVault` using the `VaultManagerV2::deposit` function. - The value of 1 million kerosene is calculated as: \[ 1,000,000 \times 0.03456 \text{ USDC} = 34,560.3 \text{ USDC} \] 4. **Minting Dyad Tokens**: - The user calls the `mintDyad` function to mint 36,500 dyad tokens to their address. - Inside the `mintDyad` function, the `getNonKeroseneValue()` function is called to calculate the collateral value excluding kerosene: ```solidity function getNonKeroseneValue(uint id) public view returns (uint) { uint totalUsdValue; uint numberOfVaults = vaults[id].length(); for (uint i = 0; i < numberOfVaults; i++) { Vault vault = Vault(vaults[id].at(i)); uint usdValue; if (vaultLicenser.isLicensed(address(vault))) { usdValue = vault.getUsdValue(id); } totalUsdValue += usdValue; } return totalUsdValue; } ``` 5. **Calculating the Asset Price**: - The `getUsdValue` function for the `unboundedKerosineVault` calculates the total value of the assets. This involves a detailed calculation using the `assetPrice()` function: ```solidity 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; } ``` 6. **Example Calculation**: Total Value Locked (TVL) in WETH and wstETH vaults is calculated: $$ \text{TVL for WETH} = \frac{500 \times 10^{18} \times 318,542,608,000 \times 10^{18}}{10^8 \times 10^{18}} = 1,592,713.04 \text{ USDC} $$ $$ \text{TVL for wstETH} = \frac{500 \times 10^{18} \times 368,088,673,998 \times 10^{18}}{10^8 \times 10^{18}} = 1,840,443.36999 \text{ USDC} $$ Total TVL: $$ \text{totalTVL} = 1,592,713.04 + 1,840,443.36999 = 3,433,156.40999 \text{ USDC} $$ Adjusted numerator and denominator for asset price calculation: $$ \text{numerator} = 3,433,156.40999 \text{ USDC} - 632,967.4 \text{ USDC} = 2,800,189.00999 \text{ USDC} $$ $$ \text{denominator} = 1,000,000,000 - 950,841,953.47 = 49,158,046.53 $$ Asset price calculation: $$ \text{assetPrice} = \frac{2,800,189.00999 \times 10^8}{49,158,046.53} = 56,962.98 $$ 7. **USD Value Calculation for Unbounded Vault**: - Using the calculated `assetPrice`: $$ \text{getUsdValue for Unbounded Vault} = 1,000,000 \times 56,962.98 / 1e18 = 56,962.98 \text{ USDC} $$ 8. **Collateral Ratio Check**: - The `mintDyad` function checks the collateralization ratio after minting: $$ \text{collatRatio} = \frac{56,962.98}{36,500} = 1.5606 > 1.50 \text{ (minimum ratio)} $$ - This check passes, allowing the user to mint the dyad tokens. ### Profit Calculation - The user effectively minted 36,500 dyad tokens, pegged 1:1 to stablecoins like USDC. - The cost of 1 million kerosene is $34,560.3. - The profit from the attack: $$ 36,500 \text{ USDC} - 34,560.3 \text{ USDC} = 1,939.7 \text{ USDC} $$ ### Exploit Amplification - The profitability can be significantly increased if the attacker uses a flash loan to deposit larger amounts of kerosene, thereby scaling the attack. ### Summary This vulnerability arises due to the improper licensing of the `unboundedKerosineVault`, allowing users to use kerosene alone as collateral. This bypasses the intended 150% collateralization ratio requirement, enabling the minting of undervalued dyad tokens and threatening the protocol's financial stability. This is not a duplicate #1029. and for coded poc -> please refers to #679
0xAbhay commented 6 months ago

@koolexcrypto Also, why would a kerosene vault be added using the add function, which is used to add exogenous collateral? This finding shows how, due to a development script error, we can add unbounded kerosene using the add function and mint the DYAD using only kerosene thank you for all the hard work you have put in i appreciate your work and #832 is a similar finding and also has a good poc please check.

c4-judge commented 6 months ago

koolexcrypto removed the grade

c4-judge commented 6 months ago

koolexcrypto marked the issue as duplicate of #1133

c4-judge commented 6 months ago

koolexcrypto marked the issue as satisfactory