Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Muscular Yellow Haddock - Assets that has not entered markets can still be seized during a liquidation. #244

Open
sherlock-admin2 opened this issue Dec 31, 2024 · 0 comments

Comments

@sherlock-admin2
Copy link
Contributor

Muscular Yellow Haddock

High

Assets that has not entered markets can still be seized during a liquidation.

Summary

NumaComptroller.liquidateBorrowAllowed() never checks for accountMembership i.e. whether the market has been entered by the user.

As a result, assets that were only deposited for lending and not collateralized can still be seized during a liquidation.

Root Cause

When a user wants to borrow tokens, they have to deposit assets, and then enter market for that particular asset. The asset for the entered market will then be used for account liquidity calculation.

    /**
     * @notice Add assets to be included in account liquidity calculation
     * @param cTokens The list of addresses of the cToken markets to be enabled
     * @return Success indicator for whether each corresponding market was entered
     */
    function enterMarkets(
        address[] memory cTokens
    ) public override returns (uint[] memory) {

https://github.com/sherlock-audit/2024-12-numa-audit/blob/main/Numa/contracts/lending/NumaComptroller.sol#L150-L157

When a user becomes unhealthy and is liquidatable, liquidators can call liquidateBorrow() on the collateral CNumaToken to seize the asset in exchange for repaying the debt. The liquidator specifies a collateral token to seize, in exchange for a debt token to repay.

https://github.com/sherlock-audit/2024-12-numa-audit/blob/main/Numa/contracts/lending/CNumaToken.sol#L343-L358

The call flow will lead into NumaComptroller.liquidateBorrowAllowed(), where it will revert if and only if the liquidation is not allowed for the collateral/debt pair, and will otherwise allow the tx to go on

    /**
     * @notice Checks if the liquidation should be allowed to occur
     * @param cTokenBorrowed Asset which was borrowed by the borrower
     * @param cTokenCollateral Asset which was used as collateral and will be seized
     * @param liquidator The address repaying the borrow and seizing the collateral
     * @param borrower The address of the borrower
     * @param repayAmount The amount of underlying being repaid
     */
    function liquidateBorrowAllowed(
        address cTokenBorrowed,
        address cTokenCollateral,
        address liquidator,
        address borrower,
        uint repayAmount
    ) external view override returns (uint) {

https://github.com/sherlock-audit/2024-12-numa-audit/blob/main/Numa/contracts/lending/NumaComptroller.sol#L557

However, liquidateBorrowAllowed() never checks if the cTokenCollateral has actually entered market yet, in fact never touches the accountMembership mapping, updated to True when the user has entered the market.

Thus the user may be wrongfully liquidated for an asset that they never collateralized.

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

  1. Alice collateralizes 1 ETH to borrow 2000 USDC.
  2. Alice deposits 0.5 WBTC, without entering market for it. This is because Alice specifically wants WBTC to only earn lending interest, but she doesn't want to lose her WBTC exposure.
  3. ETH price drops, dropping Alice's borrow power to less than 2000 USDC. Alice is now liquidatable.
  4. Liquidator chooses to liquidate WBTC. The liquidation still passes and Alice still loses her WBTC, despite it never being collateralized.

Alice has lost her WBTC to liquidation, despite the WBTC not used to collateralize anything, and has not even entered the market.

Impact

Assets that are not used for collateralization can still be seized during a liquidation. Users face a loss of the asset that they never intend to risk.

PoC

No response

Mitigation

liquidateBorrowAllowed() must additionally check if the market for cTokenCollateral is entered or not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant