Skip to content

Latest commit

 

History

History
92 lines (69 loc) · 5.14 KB

M-10.md

File metadata and controls

92 lines (69 loc) · 5.14 KB
Raw

Account creation in RegistryFactory doesn't validate the initialization data targets a known bootstrapper

Summary

The factory fails to verify if the underlying initialization call is executed using a known Bootstrap contract.

Vulnerability Details

The createAccount() function in the RegistryFactory contract decodes the initData sent to the bootstrap function during account initialization in order to validate the modules present in the arguments.

79:     function createAccount(bytes calldata initData, bytes32 salt) external payable override returns (address payable) {
80:         // Decode the initData to extract the call target and call data
81:         (, bytes memory callData) = abi.decode(initData, (address, bytes));
82: 
83:         // Extract the inner data by removing the first 4 bytes (the function selector)
84:         bytes memory innerData = BytesLib.slice(callData, 4, callData.length - 4);
85: 
86:         // Decode the call data to extract the parameters passed to initNexus
87:         (
88:             BootstrapConfig[] memory validators,
89:             BootstrapConfig[] memory executors,
90:             BootstrapConfig memory hook,
91:             BootstrapConfig[] memory fallbacks,
92:             ,
93:             ,
94: 
95:         ) = abi.decode(innerData, (BootstrapConfig[], BootstrapConfig[], BootstrapConfig, BootstrapConfig[], address, address[], uint8));

Line 81 decodes the given initData into an address (the address of the bootstrapper) and a byte array (the actual calldata sent to the bootstrapper). This is aligned with the structure of how an account is initialized in initializeAccount(). The implementation then proceeds to execute the validations on the modules, once the arguments present in the calldata have been decoded.

However, while proper care is taken to check the modules, the implementation never validates that the target present in initData is an actual Bootstrap contract known to the factory. The RegistryFactory contract then allows initialization calls to any arbitrary implementation, with completely different or unknown semantics.

Additionally, it is important to note that the implementation never validates that the first 4 bytes of the calldata actually correspond to the selector of the initNexus() function. This could also be used to target a different function in the bootstrapper contract, as long as the arguments fit.

Impact

The RegistryFactory contract fails to verify if the account initialization call is executed using a known and secure Bootstrap implementation, allowing calls to any arbitrary contract.

Note that calls to the bootstrap contract during account initialization are executed using a delegatecall, making them particularly delicate. A malicious contract could easily implement a backdoor in the account.

Tools Used

None.

Recommendations

The constructor of RegistryFactory should receive and store a reference to a known Bootstrap contract.

    contract RegistryFactory is Stakeable, INexusFactory {
        /// @notice Address of the implementation contract used to create new Nexus instances.
        /// @dev This address is immutable and set upon deployment, ensuring the implementation cannot be changed.
        address public immutable ACCOUNT_IMPLEMENTATION;
+       address public immutable BOOTSTRAPPER;

        IERC7484 public immutable REGISTRY;
        address[] public attesters;
        uint8 public threshold;

        /// @notice Error thrown when a non-whitelisted module is used.
        /// @param module The module address that is not whitelisted.
        error ModuleNotWhitelisted(address module);

        /// @notice Constructor to set the smart account implementation address and owner.
        /// @param implementation_ The address of the Nexus implementation to be used for all deployments.
        /// @param owner_ The address of the owner of the factory.
-       constructor(address implementation_, address owner_, IERC7484 registry_, address[] memory attesters_, uint8 threshold_) Stakeable(owner_) {
+       constructor(address implementation_, address bootstrapper_, address owner_, IERC7484 registry_, address[] memory attesters_, uint8 threshold_) Stakeable(owner_) {
            require(implementation_ != address(0), ImplementationAddressCanNotBeZero());
+           require(bootstrapper_ != address(0), ZeroAddressNotAllowed());
            require(owner_ != address(0), ZeroAddressNotAllowed());
            REGISTRY = registry_;
+           BOOTSTRAPPER = bootstrapper_;
            attesters = attesters_;
            threshold = threshold_;
            ACCOUNT_IMPLEMENTATION = implementation_;
        }

When the account is created, it should check that the target present in the initData matches the stored reference.

    function createAccount(bytes calldata initData, bytes32 salt) external payable override returns (address payable) {
        // Decode the initData to extract the call target and call data
-       (, bytes memory callData) = abi.decode(initData, (address, bytes));
+       (address target, bytes memory callData) = abi.decode(initData, (address, bytes));

+       require(target == BOOTSTRAPPER);