Liquidation with zero shares

[c4-bot-9]

Lines of code

https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/WiseLending.sol#L1250 https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/MainHelper.sol#L114 https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/WiseCore.sol#L123 https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/WiseCore.sol#L106

Vulnerability details

Vulnerability Details:

The liquidatePartiallyFromTokens function is used to liquidate a position which reaches a debt ratio greater than 100%. The liquidator can choose the token to payback and receive. (Both can differ). The amount to pay back is in shares of the payback token. The liquidator gets an incentive which is calculated inside the liquidation logic.

The function gets the paybackAmount by converting the users specified pay back shares using the paybackAmount function. The problem here is because the paybackAmount calculation rounds up a user can call the liquidatePartiallyFromTokens function with zero in _shareAmountToPay and the paybackAmount will be one.

    function paybackAmount(
        address _poolToken,
        uint256 _shares
    )
        public
        view
        returns (uint256)
    {
        uint256 product = _shares
            * borrowPoolData[_poolToken].pseudoTotalBorrowAmount;

        uint256 totalBorrowShares = borrowPoolData[_poolToken].totalBorrowShares;

        return product / totalBorrowShares + 1;
    }

This means the liquidation will proceed, setting the paybackAmount to 1 and updating the TotalPool and PseudoTotalBorrowAmount, yet leaving the TotalBorrowShares and userBorrowShares unchanged. This introduces inaccuracies into the protocol's balances, as a proper liquidation should simultaneously update both shares and amounts with it not being possible for one of them being zero while the other is nonzero.

This verification is correctly applied in cases where the amount is rounded down within the protocol, such as through the _validateNonZero check within the _handleDeposit function. However, this check is absent in scenarios where a user indicates zero shares.

Impact:

In such cases, the protocol's balances will be corrupted, and moreover, this vulnerability could be exploited to manipulate prices, an issue that has been observed on numerous occasions in the past.

Proof Of Concept

    it.only("Liquidate zero shares", async () => {
      const transferAmount = pow6(10000);
      const depositAmount1 = toWei("268");
      const depositAmount2 = pow6(5000);
      const borrowAmount = pow6(1700);
      const percent = toWei("0.7");

      await contracts.nft.mintPosition({
        from: alice,
      });

      await contracts.nft.mintPosition({
        from: bob,
      });

      await transferToken({
        tokens: [USDC.address],
        sender: [alice],
        receiver: [bob],
        amount: [transferAmount],
      });

      await setLastUpdateGlobal([chainlinkWETH, chainlinkUSDC]);

      await deposit({
        nftId: [1, 2],
        depositTokens: [WETH.address, USDC.address],
        depositAmount: [depositAmount1, depositAmount2],
        user: [alice, bob],
      });

      await borrow({
        nftId: [1],
        borrowTokens: [USDC.address],
        borrowAmount: [borrowAmount * 0.95],
        user: [alice],
      });

      await chainlinkWETH.setValue(pow8(6));

      const borrowSharesBefore =
        await contracts.lending.getPositionBorrowShares(1, USDC.address);
      // check total pool before
      const totalPoolUSDCBefore = await contracts.lending.getTotalPool(
        USDC.address
      );
      // total pool before WETH
      const totalPoolWETHBefore = await contracts.lending.getTotalPool(
        WETH.address
      );

      await contracts.lending.liquidatePartiallyFromTokens(
        1,
        2,
        USDC.address,
        WETH.address,
        0,
        {
          from: alice,
        }
      );

      // check total pool amount after
      const totalPoolUSDCAfter = await contracts.lending.getTotalPool(
        USDC.address
      );
      // check borrow shares after
      const borrowSharesAfter = await contracts.lending.getPositionBorrowShares(
        1,
        USDC.address
      );
      // check total pool after WETH
      const totalPoolWETHAfter = await contracts.lending.getTotalPool(
        WETH.address
      );
      // assert that the borrow shares are the same
      assert.equal(borrowSharesBefore.toString(), borrowSharesAfter.toString());
      // assert that the total pools are not the same
      assert.notEqual(
        totalPoolUSDCBefore.toString(),
        totalPoolUSDCAfter.toString()
      );
      assert.notEqual(
        totalPoolWETHBefore.toString(),
        totalPoolWETHAfter.toString()
      );
    });

Tools Used:

• Manual analysis
• Foundry

Recommendation:

Add a check to ensure the specified _shareAmountToPay is greater then zero.

    function liquidatePartiallyFromTokens(
        uint256 _nftId,
        uint256 _nftIdLiquidator,
        address _paybackToken,
        address _receiveToken,
        uint256 _shareAmountToPay
    )
        ...
    {
       _validateNonZero(
            _shareAmountToPay
        );

        ...
    }

Assessed type

Other

[c4-pre-sort]

GalloDaSballo marked the issue as sufficient quality report

[GalloDaSballo]

Worth reviewing

[c4-pre-sort]

GalloDaSballo marked the issue as primary issue

[trust1995]

A rounding issue has been demonstrated, causing misaccounting of 1 wei. However it has not been lifted to the impact required for Medium+ severity.

[c4-judge]

trust1995 changed the severity to QA (Quality Assurance)

[c4-judge]

trust1995 marked the issue as grade-c