NextGen - twcctop's results

Lines of code

https://github.com/code-423n4/2023-10-nextgen/blob/a6f2397b68ef2865374c1bf7629349f25e71a44d/smart-contracts/AuctionDemo.sol#L134-L143 https://github.com/code-423n4/2023-10-nextgen/blob/a6f2397b68ef2865374c1bf7629349f25e71a44d/smart-contracts/AuctionDemo.sol#L57-L61 https://github.com/code-423n4/2023-10-nextgen/blob/a6f2397b68ef2865374c1bf7629349f25e71a44d/smart-contracts/AuctionDemo.sol#L124-L130

Vulnerability details

Impact

The current auction mechanism has undermined the motivation of the auction and turned it into a mev game. User may don't have enough motivation to participate in the auction.

Proof of Concept

Bidder can cancel his bid and get his token before auction end.

   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);
    }

In participateToAuction , user need to take a bid higher than current highest bid.

function participateToAuction(uint256 _tokenid) public payable {
        require(msg.value > returnHighestBid(_tokenid) && block.timestamp <= minter.getAuctionEndTime(_tokenid) && minter.getAuctionStatus(_tokenid) == true);
 ..

Suppose a situation, at the beginning of an auction, malicious user take bid at a extreme high value. During the auction ,other user see the bidder price is so high, and he can not participate ,because lower bid is not allowed.Nearly at the end of auction time,malicious user can cancel the bid ,and take bid with a very small value , due to there is no other competitors，he will become the final winner. Or another situation, other mev player notice the malicious user's small value bid ,and front run,this auction will finally become a mev game.

Tools Used

manual

Recommended Mitigation Steps

consider redesign the auction mechanism

Assessed type

MEV

Lines of code

https://github.com/code-423n4/2023-10-nextgen/blob/2467db02cc446374ab9154dde98f7e931d71584d/smart-contracts/MinterContract.sol#L540 https://github.com/code-423n4/2023-10-nextgen/blob/2467db02cc446374ab9154dde98f7e931d71584d/smart-contracts/MinterContract.sol#L566

Vulnerability details

Impact

"getPrice" returns an unexpectedly high value at the end time, it's very unfair to users who mint at the end time.

Proof of Concept

When users mint, getPrice is called to get current nft price. In saleoption 2,with time goes by, the price will decrease.The issue is at the moment of endtime,the price will become extremely high. block.timestamp < collectionPhases[_collectionId].publicEndTime Notice this is < ,which should be <=. Due to this logic, the price at end will become return collectionPhases[_collectionId].collectionMintCost;,which suddenly become high.

poc:
Create a new file and paste it.

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

let signers
let contracts

describe("NextGen Tests", function () {
  before(async function () {
    ;({signers, contracts} = await loadFixture(fixturesDeployment))
  })

  context("Get Price decrease", () => {
    it.only("#Get Price decrease", async function () {

      await setEnviorment();

      const priceAtWei = await contracts.hhMinter.getPrice(
          2, // _collectionID
      )
      const priceCurrent = ethers.formatEther(priceAtWei);

      console.log("priceCurrent",priceCurrent);
      const currenttime = await time.latest();
      console.log("currenttime",currenttime);




      await time.increaseTo(1796931278 -1)
      const priceAtNearlyEndWei = await contracts.hhMinter.getPrice(
          2, // _collectionID
      )
      const priceAtNearlyEnd = ethers.formatEther(priceAtNearlyEndWei);
      console.log("priceAtNearlyEnd",priceAtNearlyEnd);


      await time.increaseTo(1796931278)
      const priceAtEndWei = await contracts.hhMinter.getPrice(
          2, // _collectionID
      )
      const priceAtEnd = ethers.formatEther(priceAtEndWei);
      console.log("priceAtEnd",priceAtEnd);

    })
  })
})


  async function setEnviorment() {

    await contracts.hhCore.createCollection(
        "Test Collection 1",
        "Artist 1",
        "For testing",
        "www.test.com",
        "CCO",
        "https://ipfs.io/ipfs/hash/",
        "",
        ["desc"],
    )

    await contracts.hhCore.createCollection(
        "Test Collection 2",
        "Artist 2",
        "For testing",
        "www.test.com",
        "CCO",
        "https://ipfs.io/ipfs/hash/",
        "",
        ["desc"],
    )

    await contracts.hhCore.createCollection(
        "Test Collection 3",
        "Artist 3",
        "For testing",
        "www.test.com",
        "CCO",
        "https://ipfs.io/ipfs/hash/",
        "",
        ["desc"],
    )




    await contracts.hhAdmin.registerCollectionAdmin(
        1,
        signers.addr1.address,
        true,
    )


    await contracts.hhAdmin.registerCollectionAdmin(
        3,
        signers.addr1.address,
        true,
    )


    await contracts.hhCore.connect(signers.addr1).setCollectionData(
        1, // _collectionID
        signers.addr1.address, // _collectionArtistAddress
        2, // _maxCollectionPurchases
        10000, // _collectionTotalSupply
        0, // _setFinalSupplyTimeAfterMint
    )


    await contracts.hhCore.setCollectionData(
        2, // _collectionID
        signers.addr1.address, // _collectionArtistAddress
        1, // _maxCollectionPurchases
        100, // _collectionTotalSupply
        1000, // _setFinalSupplyTimeAfterMint
    )


    //
    await contracts.hhCore.setCollectionData(
        3, // _collectionID
        signers.addr1.address, // _collectionArtistAddress
        1, // _maxCollectionPurchases
        100, // _collectionTotalSupply
        1000, // _setFinalSupplyTimeAfterMint
    )

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


    await contracts.hhCore.addRandomizer(
        2, contracts.hhRandomizer,
    )

    await contracts.hhCore.addRandomizer(
        3, contracts.hhRandomizer,
    )




    await contracts.hhMinter.setCollectionCosts(
        2, // _collectionID
        BigInt(1000000000000000000), // _collectionMintCost 1 eth
        BigInt(100000000000000000), // _collectionEndMintCost 0.1 eth
        BigInt(100000000000000000), // _rate
        200, // _timePeriod
        2, // _salesOptions
        '0xD7ACd2a9FD159E69Bb102A1ca21C9a3e3A5F771B', // delAddress
    )



    await contracts.hhMinter.setCollectionPhases(
        2, // _collectionID
        1698138500, // _allowlistStartTime
        1698138500, // _allowlistEndTime
        1698138500, // _publicStartTime
        1796931278, // _publicEndTime
        "0x8e3c1713145650ce646f7eccd42c4541ecee8f07040fc1ac36fe071bbfebb870", // _merkleRoot
    )

  }

logs:

  NextGen Tests
    Get Price decrease
priceCurrent 0.1
currenttime 1699414629
priceAtNearlyEnd 0.1
priceAtEnd 1.0
      ✔ #Get Price decrease (229ms)

  1 passing (2s)

compare prices at 1796931278 -1 and 1796931278, One is 1 another is '0.1`

Tools Used

manual

Recommended Mitigation Steps

change <' to <=`

Assessed type

Context

Lines of code

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

Vulnerability details

Impact

erc721 safeTransferFrom may dos claimAuction. It is unknown if the receiving address is contract or not and implements the onERC721Received function or not. If it is smart contract, it will dos the claim logic

Proof of Concept

claimAuction is to transfer nft to the auction winner. When participating in aution, it never check bidder is eoa or smart-contract. If the winner is smart-contract and do not have _onERC721Received(), according to the erc721 safeTransferFrom logic, it will revert. The claim logic, not only contains the transfer logic, but also repay token to other bidder, will dos. I think it's a serious problem.

Tools Used

manual

Recommended Mitigation Steps

check bidder in participateToAuction, if the bidder is smart contract ,check it has _onERC721Received() or not

Assessed type

ERC721