0xdanial - Juicebox V2 contest

Findings Information

🌟 Selected for report: 0xNineDec

Also found by: 0x1f8b, 0x29A, 0x52, 0xDjango, 0xdanial, 0xf15ers, Cheeezzyyyy, Chom, Franfran, GalloDaSballo, Green, IllIllI, Meera, Ruhum, bardamu, cccz, codexploder, defsec, hake, hansfriese, horsefacts, hubble, hyh, jonatascm, kebabsec, oyc_109, pashov, rbserver, simon135, tabish, tintin, zzzitron

Awards

14.8726 USDC - $14.87

Labels

bug

duplicate

3 (High Risk)

valid

External Links

Link to GitHub issue

Lines of code

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

Vulnerability details

Impact

when we use the latestRoundData we should always check the returned value because it may return stale data. (, int256 _price, , , ) = feed.latestRoundData();

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept.

Recommended Mitigation Steps

function currentPrice(uint256 _decimals) external view override returns (uint256) {
   
(uint80 roundID, int256 _price, , uint256 timestamp, uint80 answeredInRound) = feed.latestRoundData();
require(answeredInRound >= roundID, "Stale price");
require(timestamp != 0,"Round not complete");
require(_price > 0,"Chainlink price reporting 0");

#0 - mejango
2022-07-12T18:45:54Z

dup #138

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

Check for the approve of tokens

Impact: some of the tokens returns false when approve gets failed. Mitigation: add the checker for approve results.

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

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.2308 USDC - $38.23

Labels

bug

G (Gas Optimization)

valid

External Links

Link to GitHub issue

Mitigation:

replace != 0 to .when you want to check inequality with zero. POC: it saves about 6 gas for each instance it means saves about 474 gas in total contracts in scope. instances of this issue:

Mitigation: replace != 0 to .when you want to check inequality with zero.

Use ++variable instead of variable++ in for loops.

impact It saves about 57 gas for each and it means in total about 741 gas in all scopes.

POC

The instances of this issue:

mitigation: for (uint256 _i; _i < _terminalsOf[_projectId].length; ++i)

Use one mapping instead of Multiple mapping

Impact: this action save the storage slot

poc instances of issues: jbcontroller.sol jbdirectry.sol

array.length should not be looked up in every single for Loop.

storage arrays incur a Gwarmaccess (100 gas) memory arrays use MLOAD (3 gas) calldata arrays use CALLDATALOAD (3 gas)

instances of issue:

Mitigation : 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

Puts bool after the address

impact : the we can put two parameters(address , bool) in one 32 byte and its save gas

mitigation: always put the bool variable after the address variable address payable beneficiary; bool preferAddToBalance;

Juicebox V2 contest - 0xdanial's results

The decentralized fundraising and treasury protocol.

General Information

Findings Distribution

Researcher Performance

External Links

Findings Information

Awards

Labels

External Links

Lines of code

Vulnerability details

Impact

Proof of Concept

Recommended Mitigation Steps

#0 - mejango
2022-07-12T18:45:54Z

Findings Information

Awards

Labels

External Links

Check for the approve of tokens

Findings Information

Awards

Labels

External Links

Mitigation:

Use ++variable instead of variable++ in for loops.

Use one mapping instead of Multiple mapping

Puts bool after the address

Juicebox V2 contest - 0xdanial's results

The decentralized fundraising and treasury protocol.

General Information

Findings Distribution

Researcher Performance

External Links

[H-01] latestRoundData should have returned value checks - $14.87

Findings Information

Awards

Labels

External Links

Lines of code

Vulnerability details

Impact

Proof of Concept

Recommended Mitigation Steps

#0 - mejango2022-07-12T18:45:54Z

[Q-57] QA Report - $89.27

Findings Information

Awards

Labels

External Links

Check for the approve of tokens

[G-73] Gas Report - $38.23

Findings Information

Awards

Labels

External Links

Mitigation:

Use ++variable instead of variable++ in for loops.

Use one mapping instead of Multiple mapping

Puts bool after the address

#0 - mejango
2022-07-12T18:45:54Z