In case of PoolToken is blacklisted, Positions can be liquidated

[hats-bug-reporter[bot]]

Github username: @@Tri-pathi Twitter username: ... Submission hash (on-chain): 0xe567ad65dc5693e64d037e9ca7c70e5122123fe103264c960c2820d54e513595 Severity: high

Description: Description\

In case of PoolToken is blacklisted, Position can be liquidated

Attack Scenario\

Exploit path

1) Admin blacklisted the pooltoken, which will block all the deposits and withdrawal of pooltoken, if user's have some Open Borrow position

2. Since caller can't manage his portfolio during the blacklisted time,so tokens value can varies such that user's position becomes liquidable

3. A Liquidator can calls WiseLending::liquidatePartiallyFromTokens or WiseLending::coreLiquidationIsolationPools to liquidate the position

Attachments

1. Proof of Concept (PoC) File WiseLending::liquidatePartiallyFromTokens and WiseLending::coreLiquidationIsolationPools both depends on the _coreLiquidation(data) for all the security checks

    function _coreLiquidation(
        CoreLiquidationStruct memory _data
    )
        internal
        returns (uint256 receiveAmount)
    {
        _validateNonZero(
            _data.paybackAmount
        );
 //Math function computing the percentage of the receiving token which the liquidator receivs for liquidation.
        uint256 collateralPercentage = WISE_SECURITY.calculateWishPercentage(
            _data.nftId,
            _data.tokenToRecieve,
            WISE_ORACLE.getTokensInETH(
                _data.tokenToPayback,
                _data.paybackAmount
            ),
            _data.maxFeeETH,
            _data.baseRewardLiquidation
        );

        _validateParameter(
            collateralPercentage,
            PRECISION_FACTOR_E18
        );

        _corePayback(
            _data.nftId,
            _data.tokenToPayback,
            _data.paybackAmount,
            _data.shareAmountToPay
        );

        receiveAmount = _calculateReceiveAmount(
            _data.nftId,
            _data.nftIdLiquidator,
            _data.tokenToRecieve,
            collateralPercentage
        );

        WISE_SECURITY.checkBadDebtLiquidation(
            _data.nftId
        );

        _curveSecurityChecks(
            _data.lendTokens,
            _data.borrowTokens
        );

        _safeTransferFrom(
            _data.tokenToPayback,
            _data.caller,
            address(this),
            _data.paybackAmount
        );

        _safeTransfer(
            _data.tokenToRecieve,
            _data.caller,
            receiveAmount
        );
    }

After analysing every internal function , found that no where it checks that tokenToRecieve and tokenToPayback are blacklisted .

So liquidation will work as nothing is blacklisted and all pooltoken's are active

From the design of protocol as such blacklisted/shutdown condition, these functionalities should stop. As borrowers can't provide deposits during emergency period it is very high chance that their position can come into liquidation range

Even After emergency period ends, Borrowers should given a buffer time for depositing or improving their position[Most of the defi protocol do]. Event if protocol prevents liquidation during emergency , A lot of liquidation starts just after (_checkBlacklisted(_poolToken)== false due to increase/decrease value of token during emergency period.

Direct loss of user funds so marking HIGH

2. Revised Code File (Optional)

[vonMangoldt]

Hi this is intentional. Blacklist is supposed to be in case a suspected attack is happening which involves depositing and or borrowing but reducing positions is still allowed (payback etc or withdraw if 0 borrow). Users can always payback their debt to access funds too. Otherwise blacklisting could directly result in bad debt. You can also take the other side of argument and say liquidations should also be stopped but hence its a design question and we will leave it as is.

[Tri-pathi]

@vonMangoldt

This is a bit strange design than traditional defi protocols. Can't comment anything as you said it's intentional but Generally these issue is treated as acknowledged tag instead of invalid:::)

liquidation will be active even during securityShutdown so Adding such security concern in website will be helpful for protocol users Thankyou

[vm06007]

there's no acknowledged tag present in our disposal. this seems to be correlated with #51 as side-effect of how you see the protocol "should" be working according to you, and not design chosen by the client, can't say its a bug or a finding, this seems just "informational" topic for me in comparison to other protocols from 3rd party words, must check also other protocols then first to conclude.

Please also see final response here from @vonMangoldt I think it is clear and makes a valid point