code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-05-particle/blob/1caf678bc20c24c96fc8f6b0046383ff0e9d2a6f/contracts/protocol/ParticleExchange.sol#L192-L223

Vulnerability details

Impact

When the borrower cannot repay with NFT he will be forced to forsake his entire credit. This situation can be deliberately instigated by the lender.

Proof of Concept

A borrower can only leave his position by returning an NFT (buyNftFromMarket() or repayWithNft()) or being liquidated when his entire credit is consumed by interest (withdrawEthWithInterest()). Since NFTs are not always approximately fungible it may happen that the borrower cannot procure an NFT. Even if he has a lot of credit remaining he will be forced to see all his credit consumed by interest, unable to stop loss his position. This is further aggravated by the possibility of starting an auction. Since no NFT can be provided the auction will run to the maximum price in just 24 hours.

This opens for a direct exploit by the lender. The lien would usually have some margin, such that even if no extra credit is provided the desired price is still higher than the amount for which the NFT is sold. This means that concluding an auction is very profitable for the lender. All he has to do is to stall the procurement of NFTs of the same collection at a price point lower than the auction max price for 24 hours. Given the nature of NFTs not being generally fungible, for a suitable collection this should not be infeasibly difficult. He may for example simply buy the remaining tokens below his price point, and resell them 24 hours later; the price is unlikely to drastically change.

It is also possible to prevent the borrower from returning the NFT by starting and stopping an auction, which is an issue in and of itself and reported separately. This can obviously not be used in the above auction exploit but can be used to force the borrower to keep loaning and paying more interest.

Recommended Mitigation Steps

Implement an option for the borrower to self-liquidate, to pay only the price and interest up until that time, saving him his remaining credit. This may be complemented with a function to remove credit.

Assessed type

Context

hansfriese commented 1 year ago

I believe this is a design choice, i.e., by adding a margin (credit) a borrower accepts the risk of the credit consumed for interest. Essentially the warden is suggesting providing a way for a borrower to "decrease credit". Keep it open for the sponsor's review for now.

c4-judge commented 1 year ago

hansfriese marked the issue as satisfactory

c4-sponsor commented 1 year ago

wukong-particle marked the issue as sponsor disputed

c4-sponsor commented 1 year ago

wukong-particle marked the issue as disagree with severity

wukong-particle commented 1 year ago

Judge is correct, this is a design choice, hence dispute the validity and disagree with severity.

We acknowledge the concern though, in our frontend, we will clearly message both the lender that the borrower that the fungibility assumption is made (including all the implications) once asset is supplied and the position is opened.

Moreover, it's actually incorrect that a concluded auction will forsake borrower's entire credit. We only transfer the accrued interest to lender, and the rest is back to the borrower: https://github.com/code-423n4/2023-05-particle/blob/main/contracts/protocol/ParticleExchange.sol#L214-L217.