code-423n4 / 2023-10-nextgen-findings

5 stars 3 forks source link

Owner of the token will not receive the funds of the highest bid after an Auction is claimed #1986

Closed c4-submissions closed 7 months ago

c4-submissions commented 7 months ago

Lines of code

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

Vulnerability details

Impact

After the auction is completed, the winner or protocol owner must call the claimAuction function. At this point, the token is sent to the auction participant who made the highest bid, and the funds from that bid are sent to the owner of the protocol. And not to the original owner of the NFT.

Funds should be sent to the original owner of the NFT because this can be understood from the description of bad cases in the project description: "Consider ways in which the owner of the token will not receive the funds of the highest bid after an Auction is claimed."

Proof of Concept

  1. The winner of auction call claimAuction()
  2. Nft transfers from ownerOfNft to winner
    IERC721(gencore).safeTransferFrom(ownerOfToken, highestBidder, _tokenid);
  3. Funds transfers to owner of protocol
    (bool success, ) = payable(owner()).call{value: highestBid}("");

Tools Used

Manual review

Recommended Mitigation Steps

 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}("");
+               (bool success, ) = payable(ownerOfToken).call{value: highestBid}("");
              ...
        }
    }

Assessed type

Other

c4-pre-sort commented 7 months ago

141345 marked the issue as duplicate of #245

c4-judge commented 7 months ago

alex-ppg marked the issue as satisfactory