hats-finance / Wise-Lending-0xa2ca45d6e249641e595d50d1d9c69c9e3cd22573

0 stars 1 forks source link

BlackListing a token with a position tied to NFT will break functions #24

Open hats-bug-reporter[bot] opened 8 months ago

hats-bug-reporter[bot] commented 8 months ago

Github username: @https://github.com/betharavikiran Twitter username: @ravikiranweb3 Submission hash (on-chain): 0xe32d02a005dd5cb6f2bd97508b75f8981400a43137f25c1447520c1275ca5393 Severity: medium

Description: Description\ BlackListing a token with an attached positon in NFT will break many functions in WiseSecurity Contract.

The below is the list of functions that will stop working. While all these functions are view functions, they will not work for any NFT that has a position for the token that was blacklisted.

a) getLiveDebtRatio

b) overallETHCollateralsWeighted

c) maximumBorrowToken

d) maximumWithdrawToken

e) maximumWithdrawTokenSolely

f) overallETHBorrow

The underlying reason for these functions to not work is the call to _checkPoolCondition() function by the above list of functions.

Since the function reverts if any of the poolToken is black listed and hence halting the functioning of all the above functionality not being available for such NFTs.

function _checkPoolCondition(
        address _poolToken
    )
        internal
        view
    {
        if (_checkBlacklisted(_poolToken) == true) {
            revert TokenBlackListed();
        }
    }

Attack Scenario\ a) Setup poolTokens and create a positions for NFT b) Call these view functions for the above created NFT ID, all should work as expected. c) blacklist a token on one, or both arms of the NFT position related token. d) the above list of functions will stop working.

Attachments

  1. Proof of Concept (PoC) File The below function is called by above view functions. As any of the tokens for the NFT are blacklisted, the checkBlackListed will be true and hence everything will revert.
function _checkPoolCondition(
        address _poolToken
    )
        internal
        view
    {
        if (_checkBlacklisted(_poolToken) == true) {
            revert TokenBlackListed();
        }
    }
  1. Revised Code File (Optional)

The Blacklisting should be gated on new deposits. Also, cautiously gated on withdrawls to prevent any malicious activity. But, this need not be checked while generating information in view only format.

At the time of withdrawal or borrowing or deposit, prevent the interaction by reverting with detailed message. But, for all view only functions, remove the _checkPoolCondition() to resolve the issue.

vm06007 commented 8 months ago

@betharavikiran in fact this is intentional, this promotes all users to get rid of blackListed tokens from their positions. While user has blackListed tokens in their borrowed or supplied portion, they should not be able to get any new borrows or use it as a collateral. Hence these views are defined in WiseSecurity that then utilized by main protocol preventing unwanted actions for NFTs that do have any blackListed tokens in their positions.

@vonMangoldt can add more if needed.

vonMangoldt commented 8 months ago

This is intentional so that over time you are as a holder of the blacklisted token are incentivized to withdraw it (payback debt before obviously)