Bid Amount Sent to Auction Contract Owner in claimAuction Function Instead of NFT Owner

[c4-submissions]

Lines of code

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

Vulnerability details

Impact

The highest bid amount is directed to the auction contract's owner instead of the rightful owner of the NFT.

Vulnerbility Details

The claimAuction function exhibits an issue where the bid amount is erroneously transferred to the owner of the auction contract (owner()) instead of the rightful owner of the auctioned token (ownerOfToken). This issue occurs when the auction winner successfully claims the auction.

Proof of Concept

Find the complete PoC template at https://gist.github.com/zzzuhaibmohd/9bf9d4961472560f1e03ed9a640debd6

for setup run forge init and place the file nextGen.t.sol in test Folder

     function test_BidAmountIsSentWrongOwner() public {
        vm.warp(25 days);

        createCollection("Test Collection 1", 1);

        // 1. Mint a token from collection Id and set to to auction
        minter.mintAndAuction(
            bob, //_recipient
            "Bob the Builder", //_tokenData
            0, //_saltfun_o
            1, //_collectionID
            block.timestamp + 5 days // _auctionEndTime
        );

        uint256 tokenId = 10000000000; // the tokenId of the auctioned NFT

        // 2. Bob provides approval to the auction contract
        vm.prank(bob);
        IERC721(core).approve(address(auction), tokenId);

        // 3. Alice places the bid for tokenId
        vm.prank(alice);
        auction.participateToAuction{value: 2 ether}(tokenId);

        vm.prank(eve);
        auction.participateToAuction{value: 3 ether}(tokenId);

        // 5. The admin calls the claimAuction transaction
        vm.warp(30 days);

        uint bob_balance_before_sale = bob.balance;
        uint auction_owner_balance_before_sale = auction.owner().balance;

        assertEq(core.ownerOf(tokenId), bob);
        auction.claimAuction(tokenId);
        assertEq(core.ownerOf(tokenId), eve);

        // 6. The ISSUE
        //Funds are sent to the auction.owner() instead of bob who is the owner of the NFT
        assertEq(
            auction.owner().balance,
            auction_owner_balance_before_sale +
                auction.returnHighestBid(tokenId)
        );
        assertEq(bob.balance, bob_balance_before_sale);
    }

Tools Used

Foundry

Recommended Mitigation Steps

To fix this issue, the bid amount should be sent to the correct recipient, namely the owner of the auctioned token (ownerOfToken). Below is the modified code snippet highlighting the necessary correction:

The Fix:

function claimAuction(uint256 _tokenid) public WinnerOrAdminRequired(_tokenid,this.claimAuction.selector){
    // the code
    uint256 highestBid = returnHighestBid(_tokenid);
    address ownerOfToken = IERC721(gencore).ownerOf(_tokenid);
    address highestBidder = returnHighestBidder(_tokenid);
    //console.log(auctionInfoData[_tokenid].length);
    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}("");
            emit ClaimAuction(owner(), _tokenid, success, highestBid);
            //rest of the code

}

Assessed type

Access Control

[c4-pre-sort]

141345 marked the issue as duplicate of #245

[c4-judge]

alex-ppg marked the issue as satisfactory

[c4-judge]

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