QA Report #64
Labels
bug
Something isn't working
QA (Quality Assurance)
Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax
Summary of Findings
Issue#1 : Null address check missing while assigning new Governance address in LensHub.sol
Issue#2 : Donot allow any transactions when state is not changed in ModuleGlobals.sol
Issue#3 : Implementation contracts of FollowNFT and CollectNFT deployed, remains open for initialization
Details Issue#1
Title : Null address check missing while assigning new Governance address in LensHub.sol
Impact
In the event of wrongly assigning ZERO address to the governance address, the governance control of the Lens protocol will be lost.
Proof of Concept
Contract : LensHub.sol
line 79 :
line 850 :
Recommended Mitigation Steps
Add a require statement to check for null address, in function _setGovernance().
As a best practice the two step standard process for changing the Governance address is recommended.
Details Issue#2
Title : Donot allow any transactions when state is not changed in ModuleGlobals.sol
In the set functions in ModuleGlobals.sol, there is no check if the new Value being set is different than previous Value.
Allow changing to new value only if its different than prev value.
Impact
Unnecessary event generated even if no state is changed
Proof of Concept
Contract : ModuleGlobals.sol
line 94 : function _setGovernance(address newGovernance)
line 101 : function _setTreasury(address newTreasury)
line 108 : function _setTreasuryFee(uint16 newTreasuryFee)
line 115 : function _whitelistCurrency(address currency, bool toWhitelist)
Recommended Mitigation Steps
Add a require statement to check if prev value is same as new value, example below.
if (prevWhitelisted == toWhitelist ) revert Errors.ChangeParamsInvalid()
Details Issue#3
Title : Implementation contracts of FollowNFT and CollectNFT deployed, remains open for initialization.
Risk:
The implementation contracts of FollowNFT and CollectNFT, that is passed as constructor parameter of LensHub, they remain uninitialized and open for initialization. Since these contracts are used extensively for Cloning, it is better to avoid its initialization and keep its state as it was originally deployed.
Proof of Concept
Contract : FollowNFT.sol and CollectNFT.sol
Function : initialize()
Recommended Mitigation Steps
Mitigation: Add the following lines of code to FollowNFT and CollectorNFT's constructor
The text was updated successfully, but these errors were encountered: