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

0 stars 0 forks source link

auctionBuyNft() borrower can block the bidding #17

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#L688

Vulnerability details

Impact

borrower can block the bidding

Proof of Concept

auctionBuyNft() When the bid is successful and there is an extra amount, it will be refunded to borrower The code is as follows:

    function auctionBuyNft(
        Lien calldata lien,
        uint256 lienId,
        uint256 tokenId,
        uint256 amount
    ) external override validateLien(lien, lienId) auctionLive(lien) {
...
        // pay PnL to borrower
        uint256 payback = lien.credit + lien.price - payableInterest - amount;
        if (payback > 0) {
            payable(lien.borrower).transfer(payback);  //<--------@audit payback to borrower
        }    

Using transfer(), a malicious borrower who is a contract can increase the gas consumption of receive(), causing transfer() to revert and thus prevent bidding

Note: withdrawEthWithInterest() has a similar problem

Tools Used

Recommended Mitigation Steps

Add claim mechanism, if transfer fails, it will enter the cliams queue, and borrower will execute claims() to get back the money.

Assessed type

Context

c4-judge commented 1 year ago

hansfriese marked the issue as satisfactory

c4-judge commented 1 year ago

hansfriese marked the issue as duplicate of #21

c4-judge commented 1 year ago

hansfriese marked the issue as duplicate of #31

c4-sponsor commented 1 year ago

wukong-particle marked the issue as sponsor confirmed

wukong-particle commented 1 year ago

Correct finding, correct duplication. Suggestion good as well.

c4-judge commented 1 year ago

hansfriese changed the severity to 3 (High Risk)

wukong-particle commented 1 year ago

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