# Lines of code https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/AuctionDemo.sol#L58 https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/AuctionDemo.sol#L105 https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/AuctionDemo.sol#L125 https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/AuctionDemo.sol#L135 https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/AuctionDemo.sol#L113 https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/AuctionDemo.sol#L116 # Vulnerability details ## Impact As described in the `Proof of Concept` section below, a cunning user is able to always win the auction and claim the auctioned token for free. ## Proof of Concept The file `AuctionDemo.sol` contains the functions: `participateToAuction`, `claimAuction`, `cancelBid` and `cancelAllBids`: - `participateToAuction` function is intended to be used **when the auction is OPEN** - `claimAuction` function is intended to be used **when the auction is CLOSED** - `cancelBid` function is intended to be used **when the auction is OPEN** - `cancelAllBids` function is intended to be used **when the auction is OPEN** However, based on the implementations of these functions, exactly at the moment `block.timestamp == minter.getAuctionEndTime(_tokenid)`, **the auction is both OPEN and CLOSED at the same time**. The snippets from these functions allowing an auction to be OPEN and CLOSED at the same time are given below: - `participateToAuction` function: https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/AuctionDemo.sol#L58C59-L58C112 - `claimAuction` function: https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/AuctionDemo.sol#L105C17-L105C70 - `cancelBid` function: https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/AuctionDemo.sol#L125C17-L125C70 - `cancelAllBids` function: https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/AuctionDemo.sol#L135C17-L135C70 The root of the issue is that non-strict inequalities are used: `>=` and `<=` instead of strict inequalities: `>` and `<` when `block.timestamp` and `minter.getAuctionEndTime(_tokenid)` are compared. The fact that at `block.timestamp == minter.getAuctionEndTime(_tokenid)` the auction is both OPEN and CLOSED can be exploited and the following scenario is possible: 1) Alice calls the `participateToAuction` function and makes the highest bid up to the moment for the token being auctioned. The transaction becomes part of a block for which `block.timestamp == minter.getAuctionEndTime(_tokenid)` is `true`. 2) Alice calls the `claimAuction` function in a transaction that is part of a block for which `block.timestamp == minter.getAuctionEndTime(_tokenid)` is `true`. By calling the `claimAuction` function, Alice wins the auction (she has the highest bid) and gets the token. Because the return value of the low-level `call` function is not checked in the following 2 places: - https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/AuctionDemo.sol#L113C17-L113C81 - https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/AuctionDemo.sol#L116C17-L116C131 there is a possibility that the call wasn't successful, but there is no revert. **In case these `call` function invocations fail, bidders and the original owner of the token being auctioned will not receive `ETH` and some `ETH` amount will remain inside the `auctionDemo` contract**. 3) Alice calls the `cancelBid` function in a transaction that is part of a block for which `block.timestamp == minter.getAuctionEndTime(_tokenid)` is `true`. As a result of the `cancelBid` function invocation, Alice can claim the `ETH` amount from her winning bid back (this might happen if the `auctionDemo` contract has enough `ETH`, which can be possible if there are failing `call` function invocations in the `claimAuction` function beforehand, which is possible, since the return values of the `call` function invocations in `claimAuction` are not checked). To recap, following this strategy: - Alice always wins the token auction, because she makes the winning bid at the last possible moment (`block.timestamp == minter.getAuctionEndTime(_tokenid)`). Since the auction is both OPEN and CLOSED at that moment, she is able to claim the token being auctioned right after making her winning bid. - Alice takes the `ETH` amount from her winning bid back by cancelling her winning bid at the moment `block.timestamp == minter.getAuctionEndTime(_tokenid)`, right after claiming the auctioned token. So, Alice managed to win the auction and get the auctioned token for free. ## Tools Used Manual review. ## Recommended Mitigation Steps To mitigate the possibility of a cunning user always winning the auction, we should make sure that in no moment in time is it possible the auction to be both OPEN and CLOSED. This can be achieved by modifying the `claimAuction` function in the way shown below: ```solidity function claimAuction(uint256 _tokenid) public WinnerOrAdminRequired(_tokenid,this.claimAuction.selector) { >>> require(block.timestamp > minter.getAuctionEndTime(_tokenid) && auctionClaim[_tokenid] == false && minter.getAuctionStatus(_tokenid) == true); <<< auctionClaim[_tokenid] = true; uint256 highestBid = returnHighestBid(_tokenid); address ownerOfToken = IERC721(gencore).ownerOf(_tokenid); address highestBidder = returnHighestBidder(_tokenid); for (uint256 i=0; i< auctionInfoData[_tokenid].length; i ++) { if (auctionInfoData[_tokenid][i].bidder == highestBidder && auctionInfoData[_tokenid][i].bid == highestBid && auctionInfoData[_tokenid][i].status == true) { IERC721(gencore).safeTransferFrom(ownerOfToken, highestBidder, _tokenid); (bool success, ) = payable(owner()).call{value: highestBid}(""); >>>>> require(success, "ETH transfer failed"); <<<<< emit ClaimAuction(owner(), _tokenid, success, highestBid); } else if (auctionInfoData[_tokenid][i].status == true) { (bool success, ) = payable(auctionInfoData[_tokenid][i].bidder).call{value: auctionInfoData[_tokenid][i].bid}(""); >>>>> require(success, "ETH transfer failed"); <<<<< emit Refund(auctionInfoData[_tokenid][i].bidder, _tokenid, success, highestBid); } else {} } } ``` The essential change is on the line starting with `>>>` and ending with `<<<` and the change consists of replacing `block.timestamp >= minter.getAuctionEndTime(_tokenid)` with `block.timestamp > minter.getAuctionEndTime(_tokenid)`. This way a cunning user cannot make the winning bid and win the auction in the same block. This fix also ensures that the auction winner cannot take his winning bid back, since the `cancelBid` function cannot be successfully called after the `claimAuction` function, because `cancelBid` requires the auction to be OPEN and `claimAuction` requires the auction to be CLOSED and the fix ensures there is no moment in time when the auction is both OPEN and CLOSED. So, the `cancelBid` function can be called successfully only before the `claimAuction` function. For completeness, I've also added checks for the success of the `ETH` transfer in the mitigated code snippet above, although that's something already known from the bot report. The lines of the checks added start with `>>>>>` and end with `<<<<<`. ## Assessed type Timing


Code4rena	NextGen	NextGen	30/10/2023	13/11/2023	14 days	$49,250 USDC	242/243	$0.00	1	1	0	0	Completed
Code4rena	Venus Prime	Venus Protocol	28/09/2023	04/10/2023	6 days	$36,500 USDC	60/115	$32.27	1	0	1	0	Completed
Code4rena	Dopex	Dopex	21/08/2023	06/09/2023	16 days	$125,000 USDC	128/189	$19.24	2	1	0	Grade B	Completed

dethera