Platform: Code4rena
Start Date: 26/07/2022
Pot Size: $75,000 USDC
Total HM: 29
Participants: 179
Period: 6 days
Judge: LSDan
Total Solo HM: 6
Id: 148
League: ETH
Rank: 59/179
Findings: 5
Award: $188.25
🌟 Selected for report: 0
🚀 Solo Findings: 0
131.6127 USDC - $131.61
The addFee() function will stop executing any meaningful action at the contract's final epoch, final epoch being when the supply hits 1 billion. This is done on purpose in order to allow the golom token to hit it's maximum supply. The traderClaim(), multiStakerClaim() and exchangeClaim() functions allow traders, stakers and exchanges to withdraw their rewards for any epoch save the current or future ones, which means any fee that is paid when the supply achieves the 1 billion mark is locked in the contract. Eth fees paid to the distributor contract will also be stuck as stakers will not be allowed to retrieve them.
addFee() does not execute any meaningful action.fillAsk(), the 50 basis fee is transferred to the distributor contractNone
Allow the current epoch rewards to be withdrawn if and only if the supply is greater than one billion.
#0 - 0xsaruman
2022-09-02T11:30:46Z
this is more or less a duplicate of https://github.com/code-423n4/2022-07-golom-findings/issues/320 and gets resolved on fixing the above bug.
🌟 Selected for report: cloudjunky
Also found by: 0x1f8b, 0x4non, 0x52, 0xDjango, 0xHarry, 0xNazgul, 0xNineDec, 0xf15ers, 0xsanson, 0xsolstars, 8olidity, Bnke0x0, CertoraInc, Chom, Deivitto, Dravee, GalloDaSballo, GimelSec, IllIllI, Jmaxmanblue, JohnSmith, Jujic, Kenshin, Krow10, Lambda, MEP, Noah3o6, RedOneN, Ruhum, StErMi, StyxRave, TomJ, Treasure-Seeker, TrungOre, _Adam, __141345__, arcoun, asutorufos, bardamu, bearonbike, bin2chen, brgltd, bulej93, c3phas, cRat1st0s, carlitox477, cccz, codexploder, cryptonue, cryptphi, cthulhu_cult, dharma09, dipp, djxploit, durianSausage, ellahi, giovannidisiena, hansfriese, horsefacts, hyh, immeas, indijanc, jayjonah8, jayphbee, joestakey, kenzo, kyteg, ladboy233, minhquanym, navinavu, obront, oyc_109, peritoflores, rbserver, reassor, rokinot, rotcivegaf, saian, scaraven, shenwilly, simon135, sseefried, teddav, zzzitron
0.0037 USDC - $0.00
The transfer() function was introduced after The DAO hack with a hard limit of 2300 gas as a way to safely send ether while also preventing reentrancy attacks. However this limit will also prevent external contracts interacting with the protocol to execute more elaborate operations after filling a bid/ask order.
None
Use the call() function instead, the protocol is secured against reentrancy attacks already as the nonReentrant modifier is deployed at every function with a payEther() interaction.
#0 - KenzoAgada
2022-08-03T14:06:45Z
Duplicate of #343
🌟 Selected for report: TomJ
Also found by: 0x4non, 0x52, 0xDjango, 0xNazgul, 0xf15ers, 0xsanson, 8olidity, Bnke0x0, CertoraInc, Ch_301, Chom, Dravee, GalloDaSballo, GimelSec, JC, Jujic, Kenshin, Kumpa, Lambda, M0ndoHEHE, PaludoX0, RedOneN, Ruhum, Sm4rty, Treasure-Seeker, TrungOre, Twpony, Waze, _Adam, __141345__, apostle0x01, arcoun, benbaessler, bin2chen, brgltd, cccz, cloudjunky, cryptonue, djxploit, ellahi, erictee, hansfriese, i0001, minhquanym, oyc_109, peritoflores, rbserver, reassor, rokinot, rotcivegaf, saian, shenwilly, sseefried
0.1513 USDC - $0.15
https://github.com/code-423n4/2022-07-golom/blob/7bbb55fca61e6bae29e57133c1e45806cbb17aa4/contracts/core/GolomTrader.sol#L236 https://github.com/code-423n4/2022-07-golom/blob/7bbb55fca61e6bae29e57133c1e45806cbb17aa4/contracts/core/GolomTrader.sol#L301 https://github.com/code-423n4/2022-07-golom/blob/7bbb55fca61e6bae29e57133c1e45806cbb17aa4/contracts/core/GolomTrader.sol#L361
The usage of transferFrom() is discouraged as it lacks some form of check to see if the receiving address, in case it's a smart contract, could properly receive the NFT, resulting in the loss of it.
None
Use the ERC721.safeTransferFrom() method instead.
#0 - KenzoAgada
2022-08-03T15:12:41Z
Duplicate of #342
🌟 Selected for report: IllIllI
Also found by: 0x1f8b, 0x4non, 0x52, 0xA5DF, 0xDjango, 0xLovesleep, 0xNazgul, 0xNineDec, 0xSmartContract, 0xackermann, 0xc0ffEE, 0xf15ers, 0xmatt, 0xsanson, 0xsolstars, 8olidity, AuditsAreUS, Bahurum, Bnke0x0, CRYP70, CertoraInc, Ch_301, Chom, CryptoMartian, Deivitto, DevABDee, Dravee, ElKu, Franfran, Funen, GalloDaSballo, GimelSec, GiveMeTestEther, Green, JC, Jmaxmanblue, JohnSmith, Jujic, Junnon, Kenshin, Krow10, Kumpa, Lambda, MEP, Maxime, MiloTruck, Mohandes, NoamYakov, Picodes, RedOneN, Rohan16, Rolezn, Ruhum, RustyRabbit, Sm4rty, Soosh, StErMi, StyxRave, Tadashi, TomJ, Treasure-Seeker, TrungOre, Waze, _Adam, __141345__, ajtra, ak1, apostle0x01, arcoun, asutorufos, async, benbaessler, berndartmueller, bin2chen, brgltd, c3phas, cRat1st0s, carlitox477, chatch, codetilda, codexploder, cryptonue, cryptphi, csanuragjain, cthulhu_cult, delfin454000, dipp, dirk_y, djxploit, ellahi, exd0tpy, fatherOfBlocks, giovannidisiena, hansfriese, horsefacts, hyh, idkwhatimdoing, indijanc, jayfromthe13th, jayphbee, joestakey, kenzo, kyteg, lucacez, luckypanda, mics, minhquanym, obront, oyc_109, pedr02b2, rajatbeladiya, rbserver, reassor, robee, rokinot, rotcivegaf, sach1r0, saian, saneryee, sashik_eth, scaraven, shenwilly, simon135, sseefried, supernova, teddav, ych18, zuhaibmohd, zzzitron
35.1687 USDC - $35.17
amount value in fillAsk() or fillBid() is zeroTransactions do not revert with zero amount if they're for ERC1155 types
This can be fixed by adding a variable in the validateOrder() for the first value return, as well as adding a require() statement that the order isn't of type 2.
The issue can be fixed by changing the weth transfer to this line.
validateOrder() has an impossible to reach if statementThe require() statement on the following code reverts in case the signature is invalid, but the following code has an if statement which is only triggered when the signature is invalid
#L177-L180
require() statements have no revert stringhttps://github.com/code-423n4/2022-07-golom/blob/7bbb55fca61e6bae29e57133c1e45806cbb17aa4/contracts/core/GolomTrader.sol#L220 https://github.com/code-423n4/2022-07-golom/blob/7bbb55fca61e6bae29e57133c1e45806cbb17aa4/contracts/core/GolomTrader.sol#L285 https://github.com/code-423n4/2022-07-golom/blob/7bbb55fca61e6bae29e57133c1e45806cbb17aa4/contracts/core/GolomTrader.sol#L293-L296 https://github.com/code-423n4/2022-07-golom/blob/7bbb55fca61e6bae29e57133c1e45806cbb17aa4/contracts/core/GolomTrader.sol#L313 https://github.com/code-423n4/2022-07-golom/blob/7bbb55fca61e6bae29e57133c1e45806cbb17aa4/contracts/core/GolomTrader.sol#L342-L350 https://github.com/code-423n4/2022-07-golom/blob/7bbb55fca61e6bae29e57133c1e45806cbb17aa4/contracts/rewards/RewardDistributor.sol#L88 https://github.com/code-423n4/2022-07-golom/blob/7bbb55fca61e6bae29e57133c1e45806cbb17aa4/contracts/rewards/RewardDistributor.sol#L144 https://github.com/code-423n4/2022-07-golom/blob/7bbb55fca61e6bae29e57133c1e45806cbb17aa4/contracts/rewards/RewardDistributor.sol#L158
https://github.com/code-423n4/2022-07-golom/blob/7bbb55fca61e6bae29e57133c1e45806cbb17aa4/contracts/core/GolomTrader.sol#L60 https://github.com/code-423n4/2022-07-golom/blob/7bbb55fca61e6bae29e57133c1e45806cbb17aa4/contracts/rewards/RewardDistributor.sol#L111
require() message has an unclear revert messagehttps://github.com/code-423n4/2022-07-golom/blob/7bbb55fca61e6bae29e57133c1e45806cbb17aa4/contracts/rewards/RewardDistributor.sol#L141 https://github.com/code-423n4/2022-07-golom/blob/7bbb55fca61e6bae29e57133c1e45806cbb17aa4/contracts/rewards/RewardDistributor.sol#L155
🌟 Selected for report: JohnSmith
Also found by: 0x1f8b, 0xA5DF, 0xDjango, 0xKitsune, 0xLovesleep, 0xNazgul, 0xSmartContract, 0xmatt, 0xsam, Aymen0909, Bnke0x0, CRYP70, Chandr, Chinmay, CodingNameKiki, Deivitto, Dravee, ElKu, Fitraldys, Funen, GalloDaSballo, Green, IllIllI, JC, Jmaxmanblue, Junnon, Kaiziron, Kenshin, Krow10, Maxime, Migue, MiloTruck, Noah3o6, NoamYakov, Randyyy, RedOneN, ReyAdmirado, Rohan16, Rolezn, Ruhum, Sm4rty, StyxRave, TomJ, Tomio, _Adam, __141345__, ajtra, ak1, apostle0x01, asutorufos, async, benbaessler, brgltd, c3phas, cRat1st0s, carlitox477, delfin454000, djxploit, durianSausage, ellahi, erictee, fatherOfBlocks, gerdusx, gogo, hyh, jayfromthe13th, jayphbee, joestakey, kaden, kenzo, kyteg, ladboy233, lucacez, m_Rassska, mics, minhquanym, oyc_109, pfapostol, rbserver, reassor, rfa, robee, rokinot, sach1r0, saian, samruna, sashik_eth, simon135, supernova, tofunmi, zuhaibmohd
21.3211 USDC - $21.32
require() statements for custom errorsThere are 32 instances in the code
epoch variable in order to save gas++index loop increments are more efficient than index++https://github.com/code-423n4/2022-07-golom/blob/7bbb55fca61e6bae29e57133c1e45806cbb17aa4/contracts/rewards/RewardDistributor.sol#L180-L210 https://github.com/code-423n4/2022-07-golom/blob/7bbb55fca61e6bae29e57133c1e45806cbb17aa4/contracts/rewards/RewardDistributor.sol#L143 https://github.com/code-423n4/2022-07-golom/blob/7bbb55fca61e6bae29e57133c1e45806cbb17aa4/contracts/rewards/RewardDistributor.sol#L157
payable modifier in order to save gasBy upgrading to v0.8.15, some small operations become more efficient compared to the 0.8.11 used, such as:
uint256 variable != 0 being cheaper than uint256 variable > 0 comparisons
x += y being cheaper than x = x + y
unchecked{} as they have no risk of overflow#L118 #L315 #L157 #L143 #L180-L183 #L226 #L258 #L273
public functions not called in the contract should be changed to external