NextGen - Hama's results

Lines of code

https://github.com/code-423n4/2023-10-nextgen/blob/main/smart-contracts/AuctionDemo.sol#L58 https://github.com/code-423n4/2023-10-nextgen/blob/main/smart-contracts/AuctionDemo.sol#L125

Vulnerability details

Impact

The smart contract governing the auction system is susceptible to bid manipulation, allowing a user to exploit the bidding process and potentially gain an unfair advantage. The issue arises when a participant makes a small initial bid, subsequently places a significantly higher bid so potentially no other user can bid higher, and then cancels the last bid just before the auction concludes so he buy the item with the much lower bid amount. This manipulation can result in the auction system recognizing the earlier, smaller bid as the highest, potentially allowing the user to cancel a larger bid and unfairly influence the auction outcome. The impact includes a compromised auction integrity, potential financial losses for legitimate participants, and an overall degradation of trust in the auction platform.

The vulnerability allows a user to manipulate the bidding process, placing a small initial bid and then canceling a subsequent, larger bid just before the auction concludes. Financial losses may occur for participants who bid competitively and expect a fair auction process. the seller would be prevented from gaining more valid value for his art and may end up selling it for very small money.

Proof of Concept

a user would make a bid from the start which d be a small amount then directly make a very high bid just after it so no other user bid higher

function participateToAuction(uint256 _tokenid) public payable {
    require(msg.value > returnHighestBid(_tokenid) && block.timestamp <= minter.getAuctionEndTime(_tokenid) && minter.getAuctionStatus(_tokenid) == true);
    auctionInfoStru memory newBid = auctionInfoStru(msg.sender, msg.value, true);
    auctionInfoData[_tokenid].push(newBid);
}

just before the end of auction the user cancel his last bid(the very high bid) so the next high bid will be his much lower bid just before the last one

function cancelBid(uint256 _tokenid, uint256 index) public {
    require(block.timestamp <= minter.getAuctionEndTime(_tokenid), "Auction ended");
    require(auctionInfoData[_tokenid][index].bidder == msg.sender && auctionInfoData[_tokenid][index].status == true);
    auctionInfoData[_tokenid][index].status = false;
    (bool success, ) = payable(auctionInfoData[_tokenid][index].bidder).call{value: auctionInfoData[_tokenid][index].bid}("");
    emit CancelBid(msg.sender, _tokenid, index, success, auctionInfoData[_tokenid][index].bid);
}

like this the user will buy the item with a very low price without having users compete against him

Tools Used

manual review

Recommended Mitigation Steps

To mitigate the bid manipulation vulnerability, you can implement a relative bidding mechanism. This mechanism ensures that bid updates must be proportionally higher than the current highest bid, preventing users from manipulating the system by canceling and placing significantly lower bids.

Assessed type

Access Control

Lines of code

https://github.com/code-423n4/2023-10-nextgen/blob/main/smart-contracts/AuctionDemo.sol#L113

Vulnerability details

Impact

The smart contract in question contains a critical issue related to the transfer of Ether during the auction claiming process. When a user claims their auction winnings, Ether is meant to be transferred to the owner of the token being auctioned. However, the current implementation mistakenly transfers Ether to the contract's owner instead. This error can have significant consequences as it results in the incorrect distribution of funds, potentially causing financial losses for the users involved in the auction. Additionally, it could lead to unexpected behavior and disputes regarding the ownership of tokens and funds.

Proof of Concept

function claimAuction(uint256 _tokenid) public WinnerOrAdminRequired(_tokenid,this.claimAuction.selector){
   ..
    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}(""); // should be ownerOfToken
            emit ClaimAuction(owner(), _tokenid, success, highestBid);
        }..
}

Tools Used

manual review

Recommended Mitigation Steps

To address this issue, it is crucial to modify the Ether transfer mechanism in the contract's claim auction function. Specifically, replace owner() with the variable ownerOfToken in the payable call:

(bool success, ) = payable(ownerOfToken).call{value: highestBid}("");

Assessed type

Context