Golom contest - lucacez's results

An NFT marketplace that offers the lowest industry fee, a publicly available order-book along with analytical tools.

General Information

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

Golom

Findings Distribution

Researcher Performance

Rank: 112/179

Findings: 2

Award: $56.49

🌟 Selected for report: 0

🚀 Solo Findings: 0

External Links

Code4rena Contest Page Github Contest Repository Github Findings Repository Golom

Findings Information

🌟 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

Awards

35.1687 USDC - $35.17

Labels

bug

QA (Quality Assurance)

External Links

Link to GitHub issue

RETURN VALUES OF `TRANSFER()`/`TRANSFERFROM()` NOT CHECKED

Not all IERC20 implementations revert() when there’s a failure in transfer()/transferFrom(). The function signature has a boolean return value and they indicate errors that way instead. By not checking the return value, operations that should have marked as failed, may potentially go through without actually making a payment

There are 2 instances of this issue:

File: contracts/core/GolomTrader.sol

382:        WETH.transferFrom(o.signer, address(this), o.totalAmt * amount);
383:        WETH.withdraw(o.totalAmt * amount);

https://github.com/code-423n4/2022-07-golom/blob/7bbb55fca61e6bae29e57133c1e45806cbb17aa4/contracts/core/GolomTrader.sol#L382-L383

File: contracts/core/GolomTrader.sol

236:            ERC721(o.collection).transferFrom(o.signer, receiver, o.tokenId);
238:            ERC1155(o.collection).safeTransferFrom(o.signer, receiver, o.tokenId, amount, '');

https://github.com/code-423n4/2022-07-golom/blob/7bbb55fca61e6bae29e57133c1e45806cbb17aa4/contracts/core/GolomTrader.sol#L236-L238

NON-ASSEMBLY METHOD AVAILABLE

assembly{ id := chainid() } => uint256 id = block.chainid, assembly { size := extcodesize() } => uint256 size = address().code.length

There is 1 instance of this issue:

File: contracts/core/GolomTrader.sol

98:             chainId := chainid()

https://github.com/code-423n4/2022-07-golom/blob/7bbb55fca61e6bae29e57133c1e45806cbb17aa4/contracts/core/GolomTrader.sol#L98

Findings Information

🌟 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

Awards

21.3211 USDC - $21.32

Labels

bug

G (Gas Optimization)

edited-by-warden

External Links

Link to GitHub issue

`++I` COSTS LESS GAS THAN `I++`, ESPECIALLY WHEN IT’S USED IN `FOR`-LOOPS (`--I`/`I--` TOO)

Saves 6 gas PER LOOP

There are 8 instances of this issue:

File: contracts/core/GolomTrader.sol

415:        for (uint256 i = 0; i < proof.length; i++) {

https://github.com/code-423n4/2022-07-golom/blob/7bbb55fca61e6bae29e57133c1e45806cbb17aa4/contracts/core/GolomTrader.sol#L415

File: contracts/rewards/RewardDistributor.sol

143:        for (uint256 index = 0; index < epochs.length; index++) {

https://github.com/code-423n4/2022-07-golom/blob/7bbb55fca61e6bae29e57133c1e45806cbb17aa4/contracts/rewards/RewardDistributor.sol#L143

File: contracts/rewards/RewardDistributor.sol

157:        for (uint256 index = 0; index < epochs.length; index++) {

https://github.com/code-423n4/2022-07-golom/blob/7bbb55fca61e6bae29e57133c1e45806cbb17aa4/contracts/rewards/RewardDistributor.sol#L157

File: contracts/rewards/RewardDistributor.sol

180:        for (uint256 tindex = 0; tindex < tokenids.length; tindex++) {

https://github.com/code-423n4/2022-07-golom/blob/7bbb55fca61e6bae29e57133c1e45806cbb17aa4/contracts/rewards/RewardDistributor.sol#L180

File: contracts/rewards/RewardDistributor.sol

183:            for (uint256 index = 0; index < epochs.length; index++) {

https://github.com/code-423n4/2022-07-golom/blob/7bbb55fca61e6bae29e57133c1e45806cbb17aa4/contracts/rewards/RewardDistributor.sol#L183

File: contracts/vote-escrow/VoteEscrowDelegation.sol

171:        for (uint256 index = 0; index < delegated.length; index++) {

https://github.com/code-423n4/2022-07-golom/blob/7bbb55fca61e6bae29e57133c1e45806cbb17aa4/contracts/vote-escrow/VoteEscrowDelegation.sol#L171

File: contracts/vote-escrow/VoteEscrowDelegation.sol

189:        for (uint256 index = 0; index < delegatednft.length; index++) {

https://github.com/code-423n4/2022-07-golom/blob/7bbb55fca61e6bae29e57133c1e45806cbb17aa4/contracts/vote-escrow/VoteEscrowDelegation.sol#L189

File: contracts/vote-escrow/VoteEscrowDelegation.sol

199:        for (uint256 i; i < _array.length; i++) {

https://github.com/code-423n4/2022-07-golom/blob/7bbb55fca61e6bae29e57133c1e45806cbb17aa4/contracts/vote-escrow/VoteEscrowDelegation.sol#L199

`<ARRAY>.LENGTH` SHOULD NOT BE LOOKED UP IN EVERY LOOP OF A `FOR`-LOOP

The overheads outlined below are PER LOOP, excluding the first loop

storage arrays incur a Gwarmaccess (100 gas)
memory arrays use MLOAD (3 gas)
calldata arrays use CALLDATALOAD (3 gas) Caching the length changes each of these to a DUP<N> (3 gas), and gets rid of the extra DUP<N> needed to store the stack offset

There is 8 instance of this issue:

File: contracts/core/GolomTrader.sol

415:        for (uint256 i = 0; i < proof.length; i++) {

https://github.com/code-423n4/2022-07-golom/blob/7bbb55fca61e6bae29e57133c1e45806cbb17aa4/contracts/core/GolomTrader.sol#L415

File: contracts/rewards/RewardDistributor.sol

143:        for (uint256 index = 0; index < epochs.length; index++) {

https://github.com/code-423n4/2022-07-golom/blob/7bbb55fca61e6bae29e57133c1e45806cbb17aa4/contracts/rewards/RewardDistributor.sol#L143

File: contracts/rewards/RewardDistributor.sol

157:        for (uint256 index = 0; index < epochs.length; index++) {

https://github.com/code-423n4/2022-07-golom/blob/7bbb55fca61e6bae29e57133c1e45806cbb17aa4/contracts/rewards/RewardDistributor.sol#L157

File: contracts/rewards/RewardDistributor.sol

180:        for (uint256 tindex = 0; tindex < tokenids.length; tindex++) {

https://github.com/code-423n4/2022-07-golom/blob/7bbb55fca61e6bae29e57133c1e45806cbb17aa4/contracts/rewards/RewardDistributor.sol#L180

File: contracts/rewards/RewardDistributor.sol

183:            for (uint256 index = 0; index < epochs.length; index++) {

https://github.com/code-423n4/2022-07-golom/blob/7bbb55fca61e6bae29e57133c1e45806cbb17aa4/contracts/rewards/RewardDistributor.sol#L183

File: contracts/vote-escrow/VoteEscrowDelegation.sol

171:        for (uint256 index = 0; index < delegated.length; index++) {

https://github.com/code-423n4/2022-07-golom/blob/7bbb55fca61e6bae29e57133c1e45806cbb17aa4/contracts/vote-escrow/VoteEscrowDelegation.sol#L171

File: contracts/vote-escrow/VoteEscrowDelegation.sol

189:        for (uint256 index = 0; index < delegatednft.length; index++) {

https://github.com/code-423n4/2022-07-golom/blob/7bbb55fca61e6bae29e57133c1e45806cbb17aa4/contracts/vote-escrow/VoteEscrowDelegation.sol#L189

File: contracts/vote-escrow/VoteEscrowDelegation.sol

199:        for (uint256 i; i < _array.length; i++) {

https://github.com/code-423n4/2022-07-golom/blob/7bbb55fca61e6bae29e57133c1e45806cbb17aa4/contracts/vote-escrow/VoteEscrowDelegation.sol#L199

`PUBLIC` FUNCTIONS NOT CALLED BY THE CONTRACT SHOULD BE DECLARED `EXTERNAL` INSTEAD

Contracts are allowed to override their parents’ functions and change the visibility from external to public and can save gas by doing so.

There are 4 instances of this issue:

File: contracts/core/GolomTrader.sol

203:    function fillAsk(
204:        Order calldata o,
205:        uint256 amount,
206:        address referrer,
207:        Payment calldata p,
208:        address receiver
209:    ) public payable nonReentrant {

https://github.com/code-423n4/2022-07-golom/blob/7bbb55fca61e6bae29e57133c1e45806cbb17aa4/contracts/core/GolomTrader.sol#L203-L209

File: contracts/core/GolomTrader.sol

279:    function fillBid(
280:        Order calldata o,
281:        uint256 amount,
282:        address referrer,
283:        Payment calldata p
284:    ) public nonReentrant {

https://github.com/code-423n4/2022-07-golom/blob/7bbb55fca61e6bae29e57133c1e45806cbb17aa4/contracts/core/GolomTrader.sol#L279-L284

File: contracts/core/GolomTrader.sol

312:    function cancelOrder(Order calldata o) public nonReentrant {
313:        require(o.signer == msg.sender);
314:        (, bytes32 hashStruct, ) = validateOrder(o);
315:        filled[hashStruct] = o.tokenAmt + 1;
316:        emit OrderCancelled(hashStruct);
317:    }

https://github.com/code-423n4/2022-07-golom/blob/7bbb55fca61e6bae29e57133c1e45806cbb17aa4/contracts/core/GolomTrader.sol#L312-L317

File: contracts/core/GolomTrader.sol

334:    function fillCriteriaBid(
335:        Order calldata o,
336:        uint256 amount,
337:        uint256 tokenId,
338:        bytes32[] calldata proof,
339:        address referrer,
340:        Payment calldata p
341:    ) public nonReentrant {

https://github.com/code-423n4/2022-07-golom/blob/7bbb55fca61e6bae29e57133c1e45806cbb17aa4/contracts/core/GolomTrader.sol#L334-L341

Golom contest - lucacez's results

An NFT marketplace that offers the lowest industry fee, a publicly available order-book along with analytical tools.

General Information

Findings Distribution

Researcher Performance

External Links

[Q-85] QA Report - $35.17

Findings Information

Awards

Labels

External Links

RETURN VALUES OF TRANSFER()/TRANSFERFROM() NOT CHECKED

NON-ASSEMBLY METHOD AVAILABLE

[G-59] Gas Report - $21.32

Findings Information

Awards

Labels

External Links

++I COSTS LESS GAS THAN I++, ESPECIALLY WHEN IT’S USED IN FOR-LOOPS (--I/I-- TOO)

<ARRAY>.LENGTH SHOULD NOT BE LOOKED UP IN EVERY LOOP OF A FOR-LOOP

PUBLIC FUNCTIONS NOT CALLED BY THE CONTRACT SHOULD BE DECLARED EXTERNAL INSTEAD

RETURN VALUES OF `TRANSFER()`/`TRANSFERFROM()` NOT CHECKED

`++I` COSTS LESS GAS THAN `I++`, ESPECIALLY WHEN IT’S USED IN `FOR`-LOOPS (`--I`/`I--` TOO)

`<ARRAY>.LENGTH` SHOULD NOT BE LOOKED UP IN EVERY LOOP OF A `FOR`-LOOP

`PUBLIC` FUNCTIONS NOT CALLED BY THE CONTRACT SHOULD BE DECLARED `EXTERNAL` INSTEAD