code-423n4 / 2023-05-particle-findings

0 stars 0 forks source link

Attacker can spam `addCredit()` function to cause a denial-of-service during an auction #8

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

The ParticleExchange contract does not store any data about the lien in the contract storage. Instead, users must send the entire Lien struct when interacting with any existing lien, and the contract checks if the hash of the struct is correct. This poses a problem because normal users must know the lien information when it is not stored on-chain.

function _validateLien(Lien calldata lien, uint256 lienId) internal view returns (bool) {
    return liens[lienId] == keccak256(abi.encode(lien));
}

As discussed with the sponsor on Discord, the lien information is stored in a database. A service job listens to on-chain events to update the database every minute on average.

On the other hand, the addCredit() function is public and allows anyone to add any amount of ETH to the credit of any active loan. This is where the issue occurs. Since users need to know the lien information when interacting with the protocol, an attacker can simply spam the addCredit() function with tiny amounts of credit. The purpose of this is to continuously change the lien information, causing other users' transactions to revert because they submit the wrong lien information. The issue can become a big problem during an auction. Lenders can make borrowers or other users unable to interact with the lien, allowing them to liquidate the loan.

Impact

The impact of this issue is a denial-of-service (DoS) attack during an auction. The attacker can keep changing the lien information, making it impossible for other users to interact with the lien.

Proof of Concept

Consider the scenario

  1. Alice (attacker - lender) has supplied a NFT with price = 10 ETH but the floor price of it is only 7 ETH. So she wants to liquidate anyone borrow her NFT to sell her NFT for 10 ETH.
  2. Bob (victim - borrower) took the offer of Alice and deposit 10 ETH + credit.
  3. Alice calls startLoanAuction() as soon as possible to get that 10 ETH. During auction, she keeps calling addCredit() to the loan with tiny amount (1, 2 wei).
  4. No normal users could interact with the lien to auctionBuyNft() or to repayWithNft() since the lien info keeps changing continously. After auction duration, Alice is able to withdraw 10 ETH even.

Tools Used

Manual Review

Recommended Mitigation Steps

There are 2 fixes for this issues

  1. Adding a mininum requirement for addCredit() function.
  2. Only allow borrowers to call addCredit() for their loan.

Assessed type

DoS

c4-judge commented 1 year ago

hansfriese marked the issue as satisfactory

c4-judge commented 1 year ago

hansfriese marked the issue as primary issue

c4-judge commented 1 year ago

hansfriese marked the issue as duplicate of #16

wukong-particle commented 1 year ago

Judge is correct, indeed duplication. Judge nails the simplest issue as primary, we will do the fix in https://github.com/code-423n4/2023-05-particle-findings/issues/16#issuecomment-1579327687.

c4-sponsor commented 1 year ago

wukong-particle marked the issue as sponsor confirmed

c4-judge commented 1 year ago

hansfriese changed the severity to 2 (Med Risk)

wukong-particle commented 1 year ago

Fixed with https://github.com/Particle-Platforms/particle-exchange-protocol/pull/14