NextGen - Audinarey's results

Lines of code

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

Vulnerability details

Impact

At the end of every auction, the winning bidder's bid is not transferred to the token owners address. This is a direct loss of fund for the token owner.

Proof of Concept

(See Coded POC below)

At the end of the auction when claimAuction(...) function is called, the token is transferred from the ownerOfToken to the highestBidder as shown below, however the the winnig bid is not transferred to the ownerOfToken, but instead it is transferred to the owner/deployer of the AuctionDemo.sol contract.

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); // transfer token to bidder
               // @audit highestBid is transferred to the owner() of AuctionDemo
>              (bool success, ) = payable(owner()).call{value: highestBid}("");
                ...
            } else if (auctionInfoData[_tokenid][i].status == true) {
              ...
            }
        }
    }

This vulnerabiliity is borne from the fact that the AuctionDemo.sol contracts inherits only the ownable contract and its owner() function returns the owner of the contract instead of the owner of the token

Coded POC

Add the test case below to the nextGen.test.js file and run npx hardhat test

context("Funds are sent to wrong owner", () => {
    it("funds are sent to wrong address", async function () {
      const mintedTokenId = parseInt(await contracts.hhCore.viewTokensIndexMin(2) + await contracts.hhCore.viewCirSupply(2));
      
      // 1) mint to auctioner for him to auction
      await contracts.hhMinter.mintAndAuction(
        signers.auctioner, // _recipient
        "mint and auction test token", // _tokenData
        2, // _saltfun_o
        2, // _collectionID
        1799646112 // _auctionEndTime
      )

      // 2) confirm that token recipient is not the address returned by owner() in the Auctiion contract
      expect(await contracts.hhAuction.owner()).to.not.be.equal(signers.auctioner.address)

    })
  })

Tools Used

Manual Foundry

Recommended Mitigation Steps

Modify the claimAuction(...) function by removing

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

and adding

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

as shown below

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); // transfer token to bidder
               // @audit highestBid is transferred to the owner() of AuctionDemo
>              (bool success, ) = payable(ownerOfToken).call{value: highestBid}("");
                ...
            } else if (auctionInfoData[_tokenid][i].status == true) {
              ...
            }
        }
    }

Assessed type

Token-Transfer