Borrowing without paying interest

[code423n4]

Lines of code

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

Vulnerability details

Impact

A lender can be prevented from withdrawing from his lien, without having to pay him interest. (Only a few negligible wei have to be paid for technical reasons.)

This enables the trader to open a risk free position. Long: The trader swaps the NFT in the lien for ETH. If the price of the NFT goes up, he sells it on the market for a profit. If the price goes down, he returns the NFT and reclaims his ETH. Short: The trader borrows the NFT and sells it on the market. If the price goes down, he buys it on the market and returns it to the lender for a profit. If the price goes up, he just let's himself be liquidated. Combining both the long and short positions enables the trader to profit from any price movement, while the lender gains nothing.

Proof of Concept

The lender has two ways of withdrawing from his lien: withdrawNftWithInterest(), which simply returns his NFT when it is not being borrowed, and withdrawEthWithInterest(), which liquidates the borrower. withdrawNftWithInterest() will revert if the NFT is not in the contract. withdrawEthWithInterest() will revert if the loan is inactive or the borrower is solvent (or an auction is in progress).

Our goal is to make these reverts trigger without having any credit in the lien, which would be consumed as interest. This will be done by taking appropriate actions in frontrunning the lender's call to these functions.

withdrawNftWithInterest() will naturally revert because we have taken the NFT from the lien and contract.

If we provide 1 wei in credit withdrawEthWithInterest() will revert within the same block, before interest has accrued. Whenever thereafter the lender calls this we frontrun his call with repayWithNft() with another NFT from the same collection which we already own (we make use of the fungibility assumption here). Then the lender's call will revert because the loan is no longer active. This will also accrue interest. And herein lies the issue. The interest is capped to the credit. Since no (i.e. up to 1 wei) credit was provided, we pay no interest and get the full price back. Immediately thereafter we renew the loan, which prevents the lender from calling withdrawNftWithInterest().

(Note that the 1 wei is only a safeguard to prevent liquidating in the same block, which the lender might not even attempt, and that this is due technically only to the "<" instead of the equally reasonable "<=" (were it not for the interest cap) in the liquidation constraint.)

This can be repeated until the trader wants to quit his position.

Recommended Mitigation Steps

The interest should only be capped to the credit when liquidating. The borrower should be required to pay the full interest if he wants to repayWithNft().

Assessed type

Other

[c4-judge]

hansfriese marked the issue as unsatisfactory: Insufficient quality

[hansfriese]

Quality is very low and it is not clear what the warden wants to say. Very likely to be a duplicate of #33.

[hansfriese]

Leave it open for the sponsor's review for now but likely to mark it as invalid.

[wukong-particle]

This should be duplication to https://github.com/code-423n4/2023-05-particle-findings/issues/16 IIUC.

[wukong-particle]

But I agree, a little hard to digest what warden really meant, I acknowledge the issue here if it's indeed duplication of 16. Not "confirming" because the mitigation suggestion doesn't seem right, will use the approach in https://github.com/code-423n4/2023-05-particle-findings/issues/16.

[c4-sponsor]

wukong-particle marked the issue as sponsor acknowledged

[hansfriese]

Just to make it clear, I nullified this one with #41 from the same warden in mind.

[c4-judge]

hansfriese marked the issue as nullified

[d3e4]

Quality is very low and it is not clear what the warden wants to say. Very likely to be a duplicate of #33.

33 is about the possibility of opening a position that is immediately liquidatable.

This should be duplication to #16 IIUC.

16 is about DoS-ing lien validation by changing its hash by calling addCredit().

This issue has nothing to do with either #33 or #16. The exploit here opens a position with 1 wei in credit specifically so that it is not liquidatable, and nowhere in the exploit does addCredit() have to be called.

Maybe the exploit is a less straightforward, but the root issue and the impact are simple. The root issue is that the interest is capped to the credit not only for the liquidator, but also for the repaying borrower. If it was possible to be in more debt than available in the credit provided, then liquidation would require the borrower to pay his full debt in due interest. But since it is not possible to repossess funds that are not already deposited, the interest has to be capped at the credit when liquidating (the lender still gets his desired price, so this is still good for him). But this constraint does not apply to the borrower when repaying his loan, so in this case he should be forced to pay his full debt if he wants his deposit back. The fact that the repaying borrower doesn't have to pay his full interest debt can be exploited to evade paying any interest at all.

The idea is to simply open a position with no credit to pay interest with when liquidating. This way the liquidator cannot get any interest. Now, what should happen is that the borrower is simply liquidated and that's the end of it (which implies a loss for the borrower). The problem is that the borrower can fully legitimately repay his loan before the lender tries to liquidate him and then immediately renew the loan. He should have to pay interest on the time between borrowing and repaying (just as if he had plenty of credit), but since the interest cap also applies to repaying the loan he doesn't have to! This way he can renew his loan without paying interest, instead of being liquidated, which is equivalent to borrowing without paying interest.

All of this is easily solved by simply requiring that the full interest is paid when repaying the loan. This may be taken from the margin already provided (if the interest exceeds even the margin, it would be cheaper for the borrower to let himself be liquidated).

[wukong-particle]

Ok I understand the issue now, really appreciated your patience for the deep down explanation.

This is in the same spirit as your other issue https://github.com/code-423n4/2023-05-particle-findings/issues/40 (lender keeps taking borrower interest), and this time borrower is attacking the lender by not paying the interest. We can potentially patch this by forcing a minimum interest (i.e., position can't immediately become insolvent).

This way, lender is always entitled for some interest before liquidation; and if the lender is perfectly accurate, she can keep the time of interest-free 0 by auction/liquidate in time. Does this resolve the issue @d3e4

[d3e4]

The proposed exploits in #40 and #34 both involve interest but with opposite attacker-victim roles. But the root issues are not so similar. #40 is a lien validation DoS, #34 is an interest cap when repaying.

The mitigation you proposed in #40 seems good, see my comment there.

By forcing a minimum interest I suppose you mean requiring some minimum credit when opening a position? This doesn't do anything for #40. It puts an obstacle in the way for this issue, and may prevent it in practice, but doesn't truly resolve it. It sort of misses the point. And why shouldn't a borrower be able to open a position that almost immediately becomes insolvent? There is already a kind of security deposit in the price margin, so liquidation is good for the lender and bad for the borrower. Maybe he wants to borrow it for a very short time, so there is no need to require more credit from him than needed.

The borrower should not be allowed to take actions which are contrary to his paying interest. I see two instances of this: one is in adding credit and the other is in repayment. There is no legitimate reason to add credit except to become or remain solvent. That this is still allowed turns out to be exploitable, as shown in #16. The proper way to mitigate this is to enforce being solvent after adding credit (a minimum limit may still be required, see this comment). It is not legitimate to repay a loan without paying the full interest. That this is still allowed is shown here (#34) to be exploitable. The proper way to mitigate this is to enforce that the borrower pays the full interest when repaying his loan. This may imply that the borrower has to send some funds with the call to repayWithNft(), or that he first has to add credit. Only then should he be allowed to repay his loan.

[hansfriese]

The finding is titled "Borrowing without paying interest" and the warden's key point is the borrower needs to pay the full debt. But this does not harm the lender as long as the liquidation works. (See #32)

If the warden's point is that the liquidation can be DoS-ed, it becomes a duplicate of #16 in the sense of the attacker changing lien data by frontrunning.

As a side note, I would like to mention that this report is of low quality which can lead to invalidation.

Per C4 documentation:

The validity of an audit report submission is not based on whether it is ‘true’ or not. A report may contain a finding which is factually 'true' (the most literal interpretation of 'valid'), but if it does not add value or if it is not presented in such a way that adds value to a sponsor, it may be deemed invalid by a judge. This may seem harsh and exclusive, but it is essential to consider that Code4rena runs audit contests, not gotcha-hunts, and Code4rena offers guaranteed payout for valid submissions. This means that wardens are providing a service to sponsors and the product of those services should meet what judges feel is a minimum standard in order to be deemed of value.

[d3e4]

The finding is titled "Borrowing without paying interest" and the warden's key point is the borrower needs to pay the full debt. But this does not harm the lender as long as the liquidation works. (See #32)

The borrower can KEEP loaning without paying interest. Liquidation does NOT work because the borrower prevents the lender from getting his NFT, whether through liquidation or withdrawal. So the lender is at a direct loss here. The lender cannot stop the loan but still isn't paid interest. The borrower can borrow for as long as he wants, without paying interest.

If the warden's point is that the liquidation can be DoS-ed, it becomes a duplicate of #16 in the sense of the attacker changing lien data by frontrunning.

This has nothing to do with changing lien data. It renews the loan, which because of the bug let's the borrower get away without paying interest.

As a side note, I would like to mention that this report is of low quality which can lead to invalidation.

How specifically is this of low quality? I very clearly state that the issue is that the interest is capped to the credit in repayWithNft() and show how this can be exploited to directly cause a gain for the attacker at the cost of the lender.

[hansfriese]

What do you mean

Liquidation does NOT work because the borrower prevents the lender from getting his NFT, whether through liquidation or withdrawal.

[d3e4]

What do you mean

Liquidation does NOT work because the borrower prevents the lender from getting his NFT, whether through liquidation or withdrawal.

From my report:

The lender has two ways of withdrawing from his lien: withdrawNftWithInterest(), which simply returns his NFT when it is not being borrowed, and withdrawEthWithInterest(), which liquidates the borrower. withdrawNftWithInterest() will revert if the NFT is not in the contract. withdrawEthWithInterest() will revert if the loan is inactive or the borrower is solvent (or an auction is in progress).

Both of these can be deliberately caused by the borrower, which prevents liquidation, as described in my report. My later explanation above might be clearer:

The idea is to simply open a position with no credit to pay interest with when liquidating. This way the liquidator cannot get any interest. Now, what should happen is that the borrower is simply liquidated and that's the end of it (which implies a loss for the borrower). The problem is that the borrower can fully legitimately repay his loan before the lender tries to liquidate him and then immediately renew the loan. He should have to pay interest on the time between borrowing and repaying (just as if he had plenty of credit), but since the interest cap also applies to repaying the loan he doesn't have to! This way he can renew his loan without paying interest, instead of being liquidated, which is equivalent to borrowing without paying interest.

All that is required is that the borrower can do this before the lender liquidates him, which can at least be done by frontrunning.

[hansfriese]

The idea is to simply open a position with no credit to pay interest with when liquidating. This way the liquidator cannot get any interest.

This does not make sense to me. Again there is no loss for the lender as long as the liquidation succeeds.

Now, what should happen is that the borrower is simply liquidated and that's the end of it (which implies a loss for the borrower).

Who is the attacker in your issue? Is it a borrower or lender? You are saying the borrower gets a loss?

Again, I still think your report is of very low quality. Please note that this is my final response as a judge for this contest. I would recommend you write better reports in future contests. Wish you the best of luck.

[d3e4]

This does not make sense to me. Again there is no loss for the lender as long as the liquidation succeeds.

The liquidation never succeeds. The borrower prevents liquidation for as long as he wants to keep the loan. Then he simply returns the NFT. (It was confusing to speak of the liquidator, I agree. I simply meant to not provide any credit which can become interest for the lender when the borrower returns the loan.)

Who is the attacker in your issue? Is it a borrower or lender? You are saying the borrower gets a loss?

The borrower attacks the lender by borrowing from him without paying interest. The lender suffers a loss. (The sentence you quoted states what is implied by liquidation. The point is that liquidation is prevented.)