Borrower can block being defaulted or auctioned

[code423n4]

Lines of code

https://github.com/code-423n4/2023-05-particle/blob/main/contracts/protocol/ParticleExchange.sol#L216 https://github.com/code-423n4/2023-05-particle/blob/main/contracts/protocol/ParticleExchange.sol#L744

Vulnerability details

Borrower can block being defaulted or auctioned

The borrower can potentially block the liquidation and auction processed by using a contract and reverting on ETH transfers.

Impact

When a loan is being liquidated or auctioned, any credit still available to the borrower (i.e. margin discounting the lender's interests) needs to be refunded. This is present in the withdrawEthWithInterest() function, that can be used to liquidate a position, and in the auctionBuyNft() function which is used to auction a loan.

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

192:     function withdrawEthWithInterest(Lien calldata lien, uint256 lienId) external override validateLien(lien, lienId) {
             ...
214:         // transfer PnL to borrower
215:         if (lien.credit > payableInterest) {
216:             payable(lien.borrower).transfer(lien.credit - payableInterest);
217:         }
             ...

https://github.com/code-423n4/2023-05-particle/blob/main/contracts/protocol/ParticleExchange.sol#L688-L748

688:     function auctionBuyNft(
689:         Lien calldata lien,
690:         uint256 lienId,
691:         uint256 tokenId,
692:         uint256 amount
693:     ) external override validateLien(lien, lienId) auctionLive(lien) {
             ...
741:         // pay PnL to borrower
742:         uint256 payback = lien.credit + lien.price - payableInterest - amount;
743:         if (payback > 0) {
744:             payable(lien.borrower).transfer(payback);
745:         }
             ...

As we can see in both of these cases, if the borrower still has some credit left, this is refunded by using the transfer() function. This is problematic, as the transfer() function reverts on failure. If the receiver is a contract, then this failure can be easily manipulated and forced.

This means that the borrower can use a contract to take the loan and arbitrarily decide to revert when the payback is being refunded, effectively blocking the liquidation and auction processes.

Proof of concept

The attacker can create a simple contract to interact with the Particle exchange. This contract has a receive() function that can be configured to revert.

function receive() external payable {
  if (_shouldRevert) {
    revert();
  }
}

The attacker then takes loans using this contract as an interface, and can block at will auctions or liquidations by setting _shouldRevert to true.

Recommendation

Similar to how interests are accrued for the lender, prefer a pull over push process to have the receiver pull ETH instead of pushing it in the middle of a transaction. The withdrawEthWithInterest() and auctionBuyNft() functions should accumulate amounts corresponding to the borrower in internal storage and then offer a function to withdraw these by separate.

Assessed type

DoS

[c4-judge]

hansfriese marked the issue as satisfactory

[c4-judge]

hansfriese changed the severity to 3 (High Risk)

[c4-judge]

hansfriese marked the issue as primary issue

[c4-judge]

hansfriese marked the issue as duplicate of #31

[wukong-particle]

Judge is correct, indeed duplication

[wukong-particle]

will update our contract following the pull based approach to aggregate gains to trader account level

[c4-sponsor]

wukong-particle marked the issue as sponsor confirmed

[wukong-particle]

will update our contract following the pull based approach to aggregate gains to trader account level

^ fixed with https://github.com/Particle-Platforms/particle-exchange-protocol/pull/13