Some functions in RocketJoeFactory.sol have zero checks for setting specific state variables, but there zero address checks are not always applied. Setting some of these state variables to the zero address, whether intentional or not, can break the protocol functionality. Adding these checks consistently would prevent this scenario.
Proof of Concept
The constructor in RocketJoeFactory.sol performs zero address checks before setting the router, factory, penaltyCollector, and rJoe state variables. Later in the same contract, the functions setRJoe(), setPenaltyCollector(), setRouter(), and setFactory()omit the same zero address checks that were applied earlier. Since the issues that can be caused by setting these state variables to the zero address exist whether setting the value in the constructor or in the setter function, these checks should be applied consistently.
Recommended Mitigation Steps
Add zero address checks in the setter functions for these state variables just like is done in the constructor. If it is determined that a zero check for any of these state variables is not needed, then the zero check can be removed from the RocketJoeFactory.sol constructor for consistency.
cryptofish7
commented
2 years ago
Handle
sirhashalot
Vulnerability details
Impact
Some functions in RocketJoeFactory.sol have zero checks for setting specific state variables, but there zero address checks are not always applied. Setting some of these state variables to the zero address, whether intentional or not, can break the protocol functionality. Adding these checks consistently would prevent this scenario.
Proof of Concept
The constructor in RocketJoeFactory.sol performs zero address checks before setting the router, factory, penaltyCollector, and rJoe state variables. Later in the same contract, the functions
setRJoe(),setPenaltyCollector(),setRouter(), andsetFactory()omit the same zero address checks that were applied earlier. Since the issues that can be caused by setting these state variables to the zero address exist whether setting the value in the constructor or in the setter function, these checks should be applied consistently.Recommended Mitigation Steps
Add zero address checks in the setter functions for these state variables just like is done in the constructor. If it is determined that a zero check for any of these state variables is not needed, then the zero check can be removed from the RocketJoeFactory.sol constructor for consistency.