Summary
User can call installModule on Nexus contract using execute function. This will invoke withHook code twice with unexpected results depending on hook implementation.
Root Cause
https://github.com/Cyfrin/2024-07-biconomy/blob/9590f25cd63f7ad2c54feb618036984774f3879d/contracts/Nexus.sol#L119-L130
User can craft userOp that EXECUTES installModule in Nexus contract.
function execute(ExecutionMode mode, bytes calldata executionCalldata) external payable onlyEntryPointOrSelf withHook {
(CallType callType, ExecType execType) = mode.decodeBasic();
if (callType == CALLTYPE_SINGLE) {
_handleSingleExecution(executionCalldata, execType);
} else if (callType == CALLTYPE_BATCH) {
_handleBatchExecution(executionCalldata, execType);
} else if (callType == CALLTYPE_DELEGATECALL) {
_handleDelegateCallExecution(executionCalldata, execType);
} else {
revert UnsupportedCallType(callType);
}
}
function installModule(uint256 moduleTypeId, address module, bytes calldata initData) external payable onlyEntryPointOrSelf {
_installModule(moduleTypeId, module, initData);
emit ModuleInstalled(moduleTypeId, module);
}
function _installModule(uint256 moduleTypeId, address module, bytes calldata initData) internal withHook {
if (module == address(0)) revert ModuleAddressCanNotBeZero();
if (moduleTypeId == MODULE_TYPE_VALIDATOR) {
_installValidator(module, initData);
} else if (moduleTypeId == MODULE_TYPE_EXECUTOR) {
_installExecutor(module, initData);
} else if (moduleTypeId == MODULE_TYPE_FALLBACK) {
_installFallbackHandler(module, initData);
} else if (moduleTypeId == MODULE_TYPE_HOOK) {
_installHook(module, initData);
} else if (moduleTypeId == MODULE_TYPE_MULTI) {
_multiTypeInstall(module, initData);
} else {
revert InvalidModuleTypeId(moduleTypeId);
}
}
In one function call the withHook code will run twice. It is invoked during call to execute and later in _installModule internal function.
Impact
The impact depends on the hook implementation. It can cause loss of funds or open up a possible exploit scenario for an attacker.
Mitigation
One solution to that problem could be changing access control of installModule function. Change onlyEntryPointOrSelf to onlyEntryPoint.
