Biconomy: Nexus
Biconomy
HardhatFoundry
30,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

Issue in Module Check Between Nexus.sol and ModuleManager.sol

eta

Summary

The ModuleManager contract’s _isModuleInstalled function currently checks for four module types: validator, executor, fallback, and hook, but it does not check for the MultiType module, which is identified as type 0. This discrepancy leads to revert errors in the Nexus contract's isModuleInstalled and uninstallModule functions when dealing with MultiType modules.

Vulnerability Details

The ModuleManager contract’s _isModuleInstalled function omits the MultiType module from its checks, although MultiType is a recognized type (type 0). Consequently, when the Nexus contract calls isModuleInstalled to check for a MultiType module, it encounters errors. Additionally, the uninstallModule function in the Nexus contract relies on _isModuleInstalled to verify module types before uninstallation, leading to further complications.

ModuleManager::_isModuleInstalled

function _isModuleInstalled(uint256 moduleTypeId, address module, bytes calldata additionalContext) internal view returns (bool) {
additionalContext;
if (moduleTypeId == MODULE_TYPE_VALIDATOR) return _isValidatorInstalled(module);
else if (moduleTypeId == MODULE_TYPE_EXECUTOR) return _isExecutorInstalled(module);
else if (moduleTypeId == MODULE_TYPE_FALLBACK) {
bytes4 selector;
if (additionalContext.length >= 4) {
selector = bytes4(additionalContext[0:4]);
} else {
selector = bytes4(0x00000000);
}
return _isFallbackHandlerInstalled(selector, module);
}
else if (moduleTypeId == MODULE_TYPE_HOOK) return _isHookInstalled(module);
else return false; // @audit if check MultiType, it returns false
}

Nexus::isModuleInstalled

function isModuleInstalled(uint256 moduleTypeId, address module, bytes calldata additionalContext) external view returns (bool) {
@audit=> return _isModuleInstalled(moduleTypeId, module, additionalContext);
}
​

Nexus::uninstallModule

function uninstallModule(uint256 moduleTypeId, address module, bytes calldata deInitData) external payable onlyEntryPointOrSelf {
require(IModule(module).isModuleType(moduleTypeId), MismatchModuleTypeId(moduleTypeId));
@audit=> require(_isModuleInstalled(moduleTypeId, module, deInitData), ModuleNotInstalled(moduleTypeId, module));
​
emit ModuleUninstalled(moduleTypeId, module);
​
if (moduleTypeId == MODULE_TYPE_VALIDATOR) {
_uninstallValidator(module, deInitData);
} else if (moduleTypeId == MODULE_TYPE_EXECUTOR) {
_uninstallExecutor(module, deInitData);
} else if (moduleTypeId == MODULE_TYPE_FALLBACK) {
_uninstallFallbackHandler(module, deInitData);
} else if (moduleTypeId == MODULE_TYPE_HOOK) {
_uninstallHook(module, deInitData);
}
}
​

Impact

The absence of MultiType module checks in the ModuleManager contract can cause the Nexus contract’s isModuleInstalled function to return false when verifying MultiType modules. This issue can also affect the uninstallModule function, potentially preventing to direct uninstallation of MultiType modules like _multiTypeInstall and causing unexpected behavior or system malfunctions.

Tools Used

Manual Review

Recommendations

  1. Enhance _isModuleInstalled Function: Update the _isModuleInstalled function in the ModuleManager contract to include checks for the MultiType module, ensuring it is recognized as a valid module type.

  2. Add _uninstallMultiType Function: Implement a _uninstallMultiType function in the ModuleManager contract, mirroring the functionality of _multiTypeInstall, to handle the uninstallation of MultiType modules correctly.

Updates

Lead Judging Commences

0xnevi Lead Judge
over 1 year ago
0xnevi Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.