Lines of code

https://github.com/code-423n4/2024-02-wise-lending/blob/main/contracts/PowerFarms/PendlePowerFarm/PendlePowerManager.sol#L129 https://github.com/code-423n4/2024-02-wise-lending/blob/main/contracts/PowerFarms/PendlePowerFarm/PendlePowerManager.sol#L172 https://github.com/code-423n4/2024-02-wise-lending/blob/main/contracts/PowerFarms/PendlePowerFarm/PendlePowerFarmLeverageLogic.sol#L560-L573

Vulnerability details

Impact

PowerFarm positions that borrowed aWETH can't be liquidated.

Proof of Concept

PowerFarm positions can only be liquidated through the PendlePowerFarm.liquidatePartiallyFromToken() function. If a PowerFarm position is attempted to be liquidated directly in WiseLending by using the WiseLending.liquidatePartiallyFromTokens() function, the execution will revert because the call to the _checkPositionLocked() function, this check validates that locked functions can't be liquidated directly on WiseLending!

function liquidatePartiallyFromTokens(
    uint256 _nftId,
    uint256 _nftIdLiquidator,
    ...
)
    ...
{
    ...

    //@audit-ok => Allows to liquidate positions that are not locked for a farm
    //@audit-info => If the nftId IS locked for a farm, reverts
    _checkPositionLocked(
        _nftId,
        msg.sender
    );
}

function _checkPositionLocked(
    uint256 _nftId,
    address _caller
)
    internal
    view
{
    //@audit-info => If the caller is a powerFarm, returns, skipped the check of positionLocked
    //@audit-info => If the caller is anybody else, check if the position is locked!
    if (_byPassCase(_caller) == true) {
        return;
    }

    //@audit-info => If the nftId IS NOT locked for a farm, returns
    if (positionLocked[_nftId] == false) {
        return;
    }

    //@audit-info => If the nftId IS locked for a farm, reverts
    revert PositionLocked();
}

Knowing that PowerFarm positions can't be liquidated directly in WiseLending, let's explore the reason why PowerFarm positions that borrowed aWETH can't be liquidated.

When users enter a farm they have two options to select the token that will be borrowed to repay the flashloan to the balancer vault.

The first option is to borrow aWETH from WiseLending via the AaveHub.
The second option is to borrow WETH directly from WiseLending.

Users control what type of token they want to borrow (either aWETH or WETH), by setting as true or false the parameter _isAave when entering a farm.

If set to true, the borrowedToken will be aWETH.
If set to false, the borrowedToken will be WETH.

When a user enters a farm, the function executes the next couple of steps:

The first step is that the PowerFarm mints a new position, locks it in WiseLending and approve the AaveHub contract to operate on the new-minted position.
- In this step, it is important to emphasize that the ID that is assigned to this new position is based on the totalReserved and totalSupply() of positions as per the PositionNFTs contract's state.
- The PositionNFTs contract is also used to mint positions for users who use the WiseLending contract as well as the AaveHub, these positions are not isolated/reserved to be used only by PowerFarms.
- This means that the ID that will be assigned for a new position when entering a farm will vary depending on the rest of the existing positions on the PositionNFTs contract.
```
function _getWiseLendingNFT()
internal
returns (uint256)
{
//@audit-info => When there are no available nfts!
if (availableNFTCount == 0) {
//@audit-info => Mints a new position. The owner of the position will be the PowerFarm!
uint256 nftId = POSITION_NFT.mintPosition();
...
return nftId;
}
```
... }
The second step is to transfer the amount that the user will use to enter the farm.
The third step is open a position, it will request a flashloan from the Balancer_Vault for the total leveraged amount that the position will use, it will use the flashloaned tokens to deposit and add liquidity into a Pendle Market, then it will deposit the received LPTokens from Pendle onto the PENDLE_CHILD. After depositing into the PENDLE_CHILD it will deposit the PENDLE_CHILD_Tokens into WiseLending, this deposit will be used as collateral for a borrow that the position will take to get the required tokens to payback the liquidator
- The value of the _isAave parameter plays a key role in this step:
- if it is set to true, the borrowedTokens will be aWETH
- Borrowing aWETH means that the position will end up owning borrowShares on the aWETH pool.
- if it is set to false, the borrowedTokens will be WETH
- Borrowing WETH means that the position will end up owning borrowShares on the WETH pool.
```
function _borrowExactAmount(
bool _isAave,
uint256 _nftId,
uint256 _totalDebtBalancer
)
internal
{
if (_isAave == true) {
AAVE_HUB.borrowExactAmount(
    _nftId,
    WETH_ADDRESS,
    _totalDebtBalancer
);

return;
}
```
WISE_LENDING.borrowExactAmount( _nftId, WETH_ADDRESS, _totalDebtBalancer ); }
The 4th step is to check the debt ratio to validate that the position has a debt ratio under 100%
The 5th step is to repay the flashloan (including the fees)
The 6th step is a fundamental process on the logic of the powerfarms. In this step it is reserved the key that is assigned to the position. Or in other words, the WiseLendingNFT it is linked to a new key that is generated based on the totalMinted and totalReserved keys in the PowerFarm.
- Thanks to this step is possible to identify who owns which WiseLendingNFTs on a PowerFarm.
```
function _reserveKey(
address _userAddress,
uint256 _wiseLendingNFT
)
internal
returns (uint256)
{
...
```
//@audit-info => Gets the nextReserveKey! uint256 keyId = _getNextReserveKey();

//@audit-info => The keyId is assigned as the reservedKey of the user! reservedKeys[_userAddress] = keyId; //@audit-info => The wiseLendingNFTId is assigned as the associated position of the keyId! farmingKeys[keyId] = _wiseLendingNFT;

return keyId; }

function _getNextReserveKey() internal returns (uint256) { return totalMinted + _incrementReserved(); }


7. And finally, the last step, and actually, **the step where the bug exists is when flagging if the borrowedTokens were aWETH or WETH tokens.**
- This step is crucial because the state variable **`isAave` is used to determine if the borrow was taken using the AaveHub (aWETH) or directly from WiseLending (WETH)**, and, depending on the type of borrowed assets, the [`PendlePowerFarmMathLogic._checkDebtRatio() function`](https://github.com/code-423n4/2024-02-wise-lending/blob/main/contracts/PowerFarms/PendlePowerFarm/PendlePowerFarmMathLogic.sol#L389-L410) will determine if the position can be liquidated or not.

**The existing implementation uses the `keyId` to flag if the borrowedTokens were aWETH or WETH.** The problem with this approach is that **anywhere in the rest of the codebase the value that is used to read the information of the `isAave` state variable is the `nftId` instead of the `keyId`.**
- ***Revisiting step2 & step6, the `nftId` depends on the PositionNFTs contract's state, and the `keyId` depends solely on the PowerFarm's contract state.***

> 

Let's do an example of UserA entering a farm and borrowing the funds by using the AaveHub:
1. A new WiseLendingNFT is minted for this position, such a new nft is assigned the ID 10
2. The function executes all the logic to open a position and takes the borrow to repay the flashloan by using the AaveHub
3. The keyId for this position is generated and the position gets assigned the keyId 3.
4. The `isAave` uses the value of the `keyId` to flag that the borrow was made using AaveHub (aWETH tokens were borrowed)
- **Notice here how the keyId of value 3 is the one flagged as if it would have taken a borrow using the AaveHub, while the nftId of value 10 is left unassigned**

function enterFarm( //@audit-info => If true, the borrow will use the AaveHub and will borrow aWETH //@audit-info => If false, the borrow will borrow WETH directly from WiseLending bool _isAave, uint256 _amount, uint256 _leverage, uint256 _allowedSpread ) ... { //@audit-info => Step 2, mints a new position that will be used to take the borrow and deposit the purchased PENDLE_CHILD shares uint256 wiseLendingNFT = _getWiseLendingNFT();

//@audit-info => Transfers from the caller the amount of WETH that will be used to enter the farm!
_safeTransferFrom(
    WETH_ADDRESS,
    msg.sender,
    address(this),
    _amount
);

//@audit-info => Steps 3, 4 & 5
_openPosition(
    ...
);

//@audit-info => Step 6, generates the key that will be associated to the WiseLendingNFT.
uint256 keyId = _reserveKey(
    msg.sender,
    wiseLendingNFT
);

//@audit-issue => The `keyId` is used to flag that the borrow was made using the AaveHub.
//@audit-issue => Not using the `WiseLendingNFT`. In the rest of the codebase, whenever reading the `isAave` variable, the value that is used is the `positionNFT`.
isAave[keyId] = _isAave;

...

}


Now that the user has entered a farm by borrowing the leveraged funds using the AaveHub, let's see **why this type of position can't be liquidated.**
- PowerFarm positions can only be liquidated through the [`PendlePowerFarm.liquidatePartiallyFromToken() function`](https://github.com/code-423n4/2024-02-wise-lending/blob/main/contracts/PowerFarms/PendlePowerFarm/PendlePowerFarm.sol#L93-L110). When this function is executed, before doing anything it checks the debtRatio of the specified WiseLendingNFTId to be liquidated, the execution reverts if the check of the debt ratio returns `true`.
  - The [`PendlePowerFarmMathLogic._checkDebtRatio() function`](https://github.com/code-423n4/2024-02-wise-lending/blob/main/contracts/PowerFarms/PendlePowerFarm/PendlePowerFarmMathLogic.sol#L389-L410), **reads the amount of borrowShares the position owes on the aWETH pool if the `isAave` variable is flagged as true, else, it will read the amount of borrowShares the position owes on the WETH pool**
  - Here comes into play how the `isAave` variable was flagged when the user entered the farm. The `isAave` variable was set to true for the `keyId`, which was generated with a value of 3, but the WiseLendingNFTID with a value of 10 was not flagged as if it would have taken a borrow using the AaveHub. 
  - The checkDebtRatio reads the value of the flag for the WiseLendingNFT, it doesn't read the flag of the keyId. Or in other words, it will read like this `isAave[10]` (The WiseLendingNftId), instead of `isAave[3]` (The keyId).
    - Because `isAave` is reading using the ID of the WiseLendingNFT (10), the returned value won't be the real flag that was assigned to this position, the value would be returned as false (default value), unless the `keyId 10` also borrowed aWETH (because of how the `isAave` variable is flagged when entering a position!).
      - **Because the flag returns false, the function will read the number of borrowShares the position owes on the WETH pool, even though the loan was taken on the aWETH pool.**. **Because the borrow was made on the aWETH pool, the total amount of borrowShares owed on the WETH pool will be returned as 0**, ***which will cause the checkDebtRatio to return true, which will cause the liquidation execution to revert***

function liquidatePartiallyFromToken( //@audit-info => WiseLendingNftId been liquidated uint256 _nftId, ... ) ... { return _coreLiquidation( //@audit-info => WiseLendingNftId been liquidated _nftId, _nftIdLiquidator, _shareAmountToPay ); }

function _coreLiquidation( uint256 _nftId, ... ) ... { //@audit-info => Forwards the nftID of the position being liquidated! if (_checkDebtRatio(_nftId) == true) { revert DebtRatioTooLow(); } }

function _checkDebtRatio( //@audit-info => WiseLendingNftId been liquidated uint256 _nftId ) internal view returns (bool) { //@audit-info => Using the WiseLendingNFT (position's ID) will read if the loan was taken using the AaveHub (aWETH) or directly from WiseLending (WETH)

//@audit-issue => When the position was opened, the `isAave` used the `keyId` to flag that the loan was made using the AaveHub, but here is using the `nftId` to read the value of `isAave`. Using our example, `isAave[3]` (The keyId) was flagged as a position that took a aWETH loan, but `isAave[10]` (The nftId) is not flagged, so, it will return the default value, false!
  //@audit-issue => Because the flag is read as false, the function will read the borrowShares owed by the position on the WETH pool, instead of reading the borrowShares on the aWETH pool.
  //@audit-issue => The position has no loans on the WETH pool, it has all the loans on the aWETH, thus, the borrowShares will be 0.

uint256 borrowShares = isAave[_nftId]
    //@audit-info => Loads all the borrowShares owed by the position on the aWETH pool
    ? _getPositionBorrowSharesAave(
        _nftId
    )
    //@audit-info => Loads all the borrowShares owed by the position on the WETH pool
    : _getPositionBorrowShares(
        _nftId
);

//@audit-issue => Because the borrowShares were 0, it will simply return true, it doesn't check the actual debtRatio!
//@audit-issue => Even though the position still owes the borrow that was taken on the aWETH pool, the debt ratio will return true
if (borrowShares == 0) {
    return true;
}

return getTotalWeightedCollateralETH(_nftId)
    >= getPositionBorrowETH(_nftId);

}


> Extra Side Effects
1. Manually payback shares won't work because the pool where the repayment should be done will be set as the WETH pool, instead of the aWETH pool.
2. Manually withdrawing shares will erroneously compute the debtRatio, which could cause key owners to withdraw more shares than what they should be allowed to.

### Coded PoC
I coded a PoC to demonstrate that the `keyId` and the `positionNFT` can have different values and that a PowerFarm position is considered to be locked in WiseLending. This demonstrates that using the `keyId` to flag the `isAave` state variable will behave as described in the Proof of Concept section, and that the only way to liquidate PowerFarm position is by using the [`PendlePowerFarm.liquidatePartiallyFromToken() function`](https://github.com/code-423n4/2024-02-wise-lending/blob/main/contracts/PowerFarms/PendlePowerFarm/PendlePowerFarm.sol#L93-L110)
- Add the next test to the test file [`PendlePowerFarmControllerBase.t.sol`](https://github.com/code-423n4/2024-02-wise-lending/blob/main/contracts/PowerFarms/PendlePowerFarmController/PendlePowerFarmControllerBase.t.sol):

function testFarmShouldEnterPoC() public cheatSetup(true) {

uint256 keyID = powerFarmManagerInstance.enterFarm(
    false,
    1 ether,
    15 ether,
    entrySpread
);

uint256 wiseLendingNFT = powerFarmManagerInstance.farmingKeys(keyID);

console.log("wiseLendingNFT:" , wiseLendingNFT);
console.log("keyID:" , keyID);
console.log("=========");

assert(keyID != wiseLendingNFT);

//@audit-info => Validate the position is locked in WiseLending
//@audit => If the position is locked, it can't be liquidated through the `WiseLending.liquidatePartiallyFromTokens()`
//@audit => Locked functions can only be liquidated by the `WiseLending.coreLiquidationIsolationPools()`, which can only be called by a PowerFarm!
bool positionLocked = wiseLendingInstance.positionLocked(wiseLendingNFT);
console.log("Is position locked: ", positionLocked);

}


- Run the test with the below command:
> forge test --match-test testFarmShouldEnterPoC --fork-url https://eth.llamarpc.com -vvv

[PASS] testFarmShouldEnterPoC() (gas: 24257021) Logs: wiseLendingNFT: 7

keyID: 1

Is position locked: true


## Tools Used
Manual Audit

## Recommended Mitigation Steps
The recommendation to fully mitigate this problem is to standardize the type of value that is used to flag the `isAave` variable, either using only the `keyId` or the `WiseLendingNFT`.
Additionally, make sure to flag the `isAave` variable before calling the `_openPosition()`.

- The below code changes propose a mitigation by standardizing the usage of the `WiseLendingNFT` associated with the `keyId` to flag and read the `isAave` variable. Also, it could be possible to standardize by using the `keyId`, this would require updating all the places where the `isAave` variable is read to use the `keyId` instead of the `WisLendingNFT` associated with the `keyId`.

function enterFarm( ... ) ... { uint256 wiseLendingNFT = _getWiseLendingNFT();

...

//@audit => Use the value of the `WiseLendingNFT` to flag the `isAave` variable!
//@audit => In this way, `isAave` will be correctly flagged using the positionNFT instead of the keyId!

isAave[wiseLendingNFT] = _isAave;

_openPosition( ... );

uint256 keyId = _reserveKey( msg.sender, wiseLendingNFT );
isAave[keyId] = _isAave;

... }


Apply the same fix on the [`PendlePowerManager.enterFarmETH() function`](https://github.com/code-423n4/2024-02-wise-lending/blob/main/contracts/PowerFarms/PendlePowerFarm/PendlePowerManager.sol#L210-L252)

function enterFarmETH( ... ) ... {

uint256 wiseLendingNFT = _getWiseLendingNFT();

...

isAave[wiseLendingNFT] = _isAave;

_openPosition( ... );

uint256 keyId = _reserveKey( msg.sender, wiseLendingNFT );
isAave[keyId] = _isAave;

... }

Also, make sure to update the below line in the PendlePowerManager.exitFarm() function:

    function exitFarm(
        ...
    )
        ...
    {
        uint256 wiseLendingNFT = farmingKeys[
            _keyId
        ];

        ...

        _closingPosition(
-           isAave[_keyId],
            //@audit-info => In this way, all the places on the codebase that reads the `isAave` variable will correctly be using the WiseLendingNFT instead of the keyId to determine the type of borrowedTokens (aWETH or WETH)
+           isAave[wiseLendingNFT],
            wiseLendingNFT,
            _allowedSpread,
            _ethBack
        );

        ...
    }

Assessed type

Context

code-423n4 / 2024-02-wise-lending-findings

PowerFarm positions that borrowed aWETH to enter the farm can't be liquidated. #220