Re-enterancy in AuctionDemo contract

[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#L124

Vulnerability details

Impact

claimAuction function of AuctionDemo contract transfers the token to highest bidder (winner) and the bid amount is transferred to the owner. Moreover, refund is sent to all remaining participants (non-winners) of the auction via call function. However, if a non-winner is a contract, then it can invoke the cancelBid function when refund is sent to it. This way, the attacker can get the refund twice; once by the claimAuction function and secondly by cancelBid function.

Proof of Concept

1. Attacker places a bid and tries to not become the winner.
2. Winner or Admin calls claimAuction function exactly on the auction end time (i.e block.timestamp == AuctionEndTime), this causes the following condition to become true :- require(block.timestamp >= minter.getAuctionEndTime(_tokenid)...
3. Refund is sent to attacker via call and the attacker invokes the cancelBid function and his bid is cancelled as following condition is also still true :-

function cancelBid(uint256 _tokenid, uint256 index) public { require(block.timestamp <= minter.getAuctionEndTime(_tokenid), "Auction ended");

4. Attacker's bid gets cancelled and his amount is again refunded to him at line 128 (bool success, ) = payable(auctionInfoData[_tokenid][index].bidder).call{value: auctionInfoData[_tokenid][index].bid}("");
5. This allows attacker to get his bid amount refunded twice.

Same is also

Tools Used

Manual Review

Recommended Mitigation Steps

Line 115 should change the status to false before sending refund } else if (auctionInfoData[_tokenid][i].status == true) {

Moreover, remove equality from the condition in cancelBid function require(block.timestamp < minter.getAuctionEndTime(_tokenid), "Auction ended"); or remove equality from the condition in claimAuction function require(block.timestamp > minter.getAuctionEndTime(_tokenid)...

Assessed type

Reentrancy

[c4-pre-sort]

141345 marked the issue as duplicate of #962

[c4-judge]

alex-ppg marked the issue as duplicate of #1323

[c4-judge]

alex-ppg marked the issue as partial-50

[c4-judge]

alex-ppg marked the issue as full credit

[c4-judge]

alex-ppg marked the issue as partial-50