`WiseSecurity.checksWithdraw` blocks the withdrawal of pooltokens

[hats-bug-reporter[bot]]

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

Description: Description

WiseSecurity.checksWithdraw blocks the withdrawal of pooltokens
which are uncollateralized , blacklisted and have OpenBorrowPosition.

Attack Scenario

Attachments

1. Proof of Concept (PoC) File

Before withdrawing Caller need to pass checksWithdraw() and some other security checks

    function checksWithdraw(
        uint256 _nftId,
        address _caller,
        address _poolToken
    )
        external
        view
        returns (bool specialCase)
    {
        if (_checkBlacklisted(_poolToken) == true) {

            if (overallETHBorrowBare(_nftId) > 0) {
                revert OpenBorrowPosition();
            }

            return true;
        }

        if (WISE_LENDING.verifiedIsolationPool(_caller) == true) {
            return true;
        }

        if (WISE_LENDING.positionLocked(_nftId) == true) {
            return true;
        }

        if (_isUncollateralized(_nftId, _poolToken) == true) {
            return true;
        }

        if (WISE_LENDING.getPositionBorrowTokenLength(_nftId) == 0) {
            return true;
        }
    }

https://github.com/wise-foundation/lending-audit/blob/master/contracts/WiseSecurity/WiseSecurity.sol#L237

If poolToken is blacklisted and uncollateralized they should be withdrawable even if there is OpenBorrowPosition. But here in this case withdrawl will revert with OpenBorrowPosition() and user's funds will be locked

The oos text in contest page In case of withdrawing an uncollateralized asset if you happen to be above 95% debtratio anyway it will still fail (only then). Being above 95% basically means you are in liquidation mode and are therfore incentivized to to use your uncollateralized as collateral instead of removing it to save money and avoid liquidation It basically says that caller should first improve their borrow position and then withdraw

But Here it's not the case as if pooltoken is blacklisted then uncollateralized pooltokens can't be used as a collateral to improve the borrow position.

So all uncollateralized funds will stuck in the contract

Why this is a issue ?

1)

    function revokeShutdown()
        external
        onlyMaster
    {
        _setPoolState(
            false
        );
    }

All other active user's who have uncollateralized assets could front run blacklisting and withdraw all their uncollateralized tokens since protocol allows to withdraw uncollateralized assets if its not blacklisted But funds will stuck for other people who wasn't able to front run

2) Preventing them from withdrawing had a purpose so that they can first improve their borrow position and then withdraw to save their assets. But for blacklisted pooltokens uncollateralized assets can't be used so it doesn't make any sense to lock them.

Mayebe after withdrawl they can swap their withdrawl assets to borrowtoken and then repay borrow tokens to improve their position

User funds is locked here so marking as HIGH

[vm06007]

They are not stuck they can payback their loans and withdraw it.

It does not make it a valid issue. It’s a design choice by requirement. User must payback loans before they can withdraw blacklisted token. In other words protocol ensures user has no borrows before anything blacklisted gets withdrawn (collateralized or not - same behavior expected when token is blacklisted)

Hence this is also up to master to call this function and centralization topic was clearly put out of scope.

If poolToken is blacklisted and uncollateralized they should be withdrawable even if there is OpenBorrowPosition. - this assumption is wrong, based on our requirements if token is blacklisted user must first make sure they don't have any borrows to get rid of this token (regardless if it is uncollateralized or not, (also because making token uncollateralized can be frontrunned by the user whe master decided to blacklist a token)

[Tri-pathi]

@vm06007 They are stuck at the moment.

I'm not able to understand how this is centralization topic ?

it doesn't depend on the Admin when does he want to start blacklisting. Whenever he does, front running is possible and hence this issue too. Also Admin can't do anything to stop this frontrunning so It's not a centralization topic

Some user's can front run blacklisting and remove their token while for other protocol applies extra logics to having no borrow position.

Preventing them from withdrawing make sense if they have borrowed same blacklisting token. But Here it prevents from withdrawing for any borrowed position. Alice have total collateral of 50 ETH(Token A ,B,C, D and E all 10 ETH) She borrowed 20ETH (Token A= 7 ETH, token B= 7ETH and Token C= 6ETH)

Now due to some reason Token E is blacklisted, token E haven't been borrowed(Neither Alice is in liquidation mode instead she is overcollateralized) but withdrawal of E is blocked due to borrowed position in A,B and C which are not blocked.

She will have to wait until blacklisting pauses to remove the token and until then there may be value of E drops to zero(Assuming there was some issue/attack that is why it will be pause/blacklisted at first place) loosing all 10 ETH

sponsor comment- This is intentional, as the removal of blacklisted tokens from the protocol takes precedence over everything else this should be intentional when blacklisted token is borrowed or used as collateral not as the described case i.e A token is deposited and not used to borrow something but can't be withdrawn .

As protocol allows to withdraw blacklisted tokens when borrowed (=0 i.e no borrow position) has same risk scenario for above described case(blacklisted, no borrow position for blacklisted token and token is uncollateralized )

PS: It can't be design but if it is, Add this abnormal behavior in the docs for reference of protocol users. Good luck to passive lenders in such case

Thankyou

[vonMangoldt]

Hi after extensive review we decided that we are gonna leave the code as is since blacklist mainly function is when an attack is suspected so acitng as a kill switch. And then every user if they want to access their funds can always payback everything and get their money. However we did find a bug in collateralizedeposit which we wouldnt have looked at if not for your submission. So we want to still reward you of the books