drgorillamd
commented
2 years ago All the findings are incorrect:
- the parameters are used by the constructor (for all the 'immutable should be initialised') and are not part of this contract (but the inherited one)
- the visibility change concerns internal function used by inherited contract, changing them to external would make them inaccessible...
1 State directory and operatorstore in public immutable stored properties was missing
Impact constructor cant initialize the directory and operatorStore due to state of directory and operatorstore was missing
Proof of Concept https://github.com/jbx-protocol/juice-contracts-v2-code4rena/blob/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBTokenStore.sol#L160
Tool Used Manual review
Recommendation Mitigation Steps Add directory and operatorStore state and make it immutable in public immutable stored properties
2 State operatorstore in public immutable stored properties was missing
Impact constructor cant initialize the operatorStore due to state of operatorstore was missing
Proof of Concept https://github.com/jbx-protocol/juice-contracts-v2-code4rena/blob/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBSplitsStore.sol#L120
Tool Used Manual review
Recommendation Mitigation Steps Add operatorStore state and make it immutable in public immutable stored properties
3 State owner was missing
Impact constructor cant initialize the owner due to state of owner was missing
Proof of Concept https://github.com/jbx-protocol/juice-contracts-v2-code4rena/blob/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBPrices.sol#L89
Tool Used Manual review
Recommendation Mitigation Steps Add state owner and make it immutable
4 Missing state of owner and operatorStore
Impact constructor cant initialize the owner and operatorStore due to state of owner and operatorstore was missing
Proof of Concept https://github.com/jbx-protocol/juice-contracts-v2-code4rena/blob/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBDirectory.sol#L182
Tool Used Manual review
Recommendation Mitigation Steps Add owner and operatorStore state and make it immutable in public immutable stored properties
5 Missing state operatorStore
Impact constructor cant initialize the operatorStore due to state of operatorstore was missing
Proof of Concept https://github.com/jbx-protocol/juice-contracts-v2-code4rena/blob/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBController.sol#L369
Tool Used Manual review
Recommendation Mitigation Steps Add owner and operatorStore state and make it immutable in public immutable stored properties
6 Missing msg.value = 0
Impact cant save gas due to missing msg.value==0 in constructor
Proof of Concept https://github.com/jbx-protocol/juice-contracts-v2-code4rena/blob/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/abstract/JBPayoutRedemptionPaymentTerminal.sol#L284
Tool Used Manual review
Recommendation Mitigation Steps add require(msg.value==0, msg.value must be 0)
7 Must be external
Impact Public functions that are never called by the contract should be declared external to make cheaper gas.
Proof of Concept https://github.com/jbx-protocol/juice-contracts-v2-code4rena/blob/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBETHPaymentTerminal.sol#L75
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/blob/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBERC20PaymentTerminal.sol#L85
Tool Used Manual Review
Recommendation Mitigation Steps change visibility internal to external
8 Missing emit
Impact Off-chain tools will not work as expected.
Proof of Concept https://github.com/jbx-protocol/juice-contracts-v2-code4rena/blob/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/abstract/JBPayoutRedemptionPaymentTerminal.sol#L540
Tool Used Manual Review
Recommendation Mitigation Steps Add emit addToBalanceOf(_projectId, _amount, token, !isFeelessAddress[msg.sender], _memo, _metadata);