Revert Lend - btk's results

Lines of code

https://github.com/code-423n4/2024-03-revert-lend/blob/main/src/V3Vault.sol#L323 https://github.com/code-423n4/2024-03-revert-lend/blob/main/src/V3Vault.sol#L329

Vulnerability details

Impact

The V3Vault deviate from the EIP-4626 specification which may break external integrations.

Proof of Concept

As specified in EIP-4626, both maxDeposit and maxMint must account for deposit limits:

MUST return the maximum amount of assets deposit would allow to be deposited for receiver and not cause a revert, which MUST NOT be higher than the actual maximum that would be accepted (it should underestimate if necessary).

MUST return the maximum amount of shares mint would allow to be deposited to receiver and not cause a revert, which MUST NOT be higher than the actual maximum that would be accepted (it should underestimate if necessary).

• However, neither maxDeposit nor maxMint check for dailyLendIncreaseLimitLeft and they only check for the globalLendLimit:

    function maxDeposit(address) external view override returns (uint256) {
        (, uint256 lendExchangeRateX96) = _calculateGlobalInterest();
        uint256 value = _convertToAssets(totalSupply(), lendExchangeRateX96, Math.Rounding.Up);
        if (value >= globalLendLimit) {
            return 0;
        } else {
            return globalLendLimit - value;
        }
    }

    function maxMint(address) external view override returns (uint256) {
        (, uint256 lendExchangeRateX96) = _calculateGlobalInterest();
        uint256 value = _convertToAssets(totalSupply(), lendExchangeRateX96, Math.Rounding.Up);
        if (value >= globalLendLimit) {
            return 0;
        } else {
            return _convertToShares(globalLendLimit - value, lendExchangeRateX96, Math.Rounding.Down);
        }
    }

• Thus, both maxDeposit & maxMint will return more than what the receiver will actually be able to deposit/mint.

According to EIP-4626 maxWithdraw and maxRedeem must not return a higher amount than the actual maximum that would be accepted:

MUST return the maximum amount of assets that could be transferred from owner through withdraw and not cause a revert, which MUST NOT be higher than the actual maximum that would be accepted (it should underestimate if necessary).

MUST return the maximum amount of shares that could be transferred from owner through redeem and not cause a revert, which MUST NOT be higher than the actual maximum that would be accepted (it should underestimate if necessary).

• However, maxWithdraw & maxRedeem omit to check for the available amount:

    function maxWithdraw(address owner) external view override returns (uint256) {
        (, uint256 lendExchangeRateX96) = _calculateGlobalInterest();
        return _convertToAssets(balanceOf(owner), lendExchangeRateX96, Math.Rounding.Down);
    }

    /// @inheritdoc IERC4626
    function maxRedeem(address owner) external view override returns (uint256) {
        return balanceOf(owner);
    }

• Therefore, both of them will return more than what the owner will actually be able to withdraw/redeem.

Here is a simple coded PoC, paste in V3VaultIntegrationTest :

    function testLogAvailable() public {
        _deposit(2000000, WHALE_ACCOUNT);
        _deposit(1000000, TEST_NFT_ACCOUNT_2);

        // gift some USDC so later he may repay all
        vm.prank(WHALE_ACCOUNT);
        USDC.transfer(TEST_NFT_ACCOUNT, 1000000);

        _createAndBorrow(TEST_NFT, TEST_NFT_ACCOUNT, 1000000);
        _createAndBorrow(TEST_NFT_2, TEST_NFT_ACCOUNT_2, 2000000);

        (,,, uint256 available,,,) = vault.vaultInfo();

        assertEq(available, 0);
        assertEq(vault.maxWithdraw(WHALE_ACCOUNT), 2000000);
    }

Tools Used

Manual review

Recommended Mitigation Steps

• V3Vault::maxDeposit and V3Vault::maxMint should be changed to check for the dailyLendIncreaseLimitLeft.
• V3Vault::maxWithdraw and V3Vault::maxRedeem should be changed to not return more then the available.

Assessed type

ERC4626

Lines of code

https://github.com/code-423n4/2024-03-revert-lend/blob/main/src/V3Vault.sol#L360 https://github.com/code-423n4/2024-03-revert-lend/blob/main/src/V3Vault.sol#L366 https://github.com/code-423n4/2024-03-revert-lend/blob/main/src/V3Vault.sol#L372 https://github.com/code-423n4/2024-03-revert-lend/blob/main/src/V3Vault.sol#L378

Vulnerability details

Overview

The V3Vualt lacks a mechanism for lenders to express their minimum acceptable output. This deficiency exposes them to potential losses of their assets due to exchange rates fluctuations.

Impact

Users will loss assets due to the absence of slippage control.

Proof of Concept

• The V3Vualt, uses exchange rates to determine how much shares/assets a lender should receive. Those exchange rates will go up/down depending on the users interaction with the vault.
• For instance, if a lender attempts to withdraw assets from the vault and their transaction is delayed due to low gas or network congestion, any loss in the vault would result in the trade occurring at a sub-optimal price, harming the user.
• Losses can occur in the vault due to bad debt, or liquidation.

Here are the main entry points for lenders, and they all have 0 slippage protection:

    function deposit(uint256 assets, address receiver) external override returns (uint256) {
        (, uint256 shares) = _deposit(receiver, assets, false, "");
        return shares;
    }

    /// @inheritdoc IERC4626
    function mint(uint256 shares, address receiver) external override returns (uint256) {
        (uint256 assets,) = _deposit(receiver, shares, true, "");
        return assets;
    }

    /// @inheritdoc IERC4626
    function withdraw(uint256 assets, address receiver, address owner) external override returns (uint256) {
        (, uint256 shares) = _withdraw(receiver, owner, assets, false);
        return shares;
    }

    /// @inheritdoc IERC4626
    function redeem(uint256 shares, address receiver, address owner) external override returns (uint256) {
        (uint256 assets,) = _withdraw(receiver, owner, shares, true);
        return assets;
    }

Tools Used

Manual review

Recommended Mitigation Steps

Introduce a router for lenders since adding a min minOut input would not be "standard".

Assessed type

Other