hats-finance / Wise-Lending-0xa2ca45d6e249641e595d50d1d9c69c9e3cd22573

0 stars 1 forks source link

Bad debt can get stuck in the system, continue to accrue interest and count as pool utilization causing a loss for pool lenders and fee recipients #27

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

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

Github username: @nirohgo Twitter username: niroh30 Submission hash (on-chain): 0x2b8f1bc9874c443d4730900b6c71d3f3429eee718157637200dc8303cc619a63 Severity: high

Description:

Description

WiseLending handles bad debt in the following way:
A. A position in bad debt can be liquidized in full (as opposed to up to 50% at a time for regular positions) as seen in WiseSecurityHelper:

 uint256 maxShares = checkBadDebtThreshold(_borrowETHTotal, _unweightedCollateralETH)
            ? totalSharesUser
            : totalSharesUser * MAX_LIQUIDATION_50 / PRECISION_FACTOR_E18;

B. Bad debt is logged in the fee manager and can be paid back with an incentive, fincanced by the fees accrued in the fee manager (beneficiaries are blocked from withdrawing fees if bad debt exists).
C. Bad debt can also be paid back for no reward, through the fee manager paybackBadDebtNoReward function.

However, the fees in Fee Manager are not guaranteed to cover all bad debt, especially if they have been fully collected before bad debt was created (beneficiaries might front run bad debt-creating-transactions to avoid withdrawal blocks) and/or market volatility creates a large amount of bad debt at once, for example following a sharp drop in price of a token commonly used as collateral in the system.

If this happens, the system can end up with a large amount of bad debt positions whose entire collateral had already been liquidated and whose debt can not be covered by the Fee Manager. (As mentioned, the option exists to pay them back without any compensation however there is no built-in protocol mechanism that guarantees this option will be used to cover all bad debt).

The problem is that in this scenario, bad-debt positions continue to bear interest (which only increases the bad debt since without collateral there is no incentive to repay nor liquidate these positions). In adition, these bad debt positions count as pool utilization affecting the borrow rate, which means the pool is clogged with "bad debt utilization" that doesn't benefit the lender (since they will never pay back the interest) and blocks real borrowers from using the pool.

impact

How To Run:

function getTokensFromETH( address _tokenAddress, uint256 _ethAmount ) public view override returns (uint256) { uint256 res = super.getTokensFromETH(_tokenAddress,_ethAmount);

if (reducedTokens[_tokenAddress] == true) {
    res = res * 1000 / modPercent;
}
return res;

}

function setModifiedToken(address token, bool val) public { reducedTokens[token] = val; }

function setModifiedPercent1000(uint256 percent) public { modPercent = percent; }


Run with "forge test --fork-url YOUR_MAINNET_RPC --match-test testBadDebtPOC -vvv

# Recommendations.
- Bad debt positions (as least the ones with no collateral left which means their bad debt state is permanent) should not be counted for interest accrual or pool utilization. At the end of the _coreLiquidation function there should be a check that registers the amount of permanent bad debt and exludes it from interest and utilization calculations.
- It would be best to have some sort of permanentBadDebtCleanup function possibly one that can be run by anyone for an incentive (or run my the master), that collects "collateralless bad debt" positions after a certain time, burns them and distributes their burden evenly among lenders or borrowers or both (by changing lending/borrowing share prices to accomodate for the lost funds). This will prevent the bank run problem.

**Files:**
  - badDebtPOC.sol (https://hats-backend-prod.herokuapp.com/v1/files/QmUFiZJweQ2DqbgJTQz8YcaetzjAGurj59GCp53EJiqAgC)
vm06007 commented 9 months ago

I think this one can be ruled out, @vonMangoldt will provide important details why this is just a design discussion.

vonMangoldt commented 9 months ago

Reasoning for declaring invalid since its a design discussion:

First of all the paying back bad debt using the fees in itself is just a nice gesture from us but not necessary at all and the absence of that would also not be a bug. There are plenty of protocols who dont do that who actually have bad debt and are totally fine like venus. The important thing is the magnitude in comparison to TVL. Another way of viewing that gesture is that we are then forced to use ongoing fees to use for paying back bad debt. Lets see what happens in the only relevant scenario where bad debt is accumulated ( excluding a drain or exploit which also results in bad debt).

Lets forget gascost for a second and just look at what happens if at least one liquidator acts in self interest to maximize profits. If the % incentive paid to liquidator is bigger than the ratio in % between collateral and debt a liquidation may result in bad debt ( if its not bad debt already). However as you know in that case liquidators can potentially access the whole collateral. So the end result is that they may access as much value out of the collateral as possible and then a borrowposition valued at > 0 is the result. So in the end there is no(if for no gas) or dust collateral left over (rounding or gas reasons or stupid liquidator) in such a position. You can also argue if there is collateral left and it aquires interest it actually pays off the bad debt and may result in a situation where the bad debt vanishes over time vs your suggestion that might not be the case. That means it is a design question and not a bug and therfore dismissed.

vm06007 commented 9 months ago

Totally agree here with @vonMangoldt - the fact that fees are used to compat bad debt is unique feature of the protocol, then protocol owners are incentivized to make sure bad debt is gona before they can resume earning fees, and also to be on look out not to produce bad debt since that would block the fee revenue stream.

nirohgo commented 9 months ago

@vonMangoldt @vm06007 see my response to your comments:

  1. repay-debt-from-fees being a non-mandatory nice to have is irrelevant to this finding. The core of the finding is that bad debt that stays in the system is not "written off" from accounting. This creates the following problems: 1. it counts as utilization (blocking real utilization), it keeps gaining interest (enhancing its burden for no reason), it does not get redistributed, making the last lender/s out incur all loss. The repay-debt-from-fees feature was only mentioned to clarify that it is not a real/full solution to eliminate bad debt (which you seem to agree with in your comment) .
  2. As for your explanation of how liquidations should prevent bad debt or keep it minimal: This is only true in normal times. Even the largest lending systems in DeFi (not to mention small ones) incur significant bad debt from time to time (in spite of having liquidation mechanisms). This happens in market stress conditions i.e. Terra/UST collapse, high volatility, blockchain congestion etc, when liquidations can not happen fast enough to prevent bad debt (you can read more about it here). The difference is that all these systems have some way to either cover bad debt (i.e. SafetyModule in Aave), redistribute it or handle it another way (there are many variations), but not keep it in the system in a way that continues to affect accounting. I have audited dozens of lending systems, none of them leave excess bad debt in the system unhandled.
  3. Given the above, this issue can not count as a design discussion, it is a design fault that puts an unreasonable risk on Wise LPs, making it a high priority issue.
  4. How/when to fix this this issue is up to you, however that shouldn't affect the validity of the issue.
vonMangoldt commented 9 months ago

@vonMangoldt @vm06007 see my response to your comments:

  1. repay-debt-from-fees being a non-mandatory nice to have is irrelevant to this finding. The core of the finding is that bad debt that stays in the system is not "written off" from accounting. This creates the following problems: 1. it counts as utilization (blocking real utilization), it keeps gaining interest (enhancing its burden for no reason), it does not get redistributed, making the last lender/s out incur all loss. The repay-debt-from-fees feature was only mentioned to clarify that it is not a real/full solution to eliminate bad debt (which you seem to agree with in your comment) .
  2. As for your explanation of how liquidations should prevent bad debt or keep it minimal: This is only true in normal times. Even the largest lending systems in DeFi (not to mention small ones) incur significant bad debt from time to time (in spite of having liquidation mechanisms). This happens in market stress conditions i.e. Terra/UST collapse, high volatility, blockchain congestion etc, when liquidations can not happen fast enough to prevent bad debt (you can read more about it here). The difference is that all these systems have some way to either cover bad debt (i.e. SafetyModule in Aave), redistribute it or handle it another way (there are many variations), but not keep it in the system in a way that continues to affect accounting. I have audited dozens of lending systems, none of them leave excess bad debt in the system unhandled.
  3. Given the above, this issue can not count as a design discussion, it is a design fault that puts an unreasonable risk on Wise LPs, making it a high priority issue.
  4. How/when to fix this this issue is up to you, however that shouldn't affect the validity of the issue.

Have you read my explanation? You either have 0 or dust shares in collateral left over. If you have dust collateral and it aquires interest its actually reducing the bad debt amount. So it is a design choice because one could argue that in your suggestion bad debt can stay longer in the system.

nirohgo commented 9 months ago

Exactly, this is exactly the problem: positions with zero or dust collateral, but still with significant borrow shares (in other words bad debt). The problem is that with zero collateral the owner will never pay back the debt, let alone additional interest. This is why they must be excluded from further interest calculations and from pool utilization calculations (this would be the quickest resolution, redistributing debt might be to major of a code change at this point).

vonMangoldt commented 9 months ago

Exactly, this is exactly the problem: positions with zero or dust collateral, but still with significant borrow shares (in other words bad debt). The problem is that with zero collateral the owner will never pay back the debt, let alone additional interest. This is why they must be excluded from further interest calculations and from pool utilization calculations (this would be the quickest resolution, redistributing debt might be to major of a code change at this point).

I misunderstood your point first.

After long discussion on discord with you let me summarize my thoughts here why this is a design question and not a bug:

This is why I consider it a design choice because in the important scenario lenders actually benefit from our current system compared to the suggested one.

The only scenario where it would be worse is when bad debt is so big that it can not or is not believed to be able to be payed back by fees. But at this point even the suggested system will result in a bank run. For example venus which has bad debt is only not experiencing a bank run because if push comes to shove it is collectively believed by the market that they would pay ( so much for defi). So only the collective believe that bad debt is payed back in the future avoids a bank run and therfore only that scenario is important. Also LASA is set to shape the interest curve to supply and demand so thats also not a valid point just in case.. We actually have an enforceable mechanism in place that forces us to use the fees for paying it back. The lenders in other pools dont experience any difference. The amount of fees are still the same just that they dont get send to the "dao" or "master" but are used to directly payback the bad debt which we believe to be a huge improvement vs having to trust e.g aave or venus or whomever to payback baddebt with ongoing fees but thats another discussion

nirohgo commented 9 months ago

The scenario I'm talking about is where there's significant bad debt in created (a not uncommon occurrence in DeFi, usually as a result of stress market conditions as I mentioned above with examples). The fees mechanism in itself can not be relied upon to cover the debt as it is also used to pay beneficiaries and there's no knowing how much it will contain at the time the debt is formed (you yourself referred to it as a nice to have gesture that the system can do without earlier in the thread). Fees can take a very long time to cover the debt, possibly never since the debt incurs interest. During this time, this debt continues to count as utilization and accrue interest, both of which are semantically wrong, cause loss to lenders and can be avoided. (as for LASA, since bad debt will count as demand it will not solve the problem).

vonMangoldt commented 9 months ago

The scenario I'm talking about is where there's significant bad debt in created (a not uncommon occurrence in DeFi, usually as a result of stress market conditions as I mentioned above with examples). The fees mechanism in itself can not be relied upon to cover the debt as it is also used to pay beneficiaries and there's no knowing how much it will contain at the time the debt is formed (you yourself referred to it as a nice to have gesture that the system can do without earlier in the thread). Fees can take a very long time to cover the debt, possibly never since the debt incurs interest. During this time, this debt continues to count as utilization and accrue interest, both of which are semantically wrong, cause loss to lenders and can be avoided. (as for LASA, since bad debt will count as demand it will not solve the problem).

show me signficant bad debt % (over 20) on a running system in lending https://bad-debt.riskdao.org/ venus e.g which is famous for having bad debt had 2%. So my explanation above still stands. In all practical scenarios our system is better for the users since they earn more after bad debt is payed back

nirohgo commented 9 months ago

A. That graph shows a current status on systems where BD can be measured. It doesn't show past BD that was covered/platforms that went down because of BD, platforms where BD is socialized etc. B. Still, sort down by BD ratio and you'll see a few.

vonMangoldt commented 9 months ago

A. That graph shows a current status on systems where BD can be measured. It doesn't show past BD that was covered/platforms that went down because of BD, platforms where BD is socialized etc. B. Still, sort down by BD ratio and you'll see a few.

the burden of proof is on your side not mine