Juicebox V2 contest - fatherOfBlocks'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: 32/105

Findings: 3

Award: $173.37

🌟 Selected for report: 0

🚀 Solo Findings: 0

Awards

3.4075 USDC - $3.41

Labels

bug
duplicate
2 (Med Risk)
valid

External Links

Lines of code

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

Vulnerability details

Impact

As we have not defined the token implementation, it should be validated that IERC20(token).transfer() and IERC20(token).transferFrom(); return true. This is important, since the transaction could not be carried out and the function executed as correct. In the comments of the code it says that it is not necessary, but we cannot be sure that the implementation that comes whether or not revert is implemented in the event that it fails, we can only get rid of the interface and the interface specifies that it returns a bool, therefore it must be validated.

Wrap the transfer and transferFrom in a require or validate it with an if and a revert with a custom error.

#0 - mejango

2022-07-12T18:24:03Z

dup of #281

JBTokenStore.sol

  • L160 - In the constructor it is not validated that the addresses are zero and the variables in storage are immutable, therefore if they are set with an address that does not comply with the interfaces, it would generate a DoS.

JBDirectory.sol

  • L182 - In the constructor it is not validated that the addresses are zero and the variables in storage are immutable, therefore if they are set with an address that does not comply with the interfaces, it would generate a DoS.

JBController.sol

  • L369 - In the constructor it is not validated that the addresses are zero and the variables in storage are immutable, therefore if they are set with an address that does not comply with the interfaces, it would generate a DoS.

JBPayoutRedemptionPaymentTerminal.sol

  • L6/15 - There are imports that are not used in all the code, for example: IERC20, JBSplitsGroups.

  • L283 - In the constructor it is not validated that the addresses are zero and the variables in storage are immutable, therefore if they are set with an address that does not comply with the interfaces, it would generate a DoS.

JBSingleTokenPaymentTerminalStore.sol

  • L276 - In the constructor it is not validated that the addresses are zero and the variables in storage are immutable, therefore if they are set with an address that does not comply with the interfaces, it would generate a DoS.

JBTokenStore.sol

  • L353 - The operation can be unchecked since the subtraction was previously validated, therefore an underflow cannot be generated.

  • L356/367 - The variable operation > 0 generates more gas costs than if it is done: variable != 0

  • L442/445/474/477 - It is not necessary to create a variable, if it is only going to be used once, in this way gas is saved.

JBProjects.sol

  • L40 - It is not necessary to set a variable with its default value, since it already has it and generates an extra gas cost.

  • L143 - The variable operation > 0 generates more gas costs than if it is done: variable != 0

JBFundingCycleStore.sol

  • L111/114/257/260/636/639 - It is not necessary to create a variable, if it will only be used once, in this way gas is saved.

  • L149/210/347/348/364/469/560/589/601 - The variable operation > 0 generates more gas costs than if it is done: variable != 0

  • L679/683 - The operation can be unchecked since the subtraction was previously validated, therefore an underflow cannot be generated.

  • L724 - It is not necessary to set a variable with its default value, since it already has it and generates an extra gas cost.

  • L724 - Instead of i++, less gas costs are generated using {++i;} this is because it is not necessary to validate overflow and underflows when the number that can be reached is too large.

  • L752 - Instead of variable + 1, it is less expensive to make ++variable, in this way less gas is used.

JBSplitsStore.sol

  • L165/204/209/211/227/229/304 - It is not necessary to set a variable with its default value, since it already has it and generates an extra gas cost.

  • L204/211/229/304 - Instead of i++, less gas costs are generated using {++i;} this is because it is not necessary to validate overflow and underflows when the number that can be reached is too large.

  • L204/211/229 - Within a for loop, it is less expensive to create a length variable and not go through each iteration to obtain the length of the array.

  • L259/272/326 - The variable operation > 0 generates more gas costs than if it is done: variable != 0

JBOperatorStore.sol

  • L85/135/165 - It is not necessary to set a variable with its default value, since it already has it and generates an extra gas cost.

  • L85/135/165 - Instead of i++, less gas costs are generated using {++i;} this is because it is not necessary to validate overflow and underflows when the number that can be reached is too large.

  • L85/135/165 - Within a for loop, it is less expensive to create a length variable and not go through each iteration to obtain the length of the array.

JBDirectory.sol

  • L139/167/275/276 - Instead of i++, less gas costs are generated using {++i;} this is because it is not necessary to validate overflow and underflows when the number that can be reached is too large.

  • L139/167/274/275/276 - Within a for loop, it is less expensive to create a length variable and not go through each iteration to obtain the length of the array.

  • L226/232/262/267/360/365 - It is not necessary to create a variable, if it is only going to be used once, in this way gas is saved.

JBController.sol

  • L239/246/599/602/732/736/810/813 - It is not necessary to create a variable, if it will only be used once, this way gas is saved.

  • L438/481/498/875/925/1032/1040 - The variable operation > 0 generates more gas costs than if it is done: variable != 0

  • L913/1014 - Instead of i++, less gas costs are generated using {++i;} this is because it is not necessary to validate overflow and underflows when the number that can be reached is too large.

  • L913/1014 - Within a for loop, it is less expensive to create a length variable and not go through each iteration to obtain the length of the array.

  • L913 - It is not necessary to set a variable with its default value, since it already has it and generates an extra gas cost.

JBPayoutRedemptionPaymentTerminal.sol

  • L71 - The modifier generates a lot of unnecessary gas cost, you can save gas if you use a private view function.

  • L346/516/552/745/772/884/964/1022/1297 - The variable operation > 0 generates more gas costs than if it is done: variable != 0

  • L594/1008/1396/1469/1475 - It is not necessary to set a variable with its default value, since it already has it and generates an extra gas cost.

  • L1008 - Within a for loop, it is less expensive to create a length variable and not go through each iteration to obtain the length of the array.

  • L945/956/1045/1058/1110/1118/1126/1229/1232 - It is not necessary to create a variable, if it is only going to be used once, this way gas is saved.

JBSingleTokenPaymentTerminalStore.sol

  • L260/264/385/390/721/724 - It is not necessary to create a variable, if it will only be used once, this way gas is saved.

  • L475/518 - The variable operation > 0 generates more gas costs than if it is done: variable != 0

  • L862 - It is not necessary to set a variable with its default value, since it already has it and generates an extra gas cost.

  • L862 - Within a for loop, it is less expensive to create a length variable and not go through each iteration to obtain the length of the array.

  • L862 - Instead of i++, less gas costs are generated using {++i;} this is because it is not necessary to validate overflow and underflows when the number that can be reached is too large.

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax © 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter