Juicebox V2 contest - sach1r0's results

The decentralized fundraising and treasury protocol.

General Information

Platform: Code4rena

Start Date: 01/07/2022

Pot Size: $75,000 USDC

Total HM: 17

Participants: 105

Period: 7 days

Judge: Jack the Pug

Total Solo HM: 5

Id: 143

League: ETH

Juicebox

Findings Distribution

Researcher Performance

Rank: 53/105

Findings: 2

Award: $128.10

🌟 Selected for report: 0

🚀 Solo Findings: 0

External Links

Code4rena Contest Page Github Contest Repository Github Findings Repository Juicebox

Findings Information

🌟 Selected for report: IllIllI

Also found by: 0v3rf10w, 0x1f8b, 0x29A, 0xDjango, 0xNazgul, 0xNineDec, 0xdanial, 0xf15ers, Bnke0x0, Ch_301, Chandr, Chom, Funen, GimelSec, Hawkeye, JC, Kaiziron, Lambda, Meera, MiloTruck, Noah3o6, Picodes, ReyAdmirado, Rohan16, Sm4rty, TerrierLover, TomJ, Waze, _Adam, __141345__, asutorufos, aysha, berndartmueller, brgltd, cccz, codexploder, defsec, delfin454000, djxploit, durianSausage, fatherOfBlocks, hake, horsefacts, hubble, jayfromthe13th, joestakey, jonatascm, m_Rassska, oyc_109, pashov, rajatbeladiya, rbserver, robee, sach1r0, sahar, samruna, simon135, svskaushik, zzzitron

Awards

89.271 USDC - $89.27

Labels

bug

QA (Quality Assurance)

valid

External Links

Link to GitHub issue

`setControllerOf` function does not check for zero address input

Details

It's important to check for zero-address to avoid redeploying of the contract when the address is accidentally set to zero-address.

Mitigation

Add a require statement or revert when input is address(0)

Line of code

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/blob/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBDirectory.sol#L211-L239

Lack of zero-address check in the constructor

Details

Lack of zero-address checks may lead to infunctional protocol.

Mitigation

Consider adding zero-address checks such as: require(_owner != address(0));

Line of code

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/blob/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBETHPaymentTerminal.sol#L30-L53 https://github.com/jbx-protocol/juice-contracts-v2-code4rena/blob/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBERC20PaymentTerminal.sol#L37-L63

Findings Information

🌟 Selected for report: 0xA5DF

Also found by: 0v3rf10w, 0x09GTO, 0x1f8b, 0x29A, 0xDjango, 0xKitsune, 0xNazgul, 0xdanial, 0xf15ers, Aymen0909, Bnke0x0, Ch_301, Cheeezzyyyy, Chom, ElKu, Funen, Hawkeye, IllIllI, JC, JohnSmith, Kaiziron, Lambda, Limbooo, Meera, Metatron, MiloTruck, Noah3o6, Picodes, Randyyy, RedOneN, ReyAdmirado, Rohan16, Saintcode_, Sm4rty, TomJ, Tomio, Tutturu, UnusualTurtle, Waze, _Adam, __141345__, ajtra, apostle0x01, asutorufos, brgltd, c3phas, cRat1st0s, codexploder, defsec, delfin454000, djxploit, durianSausage, exd0tpy, fatherOfBlocks, hake, horsefacts, ignacio, jayfromthe13th, joestakey, jonatascm, kaden, kebabsec, m_Rassska, mektigboy, mrpathfindr, oyc_109, rajatbeladiya, rbserver, rfa, robee, sach1r0, sashik_eth, simon135

Awards

38.8282 USDC - $38.83

Labels

bug

G (Gas Optimization)

valid

External Links

Link to GitHub issue

Functions that are not called within the contract must set its visibility to `external` instead of `public`

Details

Setting function's visibility to external when it is only called externally can save gas because external function’s parameters are not copied into memory and are instead read from calldata directly. see reference: https://github.com/code-423n4/2021-06-gro-findings/issues/37

Mitigation

Set function visibility to external

Line of code

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/blob/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBProjects.sol#L69

Use a more recent version of solidity

Details

Using a solidity version of at least 0.8.10 will have external calls skip contract existence checks if the external call has a return value. see reference: https://github.com/code-423n4/2022-04-axelar-findings/issues/39

Mitigation

I suggest using pragma solidity 0.8.10;

Line of code

No need to explicitly initialize variables with their default values

Details

When variables are not set, it is assumed to have it's default value(0 for uint, false for bool, address(0) for address). Explicitly initializing it with its default value is an anti-pattern and wastes gas.

Mitigation

change uint256 i = 0; to uint256 i; see reference: https://code4rena.com/reports/2022-02-jpyc/ [G-07] GENERAL RECOMMENDATIONS

Line of code

Pre-increment cost less gas than post-increment

Details

i++ costs more gas than ++i , for uint pre-decrement is cheaper than post-decrement see reference: https://github.com/code-423n4/2021-12-nftx-findings/issues/195

Mitigation

change i++ to ++i

Line of code

Solidity compiler will always read the length of the array during each iteration

Details

.length in a loop can be extracted into a variable and used where necessary to reduce the number of storage reads see reference: https://github.com/code-423n4/2021-10-union-findings/issues/92

Mitigation:

This extra costs can be avoided by caching the array length. Example: uint256 _currentSplitsLength = _currentSplits.length; for (uint256 _i = 0; i < _currentSplitsLength; _i++) { }

Line of code

Juicebox V2 contest - sach1r0's results

The decentralized fundraising and treasury protocol.

General Information

Findings Distribution

Researcher Performance

External Links

[Q-38] QA Report - $89.27

Findings Information

Awards

Labels

External Links

setControllerOf function does not check for zero address input

Details

Mitigation

Line of code

Lack of zero-address check in the constructor

Details

Mitigation

Line of code

[G-33] Gas Report - $38.83

Findings Information

Awards

Labels

External Links

Functions that are not called within the contract must set its visibility to external instead of public

Details

Mitigation

Line of code

Use a more recent version of solidity

Details

Mitigation

Line of code

No need to explicitly initialize variables with their default values

Details

Mitigation

Line of code

Pre-increment cost less gas than post-increment

Details

Mitigation

Line of code

Solidity compiler will always read the length of the array during each iteration

Details

Mitigation:

Line of code

`setControllerOf` function does not check for zero address input

Functions that are not called within the contract must set its visibility to `external` instead of `public`