No access control to check caller of leverage functions in LeverageTransformer contract is approved vault

[c4-bot-4]

Lines of code

https://github.com/code-423n4/2024-03-revert-lend/blob/435b054f9ad2404173f36f0f74a5096c894b12b7/src/transformers/LeverageTransformer.sol#L43 https://github.com/code-423n4/2024-03-revert-lend/blob/435b054f9ad2404173f36f0f74a5096c894b12b7/src/transformers/LeverageTransformer.sol#L124

Vulnerability details

Impact

The leverageUp and leverageDown functions in LeverageTransformer contract are expected to be called by V3Vault. However, there is no access control to check that the caller is an authorized V3Vault address.

Due to this, it is possible for any user owned contract to steal position tokens on tokenIds that are already pre-approved for isAuthorizedForToken calls in nonfungiblePositionManager contract. Additionally, the user-owned contract can steal tokens held in the LeverageTransformer using the routed swap calls in leverageUp and leverageDown functions

Proof of Concept

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

1. ContractA calls leverageDown() with influenced params and callback returned values to set the asset token and LeverageDownParams params.

   function leverageDown(LeverageDownParams calldata params) external {
       address token = IVault(msg.sender).asset();
       (,, address token0, address token1,,,,,,,,) = nonfungiblePositionManager.positions(params.tokenId);

2. Assume LeverageTransformer is already authorized to call nonfungiblePositionManager.decreaseLiquidity(), it receives the position tokens from the liquidity decrement and swaps

   
   INonfungiblePositionManager.DecreaseLiquidityParams memory decreaseLiquidityParams = INonfungiblePositionManager
           .DecreaseLiquidityParams(
           params.tokenId, params.liquidity, params.amountRemoveMin0, params.amountRemoveMin1, params.deadline
       );
       (amount0, amount1) = nonfungiblePositionManager.decreaseLiquidity(decreaseLiquidityParams);
   
       INonfungiblePositionManager.CollectParams memory collectParams = INonfungiblePositionManager.CollectParams(
           params.tokenId,
           address(this),
           params.feeAmount0 == type(uint128).max ? type(uint128).max : SafeCast.toUint128(amount0 + params.feeAmount0),
           params.feeAmount1 == type(uint128).max ? type(uint128).max : SafeCast.toUint128(amount1 + params.feeAmount1)
       );
       (amount0, amount1) = nonfungiblePositionManager.collect(collectParams);

3. ContractA is sent swapped tokens

SafeERC20.safeApprove(IERC20(token), msg.sender, amount); IVault(msg.sender).repay(params.tokenId, amount, false);

    // send leftover tokens
    if (amount0 > 0 && token != token0) {
        SafeERC20.safeTransfer(IERC20(token0), params.recipient, amount0);
    }
    if (amount1 > 0 && token != token1) {
        SafeERC20.safeTransfer(IERC20(token1), params.recipient, amount1);
    }

## Tools Used
Manual

## Recommended Mitigation Steps
Add a check to ensure caller is approved vault address

## Assessed type

Access Control

[c4-pre-sort]

0xEVom marked the issue as duplicate of #366

[c4-pre-sort]

0xEVom marked the issue as sufficient quality report

[c4-judge]

jhsagd76 changed the severity to QA (Quality Assurance)

[jhsagd76]

L-A

[c4-judge]

jhsagd76 marked the issue as grade-a

[kalinbas]

https://github.com/revert-finance/lend/pull/29