Revert Lend - lanrebayode77's results

Lines of code

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

Vulnerability details

Impact

The collateralValueLimitFactorX32 sets the maximum exposure limit for each token, expressed as a fraction of the lending pool, as stated in the docs.

To ensure this invariant holds in all actions, _updateAndCheckCollateral is called in onERC721Received(during transformation), borrow, repay, and _cleanupLoan. This is done to increase or decrease tokenConfigs[token0].totalDebtShares depending on the action and to ensure that the collateral token threshold is not exceeded.

During repay tokenConfigs[token0].totalDebtShares increases and during borrow, it increases and check is done.

However, the _updateAndCheckCollateral is not called during withdrawal, this makes it possible to borrow beyond the threshold of a collateral token by first depositing a large amount before borrowing, then withdrawing deposited funds after borrowing.

Proof of Concept

1. tokenA collateralValueLimitFactorX32 was set to 0.5 * Q32;
2. Total Lent amount == 1_000_000;
3. Based on the total lent amount, the max amount that should be borrowed is 500_000;
4. Three different borrowers borrow 100_000 each, tokenConfigs[tokenA].totalDebtShares == 300_000
5. Based on the restriction, only 200_000 is left to be borrowed with tokenA collateral.
6. Alice wants to borrow 500_000, and first deposited 600_000 asset, this increases totalLent to 1_600_000, which increases threshold to 800_000;
7. Alice then borrows 500_000, and check passes since tokenConfigs[tokenA].totalDebtShares == 800_000 and does not exceed threshold.

 if (
                    collateralValueLimitFactorX32 < type(uint32).max
                        && _convertToAssets(tokenConfigs[token0].totalDebtShares, debtExchangeRateX96, Math.Rounding.Up)
                            > lentAssets * collateralValueLimitFactorX32 / Q32
                ) {
                    revert CollateralValueLimit(); //@audit exposure check not eeffective, both should be added toggether,
                }
                collateralValueLimitFactorX32 = tokenConfigs[token1].collateralValueLimitFactorX32;
                if (
                    collateralValueLimitFactorX32 < type(uint32).max
                        && _convertToAssets(tokenConfigs[token1].totalDebtShares, debtExchangeRateX96, Math.Rounding.Up)
                            > lentAssets * collateralValueLimitFactorX32 / Q32
                ) {
                    revert CollateralValueLimit();
                }

7. Alice then withdraws her deposited asset 600_000, an action that passes as well.
8. tokenConfigs[tokenA].totalDebtShares == 800_000 and threshold is now 500_000.

Tools Used

Manual review

Recommended Mitigation Steps

Expressing is relative to total borrowed will mitigate issue, since user withdrawal cannot be stopped due to threshold on borrowers.

Assessed type

Invalid Validation

Lines of code

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

Vulnerability details

Impact

The V3Vault.transform() allows users through a verified contract to transform a loan and check the health factor at the end of the function.

When V3Vault.transform() is called, transformedTokenId is set to tokenId of the loan to be transformed, and the function performs low-level calls in the data input.

  (bool success,) = transformer.call(data);
        if (!success) {
            revert TransformFailed();
        }

This call could be to any function in the vault except for ones where calls from the transformer were explicitly denied with a check(liquidate and decreaseLiquidityAndCollect ).

  function decreaseLiquidityAndCollect(DecreaseLiquidityAndCollectParams calldata params)
        external
        override
        returns (uint256 amount0, uint256 amount1)
    {
        // this method is not allowed during transform - can be called directly on nftmanager if needed from transform contract
        if (transformedTokenId > 0) {
            revert TransformNotAllowed();
        }

    function liquidate(LiquidateParams calldata params) external override returns (uint256 amount0, uint256 amount1) {
        // liquidation is not allowed during transformer mode
        if (transformedTokenId > 0) {
            revert TransformNotAllowed();
        }

The V3Vault.borrow(), calls are not rejected, and when borrowing in transform mode, loan health is not checked, this makes it possible to borrow undercollateralized loans in transform mode, since the check is only done at the end of the transform function.

Malicious users can exploit this to get free flash-loan. Couple with the fact that repay() does not have any restriction against transform mode, the loan can be repaid without any interest before control goes back to the transform function where loan health is checked.

Proof of Concept

A call through transform() to borrow() the largest possible amount considering available asset, GlobalDebtLimit, dailyDebtIncreaseLimitLeft and collateral threshold.

The user uses the asset in the same call and calls repay() with the borrowed amount to return the position to a healthy one in the same transaction.

function borrow(uint256 tokenId, uint256 assets) external override {
        bool isTransformMode =
            transformedTokenId > 0 && transformedTokenId == tokenId && transformerAllowList[msg.sender];

        (uint256 newDebtExchangeRateX96, uint256 newLendExchangeRateX96) = _updateGlobalInterest();

        _resetDailyDebtIncreaseLimit(newLendExchangeRateX96, false);
...
...
        // only does check health here if not in transform mode
        if (!isTransformMode) {
            _requireLoanIsHealthy(tokenId, debt);
        }

 function _repay(uint256 tokenId, uint256 amount, bool isShare, bytes memory permitData) internal { //@audit  repay in transform
        (uint256 newDebtExchangeRateX96, uint256 newLendExchangeRateX96) = _updateGlobalInterest();

Since all the transactions are happening in the same block, the loan is repaid with the same newDebtExchangeRateX96 that the loan was taken, so there is zero interest payment. All that is needed is for a user to have a loan position, even with the minimum acceptable amount.

    function _updateGlobalInterest()
        internal
        returns (uint256 newDebtExchangeRateX96, uint256 newLendExchangeRateX96)
    {
  @>>   // only needs to be updated once per block (when needed)
        if (block.timestamp > lastExchangeRateUpdate) {
            (newDebtExchangeRateX96, newLendExchangeRateX96) = _calculateGlobalInterest();
            lastDebtExchangeRateX96 = newDebtExchangeRateX96;
            lastLendExchangeRateX96 = newLendExchangeRateX96;
            lastExchangeRateUpdate = block.timestamp;
            emit ExchangeRateUpdate(newDebtExchangeRateX96, newLendExchangeRateX96);
        } else {
    @>>     newDebtExchangeRateX96 = lastDebtExchangeRateX96;
    @>>     newLendExchangeRateX96 = lastLendExchangeRateX96;
        }
    }

Tools Used

Manual review

Recommended Mitigation Steps

Check if in transform mode in repay() and charge a specific interest fee, since newDebtExchangeRateX96 cannot be updated twice in a single block.

Assessed type

Invalid Validation

Lines of code

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

Vulnerability details

Impact

When a user loan position becomes unhealthy, it becomes eligible for liquidation.

Due to a check, liquidate() can be denied by a malicious borrower by calling repay() with dust amount of asset to change debtShares.

    function liquidate(LiquidateParams calldata params) external override returns (uint256 amount0, uint256 amount1) {
        // liquidation is not allowed during transformer mode
        if (transformedTokenId > 0) {
            revert TransformNotAllowed();
        }

        LiquidateState memory state;

        (state.newDebtExchangeRateX96, state.newLendExchangeRateX96) = _updateGlobalInterest();

        uint256 debtShares = loans[params.tokenId].debtShares;
  @>>  if (debtShares != params.debtShares) { //@audit frontrunning to repay dust!!!!!!
            revert DebtChanged();
        }

Proof of Concept

1. Alice laon becomes unhealthy when collateral value drops
2. Bob calls liquidate() to liquidate Alice position.
3. Alice frontruns Bob transaction with 1 wei repayment.
4. Since there is no minimum repayment, debtshares is modified and reduced, the only check is to ensure resulting loan is not too small

  // if resulting loan is too small - revert
            if (_convertToAssets(loanDebtShares, newDebtExchangeRateX96, Math.Rounding.Up) < minLoanSize) {
                revert MinLoanSize();
            }

5. Also, no check in the repay() to ensure the resulting loan is healthy, so the position remains unhealthy and liquidators cannot liquidate, driving the vault to a state of insolvency.
6. Liquidator transaction fails due to debtShares difference.

Tools Used

Manual review.

Recommended Mitigation Steps

There are three options here:

1. Remove the check.
2. Have a minimum repayment amount(could be a fraction of the entire loan).
3. Ensure that the resulting loan after repayment is healthy with _requireLoanIsHealthy(tokenId, debt);

Assessed type

DoS

Lines of code

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

Vulnerability details

Impact

On days with a significant number of liquidated positions, particularly when the asset quantity is substantial, there will be an excess of assets available in the vault that cannot be borrowed, thereby causing a drastic decrease in the utilization rate.

This also contradicts what was stated in the repay() function, which asserts that repaid amounts should be borrowed again. Liquidation is also a form of repayment.

 // when amounts are repayed - they maybe borrowed again
        dailyDebtIncreaseLimitLeft += assets; 

Proof of Concept

dailyDebyIncreaseLimitLeft was not increamented in liquidate(). here

Tools Used

Manual review.

Recommended Mitigation Steps

Include dailyDebyIncreaseLimitLeft increment in liquidate().

dailyDebtIncreaseLimitLeft += state.liquidatorCost;

Assessed type

Context

Lines of code

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

Vulnerability details

Impact

To determine the health status of a loan, the collateral position is evaluated using Chainlink/Uniswap V3 Twap. The protocol tries to prevent price manipulation by providing a counter-check on the valuation and ensuring that the difference does not exceed a certain threshold.

However, the threshold difference can be surpassed without being flagged due to an error in the check.

Proof of Concept

    function _requireMaxDifference(uint256 priceX96, uint256 verifyPriceX96, uint256 maxDifferenceX10000)
        internal
        pure
    {
        uint256 differenceX10000 = priceX96 > verifyPriceX96
            ? (priceX96 - verifyPriceX96) * 10000 / priceX96
     @>>    : (verifyPriceX96 - priceX96) * 10000 / verifyPriceX96;  
        // if too big difference - revert
        if (differenceX10000 >= maxDifferenceX10000) {
            revert PriceDifferenceExceeded();
        }
    }

The priceX96 is obtained by calling _getReferencePoolPriceX96(pool, 0); which returns slot zero price, while the derivedPoolPriceX96 is the price to be verified.

The problem is when priceX96 < verifyPriceX96, the difference in % is obtained by dividing the difference by verifyPriceX96 which will return a lower value Instance:

1. priceX96 == 4000;
2. verifyPriceX96 == 4200;
3. maxDifferenceX10000 == 500 (5%); According to: (verifyPriceX96 - priceX96) * 10000 / verifyPriceX96;
4. differenceX10000 = (4200 - 4000)*10_000 / 4200 == 476; When compared, the value passes the check, and in the real sense that price difference compared to what was obtained from the slot zero of uniswap v3 pool priceX96 is greater than differenceX1000 differenceX10000 = (4200 - 4000)*10_000 / 4000 == 500;

Note: this is in both directions. If the price to be verified is lower than expected, the differences might not be caught since it is divided by the larger price.

Tools Used

Manual review.

Recommended Mitigation Steps

To mitigate this in both directions, the difference should be divided by the smaller value, to catch max difference in %.

This is to prevent a manipulation that tries to reduce the price to liquidate a position that is still healthy.

    function _requireMaxDifference(uint256 priceX96, uint256 verifyPriceX96, uint256 maxDifferenceX10000)
        internal
        pure
    {
        uint256 differenceX10000 = priceX96 > verifyPriceX96
        --  ? (priceX96 - verifyPriceX96) * 10000 / priceX96
        ++  ? (priceX96 - verifyPriceX96) * 10000 / verifyPriceX96
        --  : (verifyPriceX96 - priceX96) * 10000 / verifyPriceX96;
        ++  : (verifyPriceX96 - priceX96) * 10000 /  priceX96;  
        // if too big difference - revert
        if (differenceX10000 >= maxDifferenceX10000) {
            revert PriceDifferenceExceeded();
        }
    }

Assessed type

Math

Lines of code

https://github.com/code-423n4/2024-03-revert-lend/blob/435b054f9ad2404173f36f0f74a5096c894b12b7/src/V3Vault.sol#L718-L724 https://github.com/code-423n4/2024-03-revert-lend/blob/435b054f9ad2404173f36f0f74a5096c894b12b7/src/V3Vault.sol#L894-L897 https://github.com/code-423n4/2024-03-revert-lend/blob/435b054f9ad2404173f36f0f74a5096c894b12b7/src/V3Vault.sol#L979-L982

Vulnerability details

Impact

Permit-related functions require the user to sign an approval message using the contract as the spender/approved so that safeTransferFrom can be called.

This feature comes with a vulnerability that enables a malicious observer to copy the signature on the chain and call permit directly on the token contract to grant vault approval. For more info

The problem is that when the user transaction now tries to call permit with the same details, it reverts due to nonce usage and the overall transaction fails.

Proof of Concept

if (permitData.length > 0) {
                (ISignatureTransfer.PermitTransferFrom memory permit, bytes memory signature) =
                    abi.decode(permitData, (ISignatureTransfer.PermitTransferFrom, bytes));
                permit2.permitTransferFrom(
                    permit, ISignatureTransfer.SignatureTransferDetails(address(this), assets), msg.sender, signature
                );

If a malicious front-runs this transaction to call permit, it will cause the entire function to fail.

Tools Used

manual review and Trust article

Recommended Mitigation Steps

Wrap the permit in a try-catch block. Or check if approval is already >= to amount.

Assessed type

DoS