Settling a loan in full when deleveraging always reverts

[c4-bot-6]

Lines of code

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

Vulnerability details

Impact

A user will be denied the ability to call LeverageTransformer::leverageDown() through V3Vault::transform() if this results in the loan being repaid in full — breaking the protocol’s intended functionality.

Proof of Concept

LeverageTransformer::leverageDown() allows users to deleverage their position. It is intended to be called through V3Vault::transform(). The issue occurs when the amount passed to repay() in leverageDown() is equal to the loan’s debt, LeverageTransformer.sol#L166:

IVault(msg.sender).repay(params.tokenId, amount, false);

If a loan is fully repaid _cleanupLoan() is called, V3Vault.sol#L1003-L1005:

// if fully repayed
if (currentShares == shares) {
    _cleanupLoan(tokenId, newDebtExchangeRateX96, newLendExchangeRateX96, owner);

The NFT is sent back to the user in _cleanupLoan(), V3Vault.sol#L1083:

nonfungiblePositionManager.safeTransferFrom(address(this), owner, tokenId);

The transaction will then revert at this check in transform(), since the NFT was transferred back to the owner, V3Vault.sol#L530-L534:

// check owner not changed (NEEDED because token could have been moved somewhere else in the meantime)
address owner = nonfungiblePositionManager.ownerOf(tokenId);
if (owner != address(this)) {
    revert Unauthorized();
}

Add the following test that demonstrates this to test/integration/V3Vault.t.sol:

function test_LeverageDown() public {
    LeverageTransformer leverageTransformer = new LeverageTransformer(NPM, EX0x, UNIVERSAL_ROUTER);
    vault.setTransformer(address(leverageTransformer), true);

    _deposit(10000000, WHALE_ACCOUNT);

    vm.startPrank(TEST_NFT_ACCOUNT);
    NPM.approve(address(NPM), TEST_NFT);
    NPM.approve(address(vault), TEST_NFT);

    vault.create(TEST_NFT, TEST_NFT_ACCOUNT);

    vault.borrow(TEST_NFT, 1);

    LeverageTransformer.LeverageDownParams memory params = LeverageTransformer.LeverageDownParams({
        tokenId: TEST_NFT,
        liquidity: 1,
        amountRemoveMin0: 0,
        amountRemoveMin1: 0,
        feeAmount0: 0,
        feeAmount1: 1,
        amountIn0: 0,
        amountOut0Min: 0,
        swapData0: "",
        amountIn1: 0,
        amountOut1Min: 0,
        swapData1: "",
        recipient: TEST_NFT_ACCOUNT,
        deadline: block.timestamp
    });

    vm.expectRevert(IErrors.Unauthorized.selector);
    vault.transform(TEST_NFT, address(leverageTransformer), abi.encodeWithSelector(LeverageTransformer.leverageDown.selector, params));
    vm.stopPrank();
}

Tools Used

Foundry

Recommended Mitigation Steps

Adjust the accounting in LeverageTransformer::leverageDown() to handle this scenario, LeverageTransformer.sol#L165-L166:

+		(uint256 debt, , , , ) = IVault(msg.sender).loanInfo(params.tokenId);
+		if (amount >= debt) {
+		    amount = debt - 1;
+		}
		SafeERC20.safeApprove(IERC20(token), msg.sender, amount);
		IVault(msg.sender).repay(params.tokenId, amount, false);

In this case we call repay() passing amount as debt - 1, deleveraging the position as much as necessary without breaking the protocol’s intended functionality.

Assessed type

Error

[c4-pre-sort]

0xEVom marked the issue as duplicate of #172

[c4-pre-sort]

0xEVom marked the issue as insufficient quality report

[c4-judge]

jhsagd76 changed the severity to QA (Quality Assurance)

[c4-judge]

jhsagd76 marked the issue as grade-b