Juicebox contest - carlitox477's results

[LOW] JB721Delegate#initialize _fundingCycleStore lack of zero address check can lead to redeployment

Impact

initialize function does not check that _fundingCycleStore is not zero. Given that state variable fundingCycleStore can not be set anywhere else, setting it to zero can lead to contract redeployment

POC

The deployer mistakenly call JB721Delegate#initialize with _fundingCycleStore = IJBFundingCycleStore(0) as parameter, then mintReservesFor and _beforeTokenTransfer will always revert

Tools

Manual review.

Mitigation steps

Add a require statement require(_fundingCycleStore != IJBFundingCycleStore(0)) in JB721Delegate#initialize function

[LOW] Lack of zero-address check in LB721Delegate#_initialize function

Checking addresses against zero-address during initialization or during setting is a security best-practice. However, such checks are missing in address variable initializations/changes.

Impact

Allowing zero-addresses will lead to contract reverts and force redeployments if there are no setters for such address variables.

Affected code

Setting zero address will lead to redeployment, and payable functions didPay and didRedeem will be unusable.

Tools used

Manual review

Mitigation steps

Add zero address check.

[Non-critical] JB721TierdGovernance#delegateTier can emit wrong DelegateChanged event.

The function does not check that _oldDelegate != _delegatee, emiting a wrong DelegateChanged event.

[Non-critical] TierDelegateVotesChanged wrong parameter name

While TierDelegateVotesChanged last parameter should changed from callre to caller.

[Non-critical] JB721TieredGovernance#_moveTierDelegateVotes Wrong event emission

While TierDelegateVotesChanged is defined as

event TierDelegateVotesChanged(
    address indexed delegate,
    uint256 indexed tierId,
    uint256 previousBalance,
    uint256 newBalance,
    address callre
);

This line set parameters wrong. it should be changed to:

emit TierDelegateVotesChanged(_from, _tierId, _oldValue, _newValue, , msg.sender);

JBTiered721DelegateDeployer#constructor and JBTiered721DelegateProjectDeployer#constructor lack of address zero check

JBTiered721DelegateDeployer#constructor JBTiered721DelegateProjectDeployer#constructor

Checking addresses against zero-address during initialization or during setting is a security best-practice. However, such checks are missing in address variable initializations/changes

Lack of state variable projectId caching in JB721Delegate functions

State variable projectId is accessed twice in functions didPay and didRedeem.

Add uint256 _projectId = projectId; at the start of both functions, and replace every occurency of projectId in this functions for the new variable.

Lack of state variable pricingCurrency caching in JBTiered721Delegate function

pricingCurrency can be put in a memory variable in function _processPayment to avoid double access to state variable.

Lack of state variable store caching in JBTiered721Delegate functions

store can be put in a memory variable to avoid double access to state variable. In functions

• JBTiered721Delegate#mintReservesFor: Lines 448 and 451
• JBTiered721Delegate#_beforeTokenTransfer: Lines 731, 744 and 745
• JBTiered721Delegate#_afterTokenTransfer