Kerosene collateral is not being moved on liquidation, exposing liquidators to loss

criticalCode4rena

Lines of code

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

Vulnerability details

Impact

When a position's collateral ratio drops below 150%, it is subject to liquidation. Upon 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 liquidated position's collateral. If the collateral ratio is <100%, all the position's collateral should be moved to the liquidator, this logic is done in VaultManagerV2::liquidate.

However, that function is only moving the non-Kerosene collateral to the liquidator, which is wrong. All collateral including Kerosene should be moved to the liquidator in the case of full liquidation. This will affect both the liquidated and liquidator positions:

• Liquidator position will be exposed to loss, as he'll pay some Dyad and won't get enough collateral in return.
• Liquidated position will end up with some collateral after being fully liquidated, where it should end up with 0 collateral of both types.

Proof of Concept

This assumes that a reported bug is fixed, which is using the correct licenser, to overcome this we had to manually change the licenser in addKerosene and getKeroseneValue.

Make sure to fork the main net and set the block number to 19703450

contract VaultManagerTest is VaultManagerTestHelper {
    Kerosine keroseneV2;
    Licenser vaultLicenserV2;
    VaultManagerV2 vaultManagerV2;
    Vault ethVaultV2;
    VaultWstEth wstEthV2;
    KerosineManager kerosineManagerV2;
    UnboundedKerosineVault unboundedKerosineVaultV2;
    BoundedKerosineVault boundedKerosineVaultV2;
    KerosineDenominator kerosineDenominatorV2;
    OracleMock wethOracleV2;

    address bob = makeAddr("bob");
    address alice = makeAddr("alice");

    ERC20 wrappedETH = ERC20(MAINNET_WETH);
    ERC20 wrappedSTETH = ERC20(MAINNET_WSTETH);
    DNft dNFT = DNft(MAINNET_DNFT);

    function setUpV2() public {
        (Contracts memory contracts, OracleMock newWethOracle) = new DeployV2().runTestDeploy();

        keroseneV2 = contracts.kerosene;
        vaultLicenserV2 = contracts.vaultLicenser;
        vaultManagerV2 = contracts.vaultManager;
        ethVaultV2 = contracts.ethVault;
        wstEthV2 = contracts.wstEth;
        kerosineManagerV2 = contracts.kerosineManager;
        unboundedKerosineVaultV2 = contracts.unboundedKerosineVault;
        boundedKerosineVaultV2 = contracts.boundedKerosineVault;
        kerosineDenominatorV2 = contracts.kerosineDenominator;
        wethOracleV2 = newWethOracle;

        vm.startPrank(MAINNET_OWNER);
        Licenser(MAINNET_VAULT_MANAGER_LICENSER).add(address(vaultManagerV2));
        boundedKerosineVaultV2.setUnboundedKerosineVault(unboundedKerosineVaultV2);
        vm.stopPrank();
    }

    function test_NonKeroseneNotMovedOnLiquidate() public {
        setUpV2();

        deal(MAINNET_WETH, bob, 100e18);
        deal(MAINNET_WSTETH, alice, 100e18);
        deal(MAINNET_WETH, address(ethVaultV2), 10_000e18);

        vm.prank(MAINNET_OWNER);
        keroseneV2.transfer(bob, 100e18);

        uint256 bobNFT = dNFT.mintNft{value: 1 ether}(bob);
        uint256 aliceNFT = dNFT.mintNft{value: 1 ether}(alice);

        // Bob adds Weth vault and Bounded Kerosene vault to his NFT
        // Bob deposits 1 Weth and 1 Kerosene
        // Bob mints 2,100 Dyad
        vm.startPrank(bob);
        wrappedETH.approve(address(vaultManagerV2), type(uint256).max);
        keroseneV2.approve(address(vaultManagerV2), type(uint256).max);

        vaultManagerV2.addKerosene(bobNFT, address(boundedKerosineVaultV2));
        vaultManagerV2.add(bobNFT, address(ethVaultV2));

        vaultManagerV2.deposit(bobNFT, address(boundedKerosineVaultV2), 1e18);
        vaultManagerV2.deposit(bobNFT, address(ethVaultV2), 1e18);

        vaultManagerV2.mintDyad(bobNFT, 2_100e18, bob);
        vm.stopPrank();

        // Alice adds WstEth vault and Weth vault to her NFT
        // Alice deposits 1.3 WstEth
        // Alice mints 3,000 Dyad
        vm.startPrank(alice);
        wrappedSTETH.approve(address(vaultManagerV2), type(uint256).max);

        vaultManagerV2.addKerosene(aliceNFT, address(boundedKerosineVaultV2));
        vaultManagerV2.add(aliceNFT, address(wstEthV2));
        vaultManagerV2.add(aliceNFT, address(ethVaultV2));

        vaultManagerV2.deposit(aliceNFT, address(wstEthV2), 1.3e18);

        vaultManagerV2.mintDyad(aliceNFT, 3_000e18, alice);
        vm.stopPrank();

        // Bob not liquidatable
        assertGt(vaultManagerV2.collatRatio(bobNFT), vaultManagerV2.MIN_COLLATERIZATION_RATIO());

        // Weth price drops down
        wethOracleV2.setPrice(wethOracleV2.price() / 2);

        // Bob liquidatable
        assertLt(vaultManagerV2.collatRatio(bobNFT), vaultManagerV2.MIN_COLLATERIZATION_RATIO());
        // Bob's position collateral ratio is less than 100% => All collateral should be moved
        assertLt(vaultManagerV2.collatRatio(bobNFT), 1e18);

        // Alice liquidates Bob's position
        vm.prank(alice);
        vaultManagerV2.liquidate(bobNFT, aliceNFT);

        // Bob loses all non-Kerosene collateral, but keeps Kerosene collateral
        assertEq(vaultManagerV2.getNonKeroseneValue(bobNFT), 0);
        assertGt(vaultManagerV2.getKeroseneValue(bobNFT), 0);
    }
}

Tools Used

Manual review

Recommended Mitigation Steps

Add the following to VaultManagerV2::liquidate:

uint256 numberOfKeroseneVaults = vaultsKerosene[id].length();
for (uint256 i = 0; i < numberOfKeroseneVaults; i++) {
    Vault vault = Vault(vaultsKerosene[id].at(i));
    uint256 collateral = vault.id2asset(id).mulWadUp(liquidationAssetShare);
    vault.move(id, to, collateral);
}

Assessed type

Error