Platform: Code4rena
Start Date: 06/09/2022
Pot Size: $90,000 USDC
Total HM: 33
Participants: 168
Period: 9 days
Judge: GalloDaSballo
Total Solo HM: 10
Id: 157
League: ETH
Rank: 33/168
Findings: 3
Award: $602.47
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: zzzitron
Also found by: 0xSmartContract, ChristianKuri, ElKu, Lambda, MiloTruck, davidbrai, elad, hansfriese, immeas, ladboy233, scaraven, volky
https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/auction/Auction.sol#L206 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/token/Token.sol#L113 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/token/Token.sol#L157
Detailed description of the impact of this finding.
when creating the auction, the auction contract first mint nft and hold the NFT then when the auction is settled, nft is transfered out to the user.
/// @dev Creates an auction for the next token function _createAuction() private { // Get the next token available for bidding try token.mint() returns (uint256 tokenId) {
when token.mint() is called, the nft is supposed to be minted.
but when token.mint is called.
// Cannot realistically overflow unchecked { do { // Get the next token to mint tokenId = settings.totalSupply++; // Lookup whether the token is for a founder, and mint accordingly if so } while (_isForFounder(tokenId)); }
note this while loop,
while (_isForFounder(tokenId));
inside _isForFounder(tokenId)
/// @dev Checks if a given token is for a founder and mints accordingly /// @param _tokenId The ERC-721 token id function _isForFounder(uint256 _tokenId) private returns (bool) { // Get the base token id uint256 baseTokenId = _tokenId % 100; // If there is no scheduled recipient: if (tokenRecipient[baseTokenId].wallet == address(0)) { return false; // Else if the founder is still vesting: } else if (block.timestamp < tokenRecipient[baseTokenId].vestExpiry) { // Mint the token to the founder _mint(tokenRecipient[baseTokenId].wallet, _tokenId); return true; // Else the founder has finished vesting: } else { // Remove them from future lookups delete tokenRecipient[baseTokenId]; return false; } }
we check that if this nft token id belongs to the founder, we mint to founders,
We assign nft token id to founder in the _addFounder function
// For each token to vest: for (uint256 j; j < founderPct; ++j) { // Get the available token id baseTokenId = _getNextTokenId(baseTokenId); // Store the founder as the recipient tokenRecipient[baseTokenId] = newFounder; emit MintScheduled(baseTokenId, founderId, newFounder); // Update the base token id (baseTokenId += schedule) % 100; }
the FounderPct, can be 100 at most.
here is the key,
if token id from 0 - 99 all belongs to founder,
then if a user create auction and call mint, he has to pay the gas to mint 100 nft for the founders.
minting nft for 100 times is either very expensive, gas consuming or the transaction is running out of gas.
Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept.
Note in the test
function setMockFounderParams() internal virtual { address[] memory wallets = new address[](2); uint256[] memory percents = new uint256[](2); uint256[] memory vestingEnds = new uint256[](2); wallets[0] = founder; wallets[1] = founder2; percents[0] = 10; percents[1] = 5; vestingEnds[0] = 4 weeks; vestingEnds[1] = 4 weeks; setFounderParams(wallets, percents, vestingEnds); }
the founder1 owns 10 nft out of 100, founder2 owns 5 nft out of 100.
We can modify the test as our POC.
we change 10 and 5 to 50 and 50
function setMockFounderParams() internal virtual { address[] memory wallets = new address[](2); uint256[] memory percents = new uint256[](2); uint256[] memory vestingEnds = new uint256[](2); wallets[0] = founder; wallets[1] = founder2; percents[0] = 50; percents[1] = 50; vestingEnds[0] = 4 weeks; vestingEnds[1] = 4 weeks; setFounderParams(wallets, percents, vestingEnds); }
in Token.t.sol,
https://github.com/code-423n4/2022-09-nouns-builder/blob/main/test/Token.t.sol
add the import
import "forge-std/console.sol";
and the test
function test_FounderSettings_POC() public { deployMock(); TokenTypesV1.Founder[] memory founderSettings = token.getFounders(); for(uint256 i; i < founderSettings.length; ++i) { console.log('founder wallet', founderSettings[i].wallet); console.log('ownership pct', founderSettings[i].ownershipPct); console.log('vest expiry', founderSettings[i].vestExpiry); console.log(""); } }
we run
forge test -vv --match FounderSettings_POC
the result is
Running 1 test for test/Token.t.sol:TokenTest [PASS] test_FounderSettings_POC() (gas: 2834475) Logs: founder wallet 0xd3562Fd10840f6bA56112927f7996B7c16edFCc1 ownership pct 50 vest expiry 2419200 founder wallet 0xA7cBf566E80C4A1Df2C4aE965c79FB087f25E4bF ownership pct 50 vest expiry 2419200
this is expect.
we can run
forge test -vvvv --match FounderSettings_POC
to track the event emissioned.
we see
emit MintScheduled(baseTokenId: 0, founderId: 0, founder: (0xd3562fd10840f6ba56112927f7996b7c16edfcc1, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 2, founderId: 0, founder: (0xd3562fd10840f6ba56112927f7996b7c16edfcc1, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 4, founderId: 0, founder: (0xd3562fd10840f6ba56112927f7996b7c16edfcc1, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 6, founderId: 0, founder: (0xd3562fd10840f6ba56112927f7996b7c16edfcc1, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 8, founderId: 0, founder: (0xd3562fd10840f6ba56112927f7996b7c16edfcc1, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 10, founderId: 0, founder: (0xd3562fd10840f6ba56112927f7996b7c16edfcc1, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 12, founderId: 0, founder: (0xd3562fd10840f6ba56112927f7996b7c16edfcc1, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 14, founderId: 0, founder: (0xd3562fd10840f6ba56112927f7996b7c16edfcc1, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 16, founderId: 0, founder: (0xd3562fd10840f6ba56112927f7996b7c16edfcc1, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 18, founderId: 0, founder: (0xd3562fd10840f6ba56112927f7996b7c16edfcc1, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 20, founderId: 0, founder: (0xd3562fd10840f6ba56112927f7996b7c16edfcc1, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 22, founderId: 0, founder: (0xd3562fd10840f6ba56112927f7996b7c16edfcc1, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 24, founderId: 0, founder: (0xd3562fd10840f6ba56112927f7996b7c16edfcc1, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 26, founderId: 0, founder: (0xd3562fd10840f6ba56112927f7996b7c16edfcc1, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 28, founderId: 0, founder: (0xd3562fd10840f6ba56112927f7996b7c16edfcc1, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 30, founderId: 0, founder: (0xd3562fd10840f6ba56112927f7996b7c16edfcc1, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 32, founderId: 0, founder: (0xd3562fd10840f6ba56112927f7996b7c16edfcc1, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 34, founderId: 0, founder: (0xd3562fd10840f6ba56112927f7996b7c16edfcc1, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 36, founderId: 0, founder: (0xd3562fd10840f6ba56112927f7996b7c16edfcc1, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 38, founderId: 0, founder: (0xd3562fd10840f6ba56112927f7996b7c16edfcc1, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 40, founderId: 0, founder: (0xd3562fd10840f6ba56112927f7996b7c16edfcc1, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 42, founderId: 0, founder: (0xd3562fd10840f6ba56112927f7996b7c16edfcc1, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 44, founderId: 0, founder: (0xd3562fd10840f6ba56112927f7996b7c16edfcc1, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 46, founderId: 0, founder: (0xd3562fd10840f6ba56112927f7996b7c16edfcc1, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 48, founderId: 0, founder: (0xd3562fd10840f6ba56112927f7996b7c16edfcc1, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 50, founderId: 0, founder: (0xd3562fd10840f6ba56112927f7996b7c16edfcc1, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 52, founderId: 0, founder: (0xd3562fd10840f6ba56112927f7996b7c16edfcc1, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 54, founderId: 0, founder: (0xd3562fd10840f6ba56112927f7996b7c16edfcc1, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 56, founderId: 0, founder: (0xd3562fd10840f6ba56112927f7996b7c16edfcc1, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 58, founderId: 0, founder: (0xd3562fd10840f6ba56112927f7996b7c16edfcc1, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 60, founderId: 0, founder: (0xd3562fd10840f6ba56112927f7996b7c16edfcc1, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 62, founderId: 0, founder: (0xd3562fd10840f6ba56112927f7996b7c16edfcc1, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 64, founderId: 0, founder: (0xd3562fd10840f6ba56112927f7996b7c16edfcc1, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 66, founderId: 0, founder: (0xd3562fd10840f6ba56112927f7996b7c16edfcc1, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 68, founderId: 0, founder: (0xd3562fd10840f6ba56112927f7996b7c16edfcc1, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 70, founderId: 0, founder: (0xd3562fd10840f6ba56112927f7996b7c16edfcc1, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 72, founderId: 0, founder: (0xd3562fd10840f6ba56112927f7996b7c16edfcc1, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 74, founderId: 0, founder: (0xd3562fd10840f6ba56112927f7996b7c16edfcc1, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 76, founderId: 0, founder: (0xd3562fd10840f6ba56112927f7996b7c16edfcc1, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 78, founderId: 0, founder: (0xd3562fd10840f6ba56112927f7996b7c16edfcc1, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 80, founderId: 0, founder: (0xd3562fd10840f6ba56112927f7996b7c16edfcc1, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 82, founderId: 0, founder: (0xd3562fd10840f6ba56112927f7996b7c16edfcc1, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 84, founderId: 0, founder: (0xd3562fd10840f6ba56112927f7996b7c16edfcc1, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 86, founderId: 0, founder: (0xd3562fd10840f6ba56112927f7996b7c16edfcc1, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 88, founderId: 0, founder: (0xd3562fd10840f6ba56112927f7996b7c16edfcc1, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 90, founderId: 0, founder: (0xd3562fd10840f6ba56112927f7996b7c16edfcc1, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 92, founderId: 0, founder: (0xd3562fd10840f6ba56112927f7996b7c16edfcc1, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 94, founderId: 0, founder: (0xd3562fd10840f6ba56112927f7996b7c16edfcc1, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 96, founderId: 0, founder: (0xd3562fd10840f6ba56112927f7996b7c16edfcc1, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 98, founderId: 0, founder: (0xd3562fd10840f6ba56112927f7996b7c16edfcc1, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 1, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 3, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 5, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 7, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 9, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 11, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 13, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 15, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 17, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 19, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 21, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 23, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 25, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 27, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 29, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 31, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 33, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 35, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 37, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 39, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 41, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 43, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 45, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 47, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 49, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 51, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 53, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 55, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 57, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 59, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 61, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 63, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 65, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 67, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 69, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 71, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 73, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 75, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 77, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 79, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 81, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 83, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 85, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 87, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 89, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 91, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 93, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 95, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 97, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 50, 2419200)) │ │ │ │ ├─ emit MintScheduled(baseTokenId: 99, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 50, 2419200))
this shows that nft id 0 - 99 and belongs to founders and need to be minted before the nft is minted for auction. this means, that user creation auction and call mint, he has to mint 100 nft token for founders.
Foundry Manual Review
we can set limit on founderPct
or when adding founders,
instead of % 100, we recommend using a large number, such as % 1000.
Or we can build a separate smart contract for founder nft distriubtion.
we can say,
for every nft id that is % 10 = 0,
mint nft to a founder nft distribution contract,
then nft founder can claim the nft.
#0 - GalloDaSballo
2022-09-20T19:47:15Z
🌟 Selected for report: Jeiwan
Also found by: imare, ladboy233, rvierdiiev
492.6103 USDC - $492.61
https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/token/Token.sol#L172 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/token/metadata/MetadataRenderer.sol#L201
Detailed description of the impact of this finding.
when nft is minted,
function _mint(address _to, uint256 _tokenId) internal override { // Mint the token super._mint(_to, _tokenId); // Generate the token attributes if (!settings.metadataRenderer.onMinted(_tokenId)) revert NO_METADATA_GENERATED(); }
note we generate the nft metadata
if (!settings.metadataRenderer.onMinted(_tokenId)) revert NO_METADATA_GENERATED();
the logic inside the onMint is
function onMinted(uint256 _tokenId) external returns (bool) { // Ensure the caller is the token contract // if (msg.sender != settings.token) revert ONLY_TOKEN(); // Compute some randomness for the token id uint256 seed = _generateSeed(_tokenId); // Get the pointer to store generated attributes uint16[16] storage tokenAttributes = attributes[_tokenId]; // Cache the total number of properties available uint256 numProperties = properties.length; // Store the total as reference in the first slot of the token's array of attributes tokenAttributes[0] = uint16(numProperties); unchecked { // For each property: for (uint256 i = 0; i < numProperties; ++i) { // Get the number of items to choose from uint256 numItems = properties[i].items.length; // Use the token's seed to select an item tokenAttributes[i + 1] = uint16(seed % numItems); // Adjust the randomness seed >>= 16; } } return true; }
note, this always return true, so even though the nft properties is not generated properly.
for example, the addProperties is not called, so the numProperties is 0
// Cache the total number of properties available uint256 numProperties = properties.length;
then
for (uint256 i = 0; i < numProperties; ++i) {
will not run.
because the onMinted function is alwyas true, nft is minted but cannot get its metadata rendered.
when calling tokenURI
/// @notice The URI for a token /// @param _tokenId The ERC-721 token id function tokenURI(uint256 _tokenId) public view override(IToken, ERC721) returns (string memory) { return settings.metadataRenderer.tokenURI(_tokenId); }
and we check tokenURI in metadataRenderer
/// @notice The token URI /// @param _tokenId The ERC-721 token id function tokenURI(uint256 _tokenId) external view returns (string memory) { (bytes memory aryAttributes, bytes memory queryString) = getAttributes(_tokenId);
the getAttributes would throws error if the numPropries is 0
function getAttributes(uint256 _tokenId) public view returns (bytes memory aryAttributes, bytes memory queryString) { // Get the token's query string queryString = abi.encodePacked( "?contractAddress=", Strings.toHexString(uint256(uint160(address(this))), 20), "&tokenId=", Strings.toString(_tokenId) ); // Get the token's generated attributes uint16[16] memory tokenAttributes = attributes[_tokenId]; // Cache the number of properties when the token was minted uint256 numProperties = tokenAttributes[0]; // Ensure the given token was minted if (numProperties == 0) revert TOKEN_NOT_MINTED(_tokenId);
note if the numProperties is 0, revert TOKEN_NOT_MINTED(_tokenId),
then tokenURI cannot be called and nft metadata is not rendered, people cannot see the nft visually, which make nft lose value.
Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept.
in the file Token.sol, we can locate the mint function
function mint() external nonReentrant returns (uint256 tokenId) { // Cache the auction address address minter = settings.auction; // Ensure the caller is the auction if (msg.sender != minter) revert ONLY_AUCTION();
for testing purpose, we comment out
// if (msg.sender != minter) revert ONLY_AUCTION();
to make sure our test can call mint.
then in Token.t.sol
https://github.com/code-423n4/2022-09-nouns-builder/blob/main/test/Token.t.sol
we add the import
import "forge-std/console.sol";
we add the test
function test_TokenMint_TokenURI_POC() public { deployMock(); token.mint(); token.mint(); token.mint(); uint256 tokenId = token.totalSupply(); console.log('current token id', tokenId); console.logString(token.tokenURI(1)); }
and we run
forge test -vv --match TokenURI_POC
the output is
[FAIL. Reason: TOKEN_NOT_MINTED(1)] test_TokenMint_TokenURI_POC() (gas: 3259093) Logs: current token id 5 Test result: FAILED. 0 passed; 1 failed; finished in 12.66ms Failing tests: Encountered 1 failing test in test/Token.t.sol:TokenTest [FAIL. Reason: TOKEN_NOT_MINTED(1)] test_TokenMint_TokenURI_POC() (gas: 3259093)
the current token id 5, meaning nft from 0 -4 token id is minted,
why calling token.tokenURI(1) failed,
the error is TOKEN_NOT_MINTED(1)
which is in the line
// Cache the number of properties when the token was minted uint256 numProperties = tokenAttributes[0]; // Ensure the given token was minted if (numProperties == 0) revert TOKEN_NOT_MINTED(_tokenId);
numProperties is 0, so the error happens.
but if the metadata is created improperly in the onMinted funtion, the mint should be succeed in the first place.
Manual Review Foundry
We recommand add more testing to increase the test coverage of MetaRenderer.sol
for this issue in particularly,
make sure the function addProperty is called
/// @notice Adds properties and/or items to be pseudo-randomly chosen from during token minting /// @param _names The names of the properties to add /// @param _items The items to add to each property /// @param _ipfsGroup The IPFS base URI and extension function addProperties( string[] calldata _names, ItemParam[] calldata _items, IPFSGroup calldata _ipfsGroup ) external onlyOwner {
make sure the onMinted not just return true, the onMinted function should revert if the metadata does not set properly.
#0 - iainnash
2022-09-26T20:41:33Z
Auctions should not start until metadata is added. While ideally mint should fail in this case, this is by design. If there is no artwork, it doesn't feel like a high risk bug that the metadata render fails.
#1 - GalloDaSballo
2022-09-26T23:46:54Z
🌟 Selected for report: Lambda
Also found by: 0x1337, 0x1f8b, 0x4non, 0x85102, 0xA5DF, 0xNazgul, 0xSmartContract, 0xbepresent, 0xc0ffEE, 8olidity, Aymen0909, B2, Bnke0x0, CRYP70, Captainkay, CertoraInc, Ch_301, Chom, ChristianKuri, CodingNameKiki, Deivitto, Diana, DimitarDimitrov, ElKu, EthLedger, Franfran, Funen, GimelSec, JansenC, Jeiwan, Jujic, Lead_Belly, MEP, MasterCookie, MiloTruck, Noah3o6, PPrieditis, PaludoX0, Picodes, PwnPatrol, R2, Randyyy, RaymondFam, Respx, ReyAdmirado, Rolezn, Samatak, Tointer, Tomo, V_B, Waze, _Adam, __141345__, a12jmx, ak1, asutorufos, azephiar, ballx, bharg4v, bin2chen, bobirichman, brgltd, bulej93, c3phas, cccz, ch0bu, cloudjunky, cryptonue, cryptostellar5, cryptphi, csanuragjain, d3e4, datapunk, davidbrai, delfin454000, dharma09, dic0de, dipp, djxploit, eierina, erictee, fatherOfBlocks, gogo, hansfriese, hyh, imare, indijanc, izhuer, jonatascm, ladboy233, leosathya, lucacez, lukris02, m9800, martin, minhtrng, ne0n, neumo, oyc_109, p_crypt0, pashov, pauliax, pcarranzav, pedr02b2, peritoflores, pfapostol, rbserver, ret2basic, robee, rvierdiiev, sach1r0, sahar, scaraven, sikorico, simon135, slowmoses, sorrynotsorry, tnevler, tonisives, volky, yixxas, zkhorse, zzzitron
60.7756 USDC - $60.78
we recommend not to let admin reset the vetor after the vetoer is already burned.
/// @notice Updates the vetoer /// @param _newVetoer The new vetoer address function updateVetoer(address _newVetoer) external onlyOwner { if (_newVetoer == address(0)) revert ADDRESS_ZERO(); emit VetoerUpdated(settings.vetoer, _newVetoer); settings.vetoer = _newVetoer; } /// @notice Burns the vetoer function burnVetoer() external onlyOwner { emit VetoerUpdated(settings.vetoer, address(0)); delete settings.vetoer; }
token.transferFrom(address(this), _auction.highestBidder, _auction.tokenId);
token.burn(_auction.tokenId);
IWETH(WETH).deposit{ value: _amount }(); // Transfer WETH instead IWETH(WETH).transfer(_to, _amount);
the gas is hardcoded as 50000 wei.
assembly { // Transfer ETH to the recipient // Limit the call to 50,000 gas success := call(50000, _to, _amount, 0, 0, 0, 0) }
we recommand using
(bool sent, bytes memory data) = _to.call{value: msg.value}(""); require(sent, "Failed to send Ether");
#0 - GalloDaSballo
2022-09-27T00:23:48Z
R
Rest I disagree