V3Vault::deposit(), V3Vault::withdraw(), V3Vault::mint() and V3Vault::redeem() do not check for the maximums values

[c4-bot-4]

Lines of code

https://github.com/code-423n4/2024-03-revert-lend/blob/435b054f9ad2404173f36f0f74a5096c894b12b7/src/V3Vault.sol#L360-L363 https://github.com/code-423n4/2024-03-revert-lend/blob/435b054f9ad2404173f36f0f74a5096c894b12b7/src/V3Vault.sol#L366-L369 https://github.com/code-423n4/2024-03-revert-lend/blob/435b054f9ad2404173f36f0f74a5096c894b12b7/src/V3Vault.sol#L372-L375 https://github.com/code-423n4/2024-03-revert-lend/blob/435b054f9ad2404173f36f0f74a5096c894b12b7/src/V3Vault.sol#L378-L381

Vulnerability details

The V3Vault::withdraw function withdraws the assets, parameter for the amount of asset, by burning shares in the V3Vault::_withdraw function subsequently called.

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

The V3Vault::deposit function deposits the assets, parameter for the amount of asset, by minting shares in the V3Vault::_deposit function subsequently called.

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

The V3Vault::redeem function redeems a specific amount of shares, by burning them and withdrawing in the corresponding amount of assets in V3Vault::_withdraw function subsequently called.

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

The V3Vault::mint function mints the assets, parameter for the amount of asset, by minting shares in the V3Vault::_deposit function subsequently called.

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

It is theoretically possible for the deposit amount to mint shares more than the maximum mint-able amount, or for the withdraw amount to burn shares more than the maximum withdraw-able amount. It is also theoretically possible for the mint amount to deposit assets more than the maximum deposit-able amount, or for the redeem amount to withdraw assets more than the maximum withdraw-able amount.

However, there is neither protection against burning or minting too many user's shares for their specific assets, not protection against depositing or withdrawing too many user's assets for their specific amount of shares. Then the maxDeposit, maxMint, maxWithdraw and maxRedeem functions of V3Vault should be used to protect user's shares.

Proof of Concept

There is no check to validate if the minted/burned shares would exceed the maximum amount of shares that could've been minted/burned by V3Vault, nor check to validate if the deposited/withdrawn amount of assets would exceed the maximum amount of assets that could've been deposited/withdrawn.

However, the expression amount.mulDiv(Q96, exchangeRateX96, rounding) in the V3Vault::_convertToShares function could return a value more than the maximum mint-able or the maximum redeem-able amount.
As well, the expression shares.mulDiv(exchangeRateX96, Q96, rounding) in the V3Vault::_convertToAssets function could return a value more than the maximum deposit-able amount or the maximum withdraw-able amount.

This is possible if the vault get exploited or is severely under-collateralized.

User call V3Vault::withdraw() with assets = 1200. Normally, the burned shares should be 1000. However, there's an exploit in the vault which makes the shares burned to = 100_000, the entire user's shares.

Impact

Users can lose their entire shares when withdrawing and obtain more shares with depositing due to lack of amount burned check. Users can obtain more assets when redeeming due to lack of check on amount of withdrawn assets.

Tools Used

Manual review

Recommended Mitigation Steps

Include the maxMintable, maxDepositable, maxWithdrawable and maxRedeemable check in the V3Vault deposit, withdraw, redeem and mint corresponding functions.

Below an example :

-  function withdraw(uint256 assets, address receiver, address owner) external override returns (uint256) {
+  function withdraw(
+      uint256 assets, 
+      address receiver, 
+      address owner,
+      uint256 maxRedeem, 
+      uint256 maxWithdraw 
+  ) external override returns (uint256) {
+       if (assets > maxWithdraw) {
+           revert TooMuchAmount();
+       }
        (, uint256 shares) = _withdraw(receiver, owner, assets, false);
+      if (shares > maxRedeem) {
+       revert TooMuchAmount();
+      }
        return shares;
    }

Assessed type

Invalid Validation

[c4-pre-sort]

0xEVom marked the issue as duplicate of #281

[c4-pre-sort]

0xEVom marked the issue as sufficient quality report

[c4-judge]

jhsagd76 changed the severity to QA (Quality Assurance)

[c4-judge]

This previously downgraded issue has been upgraded by jhsagd76

[c4-judge]

jhsagd76 changed the severity to QA (Quality Assurance)