`LeverageTransformer::leverageDown` will always revert on full repayment

[c4-bot-8]

Lines of code

https://github.com/code-423n4/2024-03-revert-lend/blob/main/src/transformers/LeverageTransformer.sol#L123-L175

Vulnerability details

Impact

Users can use the LeverageTransformer::leverageDown function to leverage down their positions directly in 1 TX. This can be done by calling V3Vault::transform, while passing the needed parameters, this function checks if the NFT is still owned by the vault at the end of the transform process, if not it reverts, by the following condition:

// 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();
}

However, in case of full repayment, the NFT will be transferred to the original owner, which is the expected behavior but the whole TX reverts because of the above. Blocking users form using that transform functionality.

Proof of Concept

For the sake of simplicity, instead of borrowing a loan and fully repaying it, the test attached will show the leverage-down mechanism on a position whose pair is different from the vault's asset (but both tokens are accepted as collateral, i.e. collateral factor > 0). This has the outcome of full repayment, as the debt is 0.

Add support for USDT:

IERC20 constant USDT = IERC20(0xdAC17F958D2ee523a2206206994597C13D831ec7);
address constant CHAINLINK_USDT_USD = 0x3E7d1eAB13ad0104d2750B8863b489D65364e32D;
oracle.setTokenConfig(
    address(USDT),
    AggregatorV3Interface(CHAINLINK_USDT_USD),
    3600 * 24 * 30,
    IUniswapV3Pool(0x3416cF6C708Da44DB2624D63ea0AAef7113527C6),
    60,
    V3Oracle.Mode.CHAINLINK_TWAP_VERIFY,
    50000
);
vault.setTokenConfig(address(USDT), uint32(Q32 * 9 / 10), type(uint32).max);

Test:

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

    vault.setLimits(1e6, 150e6, 150e6, 150e6, 150e6);

    vm.startPrank(WHALE_ACCOUNT);
    USDC.approve(address(vault), type(uint256).max);
    vault.deposit(100e6, WHALE_ACCOUNT);
    vm.stopPrank();

    address USER_1 = 0xD12342aFebE3e095E1c62B7eeC4F682F30D75303;
    uint256 USER_1_NFT = 690580;

    vm.startPrank(USER_1);
    NPM.approve(address(vault), USER_1_NFT);
    vault.create(USER_1_NFT, USER_1);
    vm.stopPrank();

    (,, address token0, address token1,,,, uint128 liquidity,,,,) = NPM.positions(USER_1_NFT);

    assertEq(token0, address(WETH));
    assertEq(token1, address(USDT));
    assertEq(vault.asset(), address(USDC));

    vm.startPrank(USER_1);
    vm.expectRevert(IErrors.Unauthorized.selector);
    vault.transform(
        USER_1_NFT,
        address(transformer),
        abi.encodeWithSelector(
            LeverageTransformer.leverageDown.selector,
            LeverageTransformer.LeverageDownParams(
                USER_1_NFT,
                liquidity,
                0,
                0,
                type(uint128).max,
                type(uint128).max,
                0,
                0,
                "",
                0,
                0,
                "",
                USER_1,
                block.timestamp
            )
        )
    );
}

Tools Used

Manual review

Recommended Mitigation Steps

Refactor V3Vault::transform to handle this specific case.

Assessed type

DoS

[c4-pre-sort]

0xEVom marked the issue as sufficient quality report

[c4-pre-sort]

0xEVom marked the issue as primary issue

[c4-pre-sort]

0xEVom marked the issue as insufficient quality report

[0xEVom]

V3Vault.transform() explicitly prevents the token from changing ownership and correctly provides functionality to transform a loan (and LeverageTransformer.leverageDown() to leverage down), not to repay it. Users should use V3Vault.repay() to repay in full.

QA

[c4-judge]

jhsagd76 changed the severity to QA (Quality Assurance)

[jhsagd76]

L-B