NFT withdrawal grief

[code423n4]

Lines of code

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

Vulnerability details

Impact

A lienee whose NFT is not currently on loan may be prevented from withdrawing it.

Proof of Concept

A lienee who wishes to withdraw his NFT calls withdrawNftWithInterest() which tries to IERC721.safeTransferFrom() the NFT, which therefore reverts if the NFT is not in the contract (being on loan). A griefer might therefore sandwich his call to withdrawNftWithInterest() with a swapWithEth() and a repayWithNft(). swapWithEth() removes the NFT from the contract, which causes the following withdrawNftWithInterest() to revert. For this the griefer has to pay lien.credit + lien.price. But this is returned in full in repayWithNft() minus payableInterest which is nothing since the loan time is zero.

Recommended Mitigation Steps

Allow the option to, at any time, set a lien to not accept a new loan.

Assessed type

DoS

[hansfriese]

No economical benefit for the attacker.

[c4-judge]

hansfriese marked the issue as unsatisfactory: Invalid

[wukong-particle]

Judge is correct. And this NFT swap behavior can happen any time (similar https://github.com/code-423n4/2023-05-particle-findings/issues/19), there's no particular benefit to do so as a sandwich attack.

[c4-sponsor]

wukong-particle marked the issue as sponsor disputed

[d3e4]

No economical benefit for the attacker.

There doesn't have to be. This is a pure griefing attack to prevent the lienee from withdrawing, hence rated only Medium.

[hansfriese]

@wukong-particle I think this grief attack does not incur a direct loss for the lender but if the NFT ownership could yield any other type of profit, this can lead to an implicit economical loss for the lender. From this viewpoint, I am leaning to agree with the MEDIMUM severity although the sandwich attack costs significantly on the main net.

[c4-judge]

hansfriese marked the issue as satisfactory

[wukong-particle]

@hansfriese I agree this is a pure grief attack with no economic incentive. This could be MED or LOW severity issue because grief can technically raid any swap feature (even for any protocol). In the report please point out the pure grief nature of this attack -- we will consider patching this (e.g., minimum fee for opening a position), but if we leave this grief attack we want to acknowledge that there's no loss or anything associated with it. Thanks!

[c4-judge]

hansfriese marked the issue as selected for report

[romeroadrian]

@hansfriese I addressed this in L-9 here https://github.com/code-423n4/2023-05-particle-findings/blob/main/data/adriro-Q.md

There's no incentive here other than the grief, and the attacker will only have to pay gas. I believe the report has overinflated severity and should be downgraded to low. The recommendation also doesn't make sense cause the option could also be sandwiched by the same attack. Tagging the sponsor to hear their thoughts @wukong-particle

[d3e4]

@hansfriese I addressed this in L-9 here https://github.com/code-423n4/2023-05-particle-findings/blob/main/data/adriro-Q.md

There's no incentive here other than the grief, and the attacker will only have to pay gas. I believe the report has overinflated severity and should be downgraded to low. The recommendation also doesn't make sense cause the option could also be sandwiched by the same attack. Tagging the sponsor to hear their thoughts @wukong-particle

I completely agree L-9 in #28 is a duplicate.

But what is this idea that there must be an economic incentive? Has there been a change of severity categorisation that I am not aware of? Medium is explicitly meant for when the assets are not at direct risk but for example when availability is impacted. Grief is a very standard Medium attack. The attacker might do it just out of pure spite. The point is to protect users, not to prevent attackers from profiting.

The recommendation cannot be attacked in the same way. If the user sets the lien to not accept any new loan, then as soon as the attacker repays the loan he cannot retake it again, and then the lender can withdraw it.

[hansfriese]

@d3e4 I would invite you to provide precedents that can support your opinion. The economic benefit affects the likelihood of the attack.

But what is this idea that there must be an economic incentive? Has there been a change of severity categorisation that I am not aware of? Medium is explicitly meant for when the assets are not at direct risk but for example when availability is impacted. Grief is a very standard Medium attack. The attacker might do it just out of pure spite. The point is to protect users, not to prevent attackers from profiting.

[hansfriese]

@romeroadrian I missed it, will upgrade L-9.

[romeroadrian]

But what is this idea that there must be an economic incentive? Has there been a change of severity categorisation that I am not aware of? Medium is explicitly meant for when the assets are not at direct risk but for example when availability is impacted. Grief is a very standard Medium attack. The attacker might do it just out of pure spite. The point is to protect users, not to prevent attackers from profiting.

This doesn't make sense at all. A griefer can also spam the network to fill blocks dosing every protocol in the chain. I think you are consistently trying to inflate the severity of your reports using very vague arguments.

[c4-judge]

hansfriese marked the issue as primary issue

[wukong-particle]

We decided leave this grief in this round of contract updating and we will come back to it if there's really large volume of grief happening. We just wanted to emphasize the pure grief nature of this shenanigan since there's no loss or anything associated with it. Thanks!

[d3e4]

@d3e4 I would invite you to provide precedents that can support your opinion. The economic benefit affects the likelihood of the attack.

But what is this idea that there must be an economic incentive? Has there been a change of severity categorisation that I am not aware of? Medium is explicitly meant for when the assets are not at direct risk but for example when availability is impacted. Grief is a very standard Medium attack. The attacker might do it just out of pure spite. The point is to protect users, not to prevent attackers from profiting.

These seem to be griefing with no economic benefit for the attacker: https://github.com/code-423n4/2023-02-ethos-findings/issues/381 https://github.com/code-423n4/2023-01-reserve-findings/issues/384 https://github.com/code-423n4/2023-01-astaria-findings/issues/324 https://github.com/code-423n4/2022-09-nouns-builder-findings/issues/182

I think the main argument would be that if a direct griefing is possible and there is an economic gain for the attacker, this would immediately become High severity as it is a direct compromise on assets. It would equally be High if the grief is permanent after a one-time attack, which is a definitive loss of assets, even though they don't end up with the attacker. In this case the assets are not permanently lost so it doesn't quite reach that High level, but it is the closest step below, and a direct impact on function and availability.