code-423n4 / 2022-04-jpegd-findings

1 stars 1 forks source link

[WP-H22] Bad debts should not continue to accrue interest #167

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-04-jpegd/blob/e72861a9ccb707ced9015166fbded5c97c6991b6/contracts/vaults/NFTVault.sol#L844-L851

Vulnerability details

https://github.com/code-423n4/2022-04-jpegd/blob/e72861a9ccb707ced9015166fbded5c97c6991b6/contracts/vaults/NFTVault.sol#L844-L851

uint256 debtAmount = _getDebtAmount(_nftIndex);
require(
    debtAmount >= _getLiquidationLimit(_nftIndex),
    "position_not_liquidatable"
);

// burn all payment
stablecoin.burnFrom(msg.sender, debtAmount);

In the current design/implementation, the liquidator must fully repay the user's outstanding debt in order to get the NFT.

When the market value of the NFT fell rapidly, the liquidators may not be able to successfully liquidate as they can not sell the NFT for more than the debt amount.

In that case, the protocol will have positions that are considered bad debts.

However, these loans, which may never be repaid, are still accruing interest. And every time the DAO collects interest, new stablecoin will be minted.

When the proportion of bad debts is large enough since the interest generated by these bad debts is not backed. It will damage the authenticity of the stablecoin.

PoC

Given:

  1. Alice borrowed 10,000 USD with NFT #1;
  2. After 1 year, NFT 1's market value in USD has suddenly dropped to 10,000 USD, no liquidator is willing to repay 11,000 USD for NFT #1;
  3. The DAO collect() and minted 1,000 stablecoin;
  4. After 1 year, the DAO call collect() will mint 1,100 stablecoin. and so on...

Recommendation

Consider adding a stored value to record the amount of bad debt, and add a public function that allows anyone to mark a bad debt to get some reward. and change accrue to:

uint256 internal badDebtPortion;

function accrue() public {
    uint256 additionalInterest = _calculateAdditionalInterest();

    totalDebtAccruedAt = block.timestamp;

    totalDebtAmount += additionalInterest;

    uint256 collectibleInterest = additionalInterest * (totalDebtPortion - badDebtPortion) / totalDebtPortion;
    totalFeeCollected += collectibleInterest;
}
dmvt commented 2 years ago

I agree with the warden. Left unchecked, this issue is almost certain to occur and will cause substantial negative impacts on the protocol. The only way this would not occur is if the NFT market never crashes.