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: 145/179
Findings: 2
Award: $35.17
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 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
In order to pay fees to distributors, takers, makers, governance, and the exchange, the GolomTrader contract executes payment via a payable(payAddress).transfer(payAmt). See https://consensys.net/diligence/blog/2019/09/stop-using-soliditys-transfer-now/ for more details. This is a core function that is called several times in the GolomTrader contract.
The payment of Ether to the intended parties may fail in the following cases:
This is a common code4rena issue. See
https://github.com/code-423n4/2022-01-openleverage-findings/issues/75 https://github.com/code-423n4/2021-10-tally-findings/issues/20 https://github.com/code-423n4/2021-04-meebits-findings/issues/45 https://github.com/code-423n4/2021-08-notional-findings/issues/38
Manual Analysis
Use call() instead of transfer() without a hardcoded gas limits. Also be sure to follow the checks-effects-interactions pattern and implement reentrancy guards for reentrancy protection (which I believe is already implemented).
#0 - KenzoAgada
2022-08-03T14:02:48Z
Duplicate of #343
🌟 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
Overall, the protocol engineers and designers appear to have thought out the protocol design and implementation in great depth. However, in general, the test coverage of the protocol could improve by a lot. There are contracts that do not have any associated tests (VoteEscrowDelegation, VoteEscrowCore). There is also commented code in the codebase that does not appear to be serving any purpose.
There is no validation being done on toTokenId in the following function. I was not able to verify if there was a directly exploitable mechanism here that could result in the loss of funds or protocol leakage.
erecover returns an empty address (0x0) when the signature is invalid. It is generally considered good practice to validate on the 0x0 address such as require(signaturesigner) != address(0). Would recommend appending the 0 address check onto the current require.
The if statement in the following code block does not appear to ever get hit due to the require statement and can thus be removed.
The VoteEscrowDelegation.specs.ts is an empty file.
Would recommend more helpful user messaging in the following line https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/core/GolomTrader.sol#L217.
Inconsistent error messaging. In fillAsk() there is an appropriate error message whereas in fillBid() and fillCriteriaBid() there is no error message.
https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/core/GolomTrader.sol#L349 https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/core/GolomTrader.sol#L295 https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/core/GolomTrader.sol#L226
fillAsk() contains error messaging, fillBid() and fillCriteriaBid() do not. https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/core/GolomTrader.sol#L227 https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/core/GolomTrader.sol#L296 https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/core/GolomTrader.sol#L350
remove code or add relevant test cases.
https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/test/GolomTrader.specs.ts#L645 https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/test/GolomTrader.specs.ts#L678 https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/test/RewardDistributor.specs.ts#L132-L144 https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/test/RewardDistributor.specs.ts#L364-L368