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

Nexus creation will always revert due to `_initModuleManager()` is called twice.

zukanopro

Summary

When initaizing Nexus contract, function _initModuleManager()is called twice, lead to Nexus contract cant be deployed due to checking condition inside that function.

Vulnerability Details

In **NexusAccountFactory**contract (RegistryFactory and K1ValidatorFactory are the same), user can create Nexus contract by using createAccount()function:

function createAccount(bytes calldata initData, bytes32 salt) external payable override returns (address payable) {
// Compute the actual salt for deterministic deployment
bytes32 actualSalt;
assembly {
let ptr := mload(0x40)
let calldataLength := sub(calldatasize(), 0x04)
mstore(0x40, add(ptr, calldataLength))
calldatacopy(ptr, 0x04, calldataLength)
actualSalt := keccak256(ptr, calldataLength)
}
​
// Deploy the account using the deterministic address
(bool alreadyDeployed, address account) = LibClone.createDeterministicERC1967(msg.value, ACCOUNT_IMPLEMENTATION, actualSalt); // <---
​
if (!alreadyDeployed) {
INexus(account).initializeAccount(initData); // <---
emit AccountCreated(account, initData, salt);
}
return payable(account);
}

It will deploy contract and call initializeAccount()function if address havent deployed yet.

Nexus constructor:

constructor(address anEntryPoint) {
_SELF = address(this);
require(address(anEntryPoint) != address(0), EntryPointCanNotBeZero());
_ENTRYPOINT = anEntryPoint;
_initModuleManager(); // <--
}

initializeAccount() function:

function initializeAccount(bytes calldata initData) external payable virtual {
_initModuleManager(); // <---
(address bootstrap, bytes memory bootstrapCall) = abi.decode(initData, (address, bytes));
(bool success, ) = bootstrap.delegatecall(bootstrapCall);
require(success, NexusInitializationFailed());
}

It can be seen that _initModuleManager()function will be initalized twice, look at implement of _initModuleManager()function:

function _initModuleManager() internal virtual {
// account module storage
AccountStorage storage ams = _getAccountStorage();
ams.executors.init(); // <---
ams.validators.init(); // <---
}

Look at struct of AccountStorage:

struct AccountStorage {
SentinelListLib.SentinelList validators; ///< List of validators, initialized upon contract deployment.
SentinelListLib.SentinelList executors; ///< List of executors, similarly initialized.
mapping(bytes4 => FallbackHandler) fallbacks; ///< Mapping of selectors to their respective fallback handlers.
IHook hook; ///< Current hook module associated with this account.
}

init()function: (Link: https://github.com/rhinestonewtf/sentinellist/blob/main/src/SentinelList.sol#L30-#L34)

function init(SentinelList storage self) internal {
if (alreadyInitialized(self)) revert LinkedList_AlreadyInitialized();
self.entries[SENTINEL] = SENTINEL;
}

with SENTINEL = address(0x1);and function alreadyInitialized():

function alreadyInitialized(SentinelList storage self) internal view returns (bool) {
return self.entries[SENTINEL] != ZERO_ADDRESS;
}

It can be seen that if function init()is called twice, it will revert due to checking condition in int() function.

Impact

Unable to deploy Nexus contract.

Tools Used

Manual review.

Recommendations

While initalize Nexus contract, only call _initModuleManager()function once.

Updates

Lead Judging Commences

0xnevi Lead Judge 11 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
Assigned finding tags:

finding-initializeAccount-revert

Invalid, if a `Nexus.sol` contract is already deployed, it does not need to be initialized again within the factory as seen [here](https://github.com/Cyfrin/2024-07-biconomy/blob/9590f25cd63f7ad2c54feb618036984774f3879d/contracts/factory/NexusAccountFactory.sol#L58-L60), so no issue here

Support

FAQs

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