Revert Lend - shaka's results

Lines of code

https://github.com/code-423n4/2024-03-revert-lend/blob/457230945a49878eefdc1001796b10638c1e7584/src/V3Vault.sol#L44 https://github.com/code-423n4/2024-03-revert-lend/blob/457230945a49878eefdc1001796b10638c1e7584/src/V3Vault.sol#L1258-L1268 https://github.com/code-423n4/2024-03-revert-lend/blob/457230945a49878eefdc1001796b10638c1e7584/src/V3Vault.sol#L43 https://github.com/code-423n4/2024-03-revert-lend/blob/457230945a49878eefdc1001796b10638c1e7584/src/V3Vault.sol#L1246-L1256

Vulnerability details

The protocol imposes a daily limit on the amount deposited and borrowed of a 10% increase of the total lent assets. As described in the whitepaper, the daily limit "functions as an emergency backstop to protect in case of a catastrophic exploit, whether due to unforeseen errors in the protocol’s logic or the exploitation of counterparty risk in one of the collateral tokens."

The daily limit is updated in the _resetDailyLendIncreaseLimit and _resetDailyDebtIncreaseLimit functions, but is calculated incorrectly.

File: V3Vault.sol

1250        uint256 lendIncreaseLimit = _convertToAssets(totalSupply(), newLendExchangeRateX96, Math.Rounding.Up)
1251            * (Q32 + MAX_DAILY_LEND_INCREASE_X32) / Q32;

The daily limit is not set to 10% of the total lent assets, but to 110% of the total lent assets.

lent * (Q32 + uint32(Q32 / 10)) / Q32 = lent * (Q32 + 0.1 * Q32) / Q32 = lent * 1.1

Impact

The daily limit protection is not functioning as intended, as it allows deposits of 110% of the total lent assets and borrows of 100% of the total lent assets. This breaks the intended protection mechanism against catastrophic exploits.

Proof of Concept

<details> <summary>Borrow</summary>

Add the following code to the contract V3Vault.t.sol and run forge test --mt test_DailyBorrowIncreaseOver100PerCent.

function test_DailyBorrowIncreaseOver100PerCent() external {
    // Setup
    vault.setLimits(0, 100e6, 10e6, 100e6, 0.5e6);
    skip(1 days);
    vm.startPrank(WHALE_ACCOUNT);
    USDC.approve(address(vault), type(uint256).max);
    vault.deposit(8e6, WHALE_ACCOUNT);
    vm.stopPrank();

    // Add collateral
    vm.startPrank(TEST_NFT_ACCOUNT);
    NPM.approve(address(vault), TEST_NFT);
    vault.create(TEST_NFT, TEST_NFT_ACCOUNT);
    vm.stopPrank();

    // Total lent is 8 USDC
    (, uint256 lent,,,,,) = vault.vaultInfo();
    assertEq(lent, 8e6);

    // Daily debt increase should be max(0.5, 8 * 0.1) = 0.8 USDC,
    // but we borrow 8 USDC (all supply)
    vm.prank(TEST_NFT_ACCOUNT);
    vault.borrow(TEST_NFT, 8e6);
}

</details> <details> <summary>Deposit</summary>

Add the following code to the contract V3Vault.t.sol and run forge test --mt test_DailyLendIncreaseOver100PerCent.

function test_DailyLendIncreaseOver100PerCent() external {
    // Setup
    vault.setLimits(0, 100_000e6, 100_000e6, 1_000e6, 1_000e6);
    uint256 amount = 1_000e6;
    vm.prank(WHALE_ACCOUNT);
    USDC.approve(address(vault), type(uint256).max);

    // Deposit 1_000 USDC per day for 20 days
    for (uint256 i = 0; i < 20; i++) {
        vm.prank(WHALE_ACCOUNT);
        vault.deposit(amount, WHALE_ACCOUNT);
        skip(1 days);
    }

    // Total lent is 20_000 USDC
    (, uint256 lent,,,,,) = vault.vaultInfo();
    assertEq(lent, 20_000e6);

    // Daily lend increase should be max(1_000, 20_000 * 0.1) = 2_000 USDC,
    // but we deposit 21_000 USDC
    vm.prank(WHALE_ACCOUNT);
    vault.deposit(21_000e6, WHALE_ACCOUNT);
}

</details>

Tools Used

Manual inspection.

Recommended Mitigation Steps

File: V3Vault.sol

    function _resetDailyLendIncreaseLimit(uint256 newLendExchangeRateX96, bool force) internal {
        // daily lend limit reset handling
        uint256 time = block.timestamp / 1 days;
        if (force || time > dailyLendIncreaseLimitLastReset) {
            uint256 lendIncreaseLimit = _convertToAssets(totalSupply(), newLendExchangeRateX96, Math.Rounding.Up)
-               * (Q32 + MAX_DAILY_LEND_INCREASE_X32) / Q32;
+               * MAX_DAILY_LEND_INCREASE_X32 / Q32;
            dailyLendIncreaseLimitLeft =
                dailyLendIncreaseLimitMin > lendIncreaseLimit ? dailyLendIncreaseLimitMin : lendIncreaseLimit;
            dailyLendIncreaseLimitLastReset = time;
        }
    }

    function _resetDailyDebtIncreaseLimit(uint256 newLendExchangeRateX96, bool force) internal {
        // daily debt limit reset handling
        uint256 time = block.timestamp / 1 days;
        if (force || time > dailyDebtIncreaseLimitLastReset) {
            uint256 debtIncreaseLimit = _convertToAssets(totalSupply(), newLendExchangeRateX96, Math.Rounding.Up)
-               * (Q32 + MAX_DAILY_DEBT_INCREASE_X32) / Q32;
+               * MAX_DAILY_DEBT_INCREASE_X32 / Q32;
            dailyDebtIncreaseLimitLeft =
                dailyDebtIncreaseLimitMin > debtIncreaseLimit ? dailyDebtIncreaseLimitMin : debtIncreaseLimit;
            dailyDebtIncreaseLimitLastReset = time;
        }
    }

Assessed type

Error

Lines of code

https://github.com/code-423n4/2024-03-revert-lend/blob/435b054f9ad2404173f36f0f74a5096c894b12b7/src/V3Vault.sol#L592 https://github.com/code-423n4/2024-03-revert-lend/blob/435b054f9ad2404173f36f0f74a5096c894b12b7/src/V3Vault.sol#L703 https://github.com/code-423n4/2024-03-revert-lend/blob/435b054f9ad2404173f36f0f74a5096c894b12b7/src/V3Vault.sol#L1270-L1279

Vulnerability details

When a loan is created by calling the borrow function in V3Vault the healthy check only requires that the collateral value is greater than the debt value. This means that a loan can be liquidated in the same block that it was created.

Given that the debt value is constantly increasing with the update of newDebtExchangeRateX96, if a loan is created using the maximum loan to value ratio, it can be liquidated just in the next block after its creation.

Impact

Loans can be liquidated in the block just after their creation.

Proof of Concept

Add the following code to the contract V3Vault.t.sol and run forge test --mt test_LiquidateImmediately.

function test_LiquidateImmediately() external {        
    _setupBasicLoan(true);
    (uint256 debtShares) = vault.loans(TEST_NFT);

    skip(12);
    vm.roll(block.number + 1);

    vm.startPrank(WHALE_ACCOUNT);
    USDC.approve(address(vault), type(uint256).max);
    vault.liquidate(IVault.LiquidateParams(TEST_NFT, debtShares, 0, 0, WHALE_ACCOUNT, ""));
    vm.stopPrank();
}

Tools Used

Manual inspection.

Recommended Mitigation Steps

Require in the loan creation that the collateral value is higher than the debt value in a certain percentage to serve as a safety margin.

Assessed type

Invalid Validation

Lines of code

https://github.com/code-423n4/2024-03-revert-lend/blob/435b054f9ad2404173f36f0f74a5096c894b12b7/src/V3Vault.sol#L301-L309 https://github.com/code-423n4/2024-03-revert-lend/blob/435b054f9ad2404173f36f0f74a5096c894b12b7/src/V3Vault.sol#L312-L320 https://github.com/code-423n4/2024-03-revert-lend/blob/435b054f9ad2404173f36f0f74a5096c894b12b7/src/V3Vault.sol#L910-L911

Vulnerability details

The documentation states that V3Vault: Should comply with ERC/EIP4626, but the maxDeposit and maxMint functions do not comply with the EIP-4626 standard, which says the following for these functions:

MUST NOT be higher than the actual maximum that would be accepted (it should underestimate if necessary).

MUST factor in both global and user-specific limits.

However, the current implementations do not take into account the daily lend increase limit.

Impact

Integrations that use the values returned by maxDeposit and maxMint functions to call the deposit and mint functions will revert on their execution.

Tools Used

Manual inspection.

Recommended Mitigation Steps

Account for the daily lend increase limit in the calculations of maxDeposit and maxMint functions.

Assessed type

Other

Lines of code

https://github.com/code-423n4/2024-03-revert-lend/blob/435b054f9ad2404173f36f0f74a5096c894b12b7/src/V3Vault.sol#L695-L698

Vulnerability details

In the liquidation process, the liquidator must send the value of the debt shares for the position to be liquidated and there is a check to ensure that this value matches the current debt of the position.

The owner of the position can front-run the liquidator by repaying the minimum amount of debt (1 share) and make the liquidation fail, effectively DoSing the liquidation process.

The amount of the repayment can be as low as 1 wei so the effective cost of preventing the liquidation is the gas cost of the transaction. Depending on the amount of debt, the owner of the position can be incentivized to prevent the liquidation by repaying the minimum amount. Note that the liquidator will also incur the gas cost of the failed liquidation so, if he is front-run multiple times, will be discouraged from continuing trying.

Another important remark is that the documentation states that "the protocol should be able to be deployed on any EVM compatible chain", so depending on the chain, the gas cost of the transaction can be very low, making the attack more feasible.

Impact

The owner of a position can prevent the liquidation of his position by repaying the minimum amount of debt, DoSing the liquidation process, and disincentivizing liquidators from trying to liquidate positions.

Tools Used

Manual inspection.

Recommended Mitigation Steps

Consider removing the check, or checking instead that the amount of debt passed by the liquidator is not lower than the current debt of the position.

Assessed type

DoS