Revert Lend - 0xPhantom's results

Lines of code

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

Vulnerability details

Impact

A borrower can use the liquidity of the vault without paying any fees. A liquidator can use the liquidity of the protocol to make a liquidation. The functionality to do a liquidation with an Uniswap flashLoan is useless.

Proof of Concept

A borrower can use the liquidity of the vault without paying any fees. A liquidator can use the liquidity of the protocol to make a liquidation. The functionality to do a liquidation with an Uniswap flashLoan is useless.

When a user borrows the protocol increase his debt and he receive the amount that he borrowed. The problem is that a user can repay at any time, he can repay exactly the same amount that he borrowed if he repay in the same block, and basically give him a free flashloan. users could use this to do arbitrages or worse liquidation in the protocol itself. If a position is liquidable and the lend amount is bigger than the debt a liquidator could borrow pay the liquidation cost get the liquidation value repay exactly the same amount. He then performed a liquidation without taking any risk or paying any fees.

You can run this test in the VaultV3.t.sol

 function testFreeFlashloan() external {
    _setupBasicLoan(true);
    (, uint256 fullValue, uint256 collateralValue,,) = vault.loanInfo(TEST_NFT);
    vm.startPrank(TEST_NFT_ACCOUNT);
    USDC.approve(address(vault), collateralValue);
    vault.repay(TEST_NFT,collateralValue,false);
    vm.stopPrank();
    (uint256 debt, , ,,) = vault.loanInfo(TEST_NFT);
    assertEq(debt,0);
   }

Tools Used

Echidna

Recommended Mitigation Steps

The protocol should add a variable in the Loan struct, increment it in the borrow function(L567-L570) add require in the _repay function(L957-L960).

struct Loan {
        uint256 debtShares;
        uint256 timeOfBorrow;
    }

borrow function:

 uint256 loanDebtShares = loan.debtShares + shares;
        loan.debtShares = loanDebtShares;
        loan.timeOfBorrow = block.timestamp; 
        debtSharesTotal += shares;

in the _repay function:

  Loan storage loan = loans[tokenId];
  require(loan.timeOfBorrow> block.timestamp,"Could not repay in the same block");
        uint256 currentShares = loan.debtShares;

Assessed type

MEV

Lines of code

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

Vulnerability details

Impact

A user can get out of the protocol by another user. It could be a loss for the user specially if the user use the transform mode.

Proof of Concept

If a user want to borrow he must first create a position with the create function(L400) and after that he can borrow with the borrow function(L550).
But if in the meantime another user repay with 0 assets the internal function _cleanupLoan(L1082) will be called and it will delete the position, the user will be out of the protocol. It could be a problem especially if the user use the protocol to manage his position on Uniswap. You can run this test in V3Vault.t.sol :

function testUserCanGetAnotherUserOutOfTheProtocol() external {
    _setupBasicLoan(false);
    vault.repay(TEST_NFT,0,false);
    assertEq(NPM.ownerOf(TEST_NFT),TEST_NFT_ACCOUNT);
}

Tools Used

Echidna

Recommended Mitigation Steps

The problem is that the protocol allows repaying any amount. The best way to prevent that is a minimum repay amount like 10% of the debt, add a require in case the user have no debt and add another external function to _cleanupLoan if the user have no debt.

Assessed type

Other

Lines of code

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

Vulnerability details

Impact

The user will repay but his debt will not decrease.

Proof of Concept

Because of a rounding error in the _repay function (L954) a user may repay his loan and his debt don't decrease. It happen whenever the user repay an amount above the debt exchange rate. Because of this lines in the _repay function (L968-L969)

assets = amount;
shares = _convertToShares(amount, newDebtExchangeRateX96, Math.Rounding.Down);

Here we can see that the rounding is down so it will round down to zero. Moreover the protocol give absolutely no view function to convert assets in debt shares, so a user or a protocol build on top of this one can make a mistake a repay without having the debt decrease. You can run this test in V3Vault.t.sol :

function testBorrowerDontRepayAnything() external {
     _setupBasicLoan(true);
  (,,,,, uint256 _debtExchangeRateX96,uint256 _lendExchangeRateX96)= vault.vaultInfo();
  //We let two year pass in order to let the debt exchange rate increase.
   vm.warp(block.timestamp+ 2*60*60*24*365);
  ( uint256 debtBefore)= vault.loans(TEST_NFT);
   _repay(_debtExchangeRateX96/Q96,TEST_NFT_ACCOUNT,TEST_NFT,false); 
  ( uint256 debtAfter)= vault.loans(TEST_NFT);
     assertEq(debtAfter,debtBefore);
   }

Tools Used

Echidna

Recommended Mitigation Steps

The _repay function should have a require in case that the shares repayed round down to zero.

assets = amount;
shares = _convertToShares(amount, newDebtExchangeRateX96, Math.Rounding.Down);
require(shares>0);

Assessed type

Math