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

11 stars 8 forks source link

Users may lose assets with drained liquidity, impacting borrowing and withdrawal functionality. #179

Closed c4-bot-10 closed 5 months ago

c4-bot-10 commented 6 months ago

Lines of code

https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/WiseLending.sol#L714-L745 https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/WiseLending.sol#L724-L732

Vulnerability details

Impact

If an attacker successfully drains liquidity from the lending pools, users who have deposited their funds into those pools may lose their assets, leading to financial losses and a loss of trust in the protocol.

  1. With depleted liquidity, the protocol may not have sufficient funds to fulfill borrowing requests or allow users to withdraw their deposits, effectively halting the core lending and borrowing functionality.

  2. Liquidity-draining attacks can severely damage the reputation of the Wise Lending protocol, eroding user confidence and potentially leading to a decline in adoption and usage.

Proof of Concept

Liquidity-draining attacks pose a significant risk to the Wise Lending protocol, as they aim to exploit vulnerabilities or design flaws to drain funds from lending pools or token reserves. If successful, these attacks can severely impact the protocol's liquidity.

  1. Withdrawal Logic: In the WiseLending contract, the withdrawExactAmount function allows users to withdraw their deposited funds: WiseLending.sol#withdrawExactAmount
function withdrawExactAmount(
    uint256 _nftId,
    address _poolToken,
    uint256 _withdrawAmount
)
    external
    syncPool(_poolToken)
    healthStateCheck(_nftId)
    returns (uint256)
{
    uint256 withdrawShares = _handleWithdrawAmount(
        {
            _caller: msg.sender,
            _nftId: _nftId,
            _poolToken: _poolToken,
            _amount: _withdrawAmount,
            _onBehalf: false
        }
    );

    _validateNonZero(
        withdrawShares
    );

    _safeTransfer(
        _poolToken,
        msg.sender,
        _withdrawAmount
    );

    return withdrawShares;
}

The withdrawExactAmount function relies on the _handleWithdrawAmount function to process the withdrawal: _handleWithdrawAmount#L724-L732

function _handleWithdrawAmount(
    address _caller,
    uint256 _nftId,
    address _poolToken,
    uint256 _amount,
    bool _onBehalf
)
    private
    returns (uint256 withdrawShares)
{
    withdrawShares = _preparationsWithdraw(
        _nftId,
        msg.sender,
        _poolToken,
        _amount
    );

    _coreWithdrawToken(
        {
            _caller: _caller,
            _nftId: _nftId,
            _poolToken: _poolToken,
            _amount: _amount,
            _shares: withdrawShares,
            _onBehalf: _onBehalf
        }
    );
}

The withdrawExactAmount function does not have sufficient access controls to prevent unauthorized withdrawals. It relies on the _nftId and msg.sender to determine the caller's eligibility to withdraw funds, but there are no explicit checks to ensure that the caller is the rightful owner of the deposited funds.

_The code lacks proper balance checks to ensure that the requested withdrawal amount does not exceed the user's actual deposited balance. The _preparationsWithdraw function calculates the withdrawal shares based on the requested amount, but it does not verify if the user has sufficient shares or funds available for withdrawal._

So as we can see, the protocol's liquidity management techniques may be flawed, allowing attackers to manipulate the liquidity pools. For example, if the protocol relies solely on the total supply of tokens in the pool to determine liquidity, an attacker could artificially inflate the supply by depositing and withdrawing funds repeatedly, making it appear as if there is more liquidity than available.

Consider a scenario where an attacker discovers the vulnerability in the Wise Lending protocol's withdrawal logic. They notice that the withdrawExactAmount function lacks proper access controls and balance checks. The attacker proceeds to exploit this vulnerability by making multiple withdrawal requests, each time requesting an amount greater than their actual deposited balance. Due to the lack of proper checks, the protocol allows these withdrawals, effectively draining funds from the lending pool.

Tools Used

VsCode

Recommended Mitigation Steps

Implement strict access controls to ensure that only the rightful owners of deposited funds can initiate withdrawals. This can be achieved by verifying the caller's identity through authentication mechanisms such as digital signatures or access tokens.

  1. Introduce balance checks in the withdrawal logic to ensure that users can only withdraw funds up to their actual deposited balance. Compare the requested withdrawal amount with the user's available balance and shares before processing the withdrawal.

Assessed type

Invalid Validation

c4-pre-sort commented 6 months ago

GalloDaSballo marked the issue as insufficient quality report

GalloDaSballo commented 6 months ago

If an attacker successfully drains liquidity from the lending pools, users who have deposited their funds into those pools may lose their assets, leading to financial losses and a loss of trust in the protocol.

Yes

vm06007 commented 5 months ago

These are some unclear thoughts, if you think the accounting is not done correctly please provide POC of a depositor being able to withdraw more than they have deposited any way possible. Should be dismissed.

c4-judge commented 5 months ago

trust1995 marked the issue as unsatisfactory: Invalid