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: 127/179
Findings: 2
Award: $35.32
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 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
The transferFrom() method is used instead of safeTransferFrom(), presumably to save gas. I however argue that this isn’t recommended because:
OpenZeppelin’s documentation discourages the use of transferFrom(), use safeTransferFrom() whenever possible
Finding
File: contracts/core/GolomTrader.sol 236: ERC721(o.collection).transferFrom(o.signer, receiver, o.tokenId); 301: nftcontract.transferFrom(msg.sender, o.signer, o.tokenId) 363: nftcontract.transferFrom(msg.sender, o.signer, tokenId);
used safeTransferFrom(),
#0 - KenzoAgada
2022-08-03T15:05:06Z
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
address(0x0)validateOrder() doesn't check if signaturesigner and o.signer are not a 0x0 address
Finding
File: contracts/core/GolomTrader.sol address signaturesigner = ecrecover(hash, o.v, o.r, o.s); require(signaturesigner == o.signer, 'invalid signature');
Recommended Mitigation Steps
Add a require()
address signaturesigner = ecrecover(hash, o.v, o.r, o.s); require(signaturesigner == o.signer, 'invalid signature'); require(signaturesigner != address(0), "ECDSA: invalid signature");
amount = 0A griefer is able to bypass all the checks fillAsk()
Finding
File: contracts/core/GolomTrader.sol require(msg.value >= o.totalAmt * amount + p.paymentAmt, 'mgmtm');
if() statement will never enterThe if() check here is always false because the require() before it do the same check so if signaturesigner == o.signer is false the faction will faile
Finding
File: contracts/core/GolomTrader.sol if (signaturesigner != o.signer) { return (0, hashStruct, 0); }
block.chainid rather than chainId := chainid()the variable block.chainid is available on Solidity
https://github.com/ethereum/solidity/pull/10557
Finding
File: contracts/core/GolomTrader.sol assembly { chainId := chainid() }