Platform: Code4rena
Start Date: 20/09/2022
Pot Size: $100,000 USDC
Total HM: 4
Participants: 109
Period: 7 days
Judge: GalloDaSballo
Id: 163
League: ETH
Rank: 12/109
Findings: 2
Award: $2,830.23
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: wagmi
Also found by: 0x52, CertoraInc, Lambda, RaymondFam, arcoun
https://github.com/code-423n4/2022-09-artgobblers/blob/main/src/ArtGobblers.sol#L457-L458 https://github.com/code-423n4/2022-09-artgobblers/blob/main/src/ArtGobblers.sol#L469 https://github.com/code-423n4/2022-09-artgobblers/blob/main/src/utils/token/GobblersERC721.sol#L166
During a Legendary Gobbler mint, the number of gobblers owned by the user is wrongly updated. After the mintLegendaryGobbler() call, the gobblersOwned variable in the user's data will have one more gobbler than expected. Any caller of the balanceOf() method will get a wrong result and may make bad assumptions.
The issue is due to the fact that the minted Legendary token is added two times:
mintLegendaryGobbler(), the number of burned gobblers is removed and one token is added for the minted Legendary token_mint() the minted token is also added, although it has already be addedgL gobblersgU gobblers, with gu >= gLmintLegendaryGobbler to mint a legendarygU will first be updated to gU - gL + 1gU will be increased by 1 during _mint()gU - gL + 2 instead of gu - gL + 1The minted token should not be added during mintLegendaryGobbler() as it will already be added during _mint().
#0 - Shungy
2022-09-28T15:30:57Z
#1 - GalloDaSballo
2022-09-29T20:35:56Z
🌟 Selected for report: IllIllI
Also found by: 0x1f8b, 0x4non, 0x52, 0x5rings, 0xNazgul, 0xRobocop, 0xSmartContract, 0xdeadbeef, 0xsanson, 8olidity, Amithuddar, Aymen0909, B2, B353N, CertoraInc, Ch_301, Chom, CodingNameKiki, Deivitto, ElKu, Funen, JC, JohnnyTime, Kresh, Lambda, Noah3o6, RaymondFam, ReyAdmirado, RockingMiles, Rolezn, Sm4rty, SuldaanBeegsi, Tadashi, TomJ, Tomio, V_B, Waze, __141345__, a12jmx, ak1, arcoun, asutorufos, aviggiano, berndartmueller, bharg4v, bin2chen, brgltd, bulej93, c3phas, catchup, cccz, ch0bu, cryptonue, cryptphi, csanuragjain, delfin454000, devtooligan, djxploit, durianSausage, eighty, erictee, exd0tpy, fatherOfBlocks, giovannidisiena, hansfriese, ignacio, joestakey, ladboy233, lukris02, m9800, malinariy, martin, minhtrng, obront, oyc_109, pedr02b2, pedroais, pfapostol, philogy, prasantgupta52, rbserver, ronnyx2017, rotcivegaf, rvierdiiev, sach1r0, shung, simon135, throttle, tnevler, tonisives, wagmi, yixxas, zkhorse, zzykxx, zzzitron
55.1985 USDC - $55.20
https://github.com/code-423n4/2022-09-artgobblers/blob/main/src/ArtGobblers.sol
The ArtGobblers contract can receive ERC1155 tokens using the safe receiver functions inherited from ERC1155TokenReceiver but it does not return the related interface in the supportsInterface method (ERC165).
EIP-1155 states that:
Smart contracts MUST implement the ERC-165
supportsInterfacefunction and signify support for theERC1155TokenReceiverinterface to accept transfers.
The supportsInterface method is currently inherited from ERC721 which does not support ERC1155. The ArtGobblers contract should implement its own supportsInterface method to support both ERC721 and ERC1155TokenReceiver interfaces.
https://github.com/code-423n4/2022-09-artgobblers/blob/main/README.md?plain=1#L213 https://github.com/code-423n4/2022-09-artgobblers/blob/main/src/ArtGobblers.sol#L83
The README.md file states that Art Gobblers are ERC1155 NFTs but they are not. The ArtGobblers contract implements the token as an ERC721 token which can receive ERC1155 NFTs.
Use 1e18 instead of 1000000000000000000 to increase readability.
#0 - GalloDaSballo
2022-10-14T00:28:00Z
Edit:
Per https://eips.ethereum.org/EIPS/eip-1155#:~:text=ERC1155TokenReceiver%20ERC%2D165%20rules%3A
If you're looking to receive a ERC1155 you want to signify it via the ERC1155TokenReceiver token support on supportsInterface
Meaning the finding is valid
L
NC
R
1L 1R 1NC
Genuine work, but missing a few more findings, good start!