Golom contest - Noah3o6'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: 167/179

Findings: 2

Award: $21.32

🌟 Selected for report: 0

🚀 Solo Findings: 0

External Links

Code4rena Contest Page Github Contest Repository Github Findings Repository Golom

Findings Information

🌟 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

Awards

0.0037 USDC - $0.00

Labels

bug

duplicate

2 (Med Risk)

External Links

Link to GitHub issue

Lines of code

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

Vulnerability details

Impact

The use of the deprecated transfer() function for an address will inevitably make the transaction fail when:

-The claimer smart contract does not implement a payable function. -The claimer smart contract does implement a payable fallback which uses more than 2300 gas unit. -The claimer smart contract implements a payable fallback function that needs less than 2300 gas units but is called through proxy, raising the call’s gas usage above 2300. Additionally, using higher than 2300 gas might be mandatory for some multisig wallets.

Proof of Concept

I am not 100% clear about this, but i saw it in an other report from a contest before https://code4rena.com/reports/2022-04-backd/#m-01-call-should-be-used-instead-of-transfer-on-an-address-payable

Tools Used

No tools used for this

Recommended Mitigation Steps

I recommend using call() instead of transfer().

#0 - KenzoAgada
2022-08-03T14:06:25Z

Duplicate of #343

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)

External Links

Link to GitHub issue

-> X = X + Y IS CHEAPER THAN X += Y (Same for X = X - Y is cheaper than X -= Y)

https://github.com/code-423n4/2022-07-golom/blob/7bbb55fca61e6bae29e57133c1e45806cbb17aa4/contracts/vote-escrow/TokenUriHelper.sol#:~:text=digits-,%2D%3D,-1%3B

->STATE VARIABLES ONLY SET IN THE CONSTRUCTOR SHOULD BE DECLARED IMMUTABLE Avoids a Gsset (20000 gas) in the constructor, and replaces each Gwarmacces (100 gas) with a PUSH32 (3 gas)

https://github.com/code-423n4/2022-07-golom/blob/7bbb55fca61e6bae29e57133c1e45806cbb17aa4/contracts/core/GolomTrader.sol#:~:text=bytes32%20public-,immutable,-EIP712_DOMAIN_TYPEHASH%3B

-> COMPARISON OPERATORS Problem

In the EVM, there is no opcode for >= or <=. When using greater than or equal, two operations are performed: > and =. Using strict comparison operators hence saves gas

https://github.com/code-423n4/2022-07-golom/blob/7bbb55fca61e6bae29e57133c1e45806cbb17aa4/contracts/governance/GolomToken.sol#:~:text=require(minterEnableDate-,%3C%3D,-block.timestamp

https://github.com/code-423n4/2022-07-golom/blob/7bbb55fca61e6bae29e57133c1e45806cbb17aa4/contracts/vote-escrow/VoteEscrowDelegation.sol#:~:text=1%5D.fromBlock-,%3C%3D,-blockNumber)%20%7B

https://github.com/code-423n4/2022-07-golom/blob/7bbb55fca61e6bae29e57133c1e45806cbb17aa4/contracts/core/GolomTrader.sol#:~:text=require(-,o.totalAmt%20%3E%3D%20o.exchange.paymentAmt,-%2B%20o.prePayment.paymentAmt%20%2B%20o.refererrAmt%20%2B

https://github.com/code-423n4/2022-07-golom/blob/7bbb55fca61e6bae29e57133c1e45806cbb17aa4/contracts/core/GolomTrader.sol#:~:text=msg.value-,%3E%3D,-o.totalAmt%20*

https://github.com/code-423n4/2022-07-golom/blob/7bbb55fca61e6bae29e57133c1e45806cbb17aa4/contracts/core/GolomTrader.sol#:~:text=require(amountRemaining-,%3E%3D,-amount%2C%20%27order

-> ++i costs less gas compared to i++ or i += 1 (Also --i costs less gas compared to i--- or i -= 1)

https://github.com/code-423n4/2022-07-golom/blob/7bbb55fca61e6bae29e57133c1e45806cbb17aa4/contracts/vote-escrow/VoteEscrowDelegation.sol#:~:text=delegatednft.length%3B-,index%2B%2B,-)%20%7B

https://github.com/code-423n4/2022-07-golom/blob/7bbb55fca61e6bae29e57133c1e45806cbb17aa4/contracts/vote-escrow/VoteEscrowDelegation.sol#:~:text=_array.length%3B-,i%2B%2B,-)%20%7B

https://github.com/code-423n4/2022-07-golom/blob/7bbb55fca61e6bae29e57133c1e45806cbb17aa4/contracts/core/GolomTrader.sol#:~:text=proof.length%3B-,i%2B%2B),-%7B

->SPLITTING REQUIRE() STATEMENTS THAT USE && SAVES GAS

https://github.com/code-423n4/2022-07-golom/blob/7bbb55fca61e6bae29e57133c1e45806cbb17aa4/contracts/vote-escrow/VoteEscrowDelegation.sol#:~:text=require(attachments%5B_tokenId%5D%20%3D%3D%200%20%26%26%20!voted%5B_tokenId%5D%2C%20%27attached%27)%3B

-> require()/revert() strings longer than 32 bytes cost extra gas