Highest bidder can frontrun late bidder and prevent their refund

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/AuctionDemo.sol#L104
https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/AuctionDemo.sol#L57

Vulnerability details

Impact

The highest bidder can frontrun anyone that waits until the end of the auction to bid(block.timestamp == minter.getAuctionEndTime(_tokenid)).

The current highest bidder can frontrun any bids at the deadline with a call to claimAuction(). The bids will still succeed and the new highest bidder will not be able to refund and will not be able to claim their NFT.

Proof of Concept

In this example Alice is the highest bidder and Bob waits until the end to bid.

1. Alices places a bid

// Alice places a bid of 1 ether
vm.prank(alice);
auction.participateToAuction{value: 1 ether}(tokenIndex);

Advance to the end of the auction

2a. Bob places a bid at the deadline

// Bob places a bid of 2 ether
vm.prank(bob);
auction.participateToAuction{value: 2 ether}(tokenIndex);

2b. Alice frontruns bob with a call to claimAuction()

vm.prank(alice);
auction.claimAuction(tokenIndex);

Alice has recieved the NFT
Bob has not recieved the NFT and has no way to refund/cancel his bid

Tools Used

Foundry

Recommended Mitigation Steps

It should not be possible to bid at the same time as calling claimAuction()

To solve this we can change claimAuction so that it can only be called after the end of the auction and not at the end.

-    require(block.timestamp >= minter.getAuctionEndTime(_tokenid) && auctionClaim[_tokenid] == false && minter.getAuctionStatus(_tokenid) == true);

+    require(block.timestamp > minter.getAuctionEndTime(_tokenid) && auctionClaim[_tokenid] == false && minter.getAuctionStatus(_tokenid) == true);

Assessed type

Timing

[c4-pre-sort]

141345 marked the issue as duplicate of #962

[c4-judge]

alex-ppg marked the issue as not a duplicate

[c4-judge]

alex-ppg marked the issue as duplicate of #1926

[c4-judge]

alex-ppg marked the issue as partial-50

[c4-judge]

alex-ppg changed the severity to 2 (Med Risk)