Platform: Code4rena
Start Date: 11/08/2022
Pot Size: $40,000 USDC
Total HM: 8
Participants: 108
Period: 4 days
Judge: hickuphh3
Total Solo HM: 2
Id: 152
League: ETH
Rank: 22/108
Findings: 3
Award: $104.64
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: Saw-mon_and_Natalie
Also found by: 0x1f8b, 0x52, 0xDjango, 0xNazgul, 0xSmartContract, 0xSolus, 0xackermann, 0xmatt, 0xsolstars, Aymen0909, Bnke0x0, Chom, Deivitto, DevABDee, Dravee, ElKu, IllIllI, JC, Kumpa, Lambda, LeoS, MiloTruck, PwnedNoMore, ReyAdmirado, Rohan16, Rolezn, Ruhum, Sm4rty, TomJ, Treasure-Seeker, Vexjon, Waze, Yiko, __141345__, apostle0x01, auditor0517, berndartmueller, bin2chen, bobirichman, brgltd, bulej93, c3phas, carlitox477, cccz, cryptphi, csanuragjain, d3e4, danb, delfin454000, durianSausage, erictee, fatherOfBlocks, gogo, iamwhitelights, joestakey, jonatascm, ladboy233, mics, oyc_109, rbserver, ret2basic, robee, rokinot, rvierdiiev, shenwilly, sikorico, simon135, thank_you, wagmi, yash90, zeesaw, zkhorse
41.2059 USDC - $41.21
| Id | Title |
|---|---|
| 1 | postRevealBaseURIHash is not used to validate baseURI |
| 2 | Users can always bypass the limitPerAccount config |
| 3 | Not subtract referrerFee in getFeesAndRecipients() |
postRevealBaseURIHash is not used to validate baseURIIn documentation, postRevealBaseURIHash is supposed to be used to validate baseURI by making sure hash(baseURI) == postRevealBaseURIHash when admin reveal the baseURI
But in implementaion (function reveal()), this value is not used for that purpose and have no value for on-chain validation.
limitPerAccount configThere is a config limitPerAccount in saleConfig. It is supposed to be used to confirm that the buyer will not exceed the limit specified after minting.
But this kind of limit in general can always be bypassed by users. It checks that current balance of users add count should be smaller or equal to limitPerAccount
if (IERC721(nftContract).balanceOf(msg.sender) + count > saleConfig.limitPerAccount) {
Users can transfer these ERC721 token to another wallet to modify value of balanceOf or they can simply use another wallet to bypass this check.
referrerFee in getFeesAndRecipients()In function getFeesAndRecipients(), there is an open TODO that should be update to add referral info. Otherwise, these function will return values without referrerFee and can harm user experience.
#0 - HardlyDifficult
2022-08-18T15:56:53Z
Good report! Although each of these were submitted by many wardens, they are all worth consideration and I agree with how they were prioritized by this warden.
postRevealBaseURIHash is not used to validate baseURI
Agree, we have decided to simplify this into true/false, for context see our comment here.
Can bypass
limitPerAccount
Agree, for context see our comment here.
Unresolved TODO comments
Agree, will fix.
🌟 Selected for report: Dravee
Also found by: 0x040, 0x1f8b, 0xDjango, 0xHarry, 0xNazgul, 0xSmartContract, 0xbepresent, 0xkatana, Amithuddar, Aymen0909, Bnke0x0, Chom, CodingNameKiki, Deivitto, DevABDee, Diraco, ElKu, Fitraldys, Funen, IllIllI, JC, LeoS, Metatron, MiloTruck, Noah3o6, ReyAdmirado, Rohan16, Rolezn, Saw-mon_and_Natalie, Sm4rty, SpaceCake, TomJ, Tomio, Trabajo_de_mates, Waze, Yiko, __141345__, ajtra, apostle0x01, bobirichman, brgltd, bulej93, c3phas, cRat1st0s, carlitox477, d3e4, durianSausage, erictee, fatherOfBlocks, gerdusx, gogo, hakerbaya, ignacio, jag, joestakey, ladboy233, medikko, mics, newfork01, oyc_109, pfapostol, robee, rvierdiiev, sach1r0, saian, samruna, sikorico, simon135, wagmi, zeesaw, zkhorse, zuhaibmohd
20.5997 USDC - $20.60
In function, if there are 2 variable with the same values, but 1 in memory and 1 in storage, we should use memory one when reading.
For example, this event line
emit CreateFixedPriceSale(nftContract, saleConfig.seller, saleConfig.price, saleConfig.limitPerAccount);
saleConfig is storage variable and these values can be replaced like saleConfig.seller = msg.sender to save gas
#0 - HardlyDifficult
2022-08-19T15:03:13Z
- Use memory variable instead of storage in event to save gas
Agree, fixed!