Orchestrator_v1.sol#initiateAddModuleWithTimelock() - indefinite amounts of privileged modules can be added

[hats-bug-reporter[bot]]

Github username: @PlamenTSV Twitter username: @p_tsanev Submission hash (on-chain): 0x9b871e35b67ba70c4c6d699c71f6dfc078961e685c1410edccdcda2d043cf6fd Severity: low

Description: Description\ The initiateAddModuleWithTimelock() is meant to add new logic modules to the Orchestrator. It's counter-parts for privileged modules:

• initiateSetPaymentProcessorWithTimelock
• initiateSetFundingManagerWithTimelock
• initiateSetAuthorizerWithTimelock all remove the old module and add the new one. However the initiateAddModuleWithTimelock() never ensures that the new added module is indeed a logic module and is not a privileged module.

Attack Scenario\ initiateAddModuleWithTimelock():

function _initiateAddModuleWithTimelock(address module)
        internal
        __ModuleManager_onlyAuthorized
        moduleLimitNotExceeded
        isNotModule(module)
        validModule(module)
    {
        _startModuleUpdateTimelock(module);
    }

As it can be seen, the function only checks the validity, dupes and limit of modules. However if the new module is a privileged module, it can be freely added without removing the old privileged onces, differing from the intended behavior.

Attachments

1. Proof of Concept (PoC) File

2. Revised Code File (Optional)

Recommendation\ Aswell as checking if the module supports the IModule_v1 interface, it should also be checked if the module supports one of the privileged interfaces and revert if it does.

[FHieser]

isnt this a copy of #77 or #78

[PlamenTSV]

77 and #78 talk about completely different functions than the ones mentioned above. Fixing them does not fix this issue here.