code-423n4 / 2024-07-benddao-findings

9 stars 6 forks source link

Bad debt is never handled which places insolvency risks on BendDAO #23

Open c4-bot-1 opened 3 months ago

c4-bot-1 commented 3 months ago

Lines of code

https://github.com/code-423n4/2024-07-benddao/blob/main/src/libraries/logic/LiquidationLogic.sol#L99-L105

Vulnerability details

Impact

Bad debt is never handled, which may happen whenever the collateral asset price crashes or the debt value spikes. Whenever this happens ERC20 positions will be partially liquidated and ERC721 positions will not be profitable for liquidation, which means the debt will not be repaid. When this happens, the protocol will not have enough liquidity to fulfil withdrawals, DoSing them forever or until the price comes back up again.

Proof of Concept

LiquidationLogic::executeCrossLiquidateERC20() allows partial, liquidations, so the likely outcome here would be liquidation of the debt corresponding to the available collateral, but some debt could be outstanding. ERC721 liquidations don't allow partial liquidations in the same way, as the price of each collateral asset is based on the debt value only, so it could happen that these debt assets are not liquidated at all.

As withdrawing deposits depends on the protocol having liquidity, if these assets are borrowed and there is no way to repay them, users will not be able to withdraw.

Tools Used

Vscode

Foundry

Recommended Mitigation Steps

On liquidation, place a mechanism to handle the creation of bad debt. This could be implemented as a reserve or by decreasing the supply index or similar, so the bad debt is redistributed by the suppliers.

Assessed type

Other

c4-judge commented 3 months ago

MarioPoneder marked the issue as unsatisfactory: Insufficient proof

thorseldon commented 3 months ago

The bad debt should be handled by the DAO Treasury and the protocol income.

Simao123133 commented 3 months ago

@MarioPoneder There was no mention of the treasury handling bad debt at the time of the contest. As per the report, the bad debt is not handled because: 1) ERC20 tokens clear debt up to the available collateral, so if debt > collateral, this leftover debt will never be handled. 2) ERC721 tokens are not profitable for liquidation (separate issue here).

Also, for ERC20 tokens, the treasury and protocol income can not cover the bad debt because it is not possible to clear the extra debt. It will revert here as the user has no collateral left. So the only option for the protocol to clear this bad debt would be gifting the user collateral to liquidate him, which is a very weird flow.

MarioPoneder commented 3 months ago

Thanks for following-up with more info after I initially closed due to insufficient proof.

The Warden has shown that the protocol seems to have no graceful way of handling bad dept position in case of collateral value crashes which are common in the DeFi space.
Also, the source of truth (codebase & README) do not outline how these situations could be handled by the DAO treasury.

c4-judge commented 3 months ago

MarioPoneder marked the issue as satisfactory

c4-judge commented 3 months ago

MarioPoneder marked the issue as selected for report

thorseldon commented 3 months ago

@c4-judge @Simao123133 For the bad debt, DAO treasury should actively repay the debt using crossRepayERC20. I don't know why reopen this finding? Most of the lending pool will has bad debt need to covered by DAO treasury or protocol income.

I think security audit should focus on the vulnerabilities in the code, not question the service model design.

Simao123133 commented 3 months ago

@thorseldon it wasn't publicly mentioned at the time of the contest you would be taking losses and handling bad debt directly, so the finding is in scope.