Console account cannot execute a transaction on a sub account unless it registers itself as an executor

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-10-brahma/blob/main/contracts/src/core/ExecutorPlugin.sol#L69 https://github.com/code-423n4/2023-10-brahma/blob/main/contracts/src/core/ExecutorPlugin.sol#L112

Vulnerability details

The Executor is an account authorized to make module transactions on a subAccount via ExecutorPlugin. The executor is assigned/registered by the subaccount created by the console account. But the console account itself cannot execute the transaction & is dependent on the executor to carry it out because if the console account tries to execute, the transaction will revert.

Proof of Concept

executeTransaction() nables executors to raise execution requests that will be executed via a module transaction. It calls _validateExecutionRequest(execRequest); to validate the request.

File: ExecutorPlugin.sol

    function executeTransaction(ExecutionRequest calldata execRequest) external nonReentrant returns (bytes memory) {
        _validateExecutionRequest(execRequest);         /// @note - issue arises here

        bytes memory txnResult = _executeTxnAsModule(execRequest.account, execRequest.exec);

        TransactionValidator(AddressProviderService._getAuthorizedAddress(_TRANSACTION_VALIDATOR_HASH))
            .validatePostExecutorTransaction(msg.sender, execRequest.account);

        return txnResult;
    }

In _validateExecutionRequest(), the if condition checks if the executor is valid for a given account & reverts otherwise.

File: ExecutorPlugin.sol

     function _validateExecutionRequest(ExecutionRequest calldata execRequest) internal {
        // Fetch executor registry
        ExecutorRegistry executorRegistry =
            ExecutorRegistry(AddressProviderService._getRegistry(_EXECUTOR_REGISTRY_HASH));

        // Check if executor is valid for given account
        if (!executorRegistry.isExecutor(execRequest.account, execRequest.executor)) {   /// @note - returns false for console account
            revert InvalidExecutor();
        }
        ..........

The issue lies that the creator of the subaccount that is the console account itself cannot execute the transaction on its subaccount if the subaccount does not registers the console account as an executor. The isExecutor() will returns false in such a situation.

File: ExecutorRegistry.sol

    function isExecutor(address _subAccount, address _executor) external view returns (bool) {
        return subAccountToExecutors[_subAccount].contains(_executor);
    }

Impact

The transaction will always revert if a console account tries to execute a transaction on its subaccount.

Tools Used

Manual Review

Recommended Mitigation Steps

Automatically add the owner of the subaccount (console account) as an executor.

Assessed type

Error

[c4-pre-sort]

raymondfam marked the issue as low quality report

[raymondfam]

Invalid assumptions.

[c4-pre-sort]

raymondfam marked the issue as primary issue

[alex-ppg]

While the Warden's statement is correct in relation to the ExecutorPlugin, the SafeDeployer::deploySubAccount function will deploy a sub-account with the Console Account registered as a module.

As such, the Console Account can invoke the ModuleManager::execTransactionFromModule function to f.e. perform a transaction on behalf of the Console Account via the sub-account. This fact is missed by the Warden rendering their submission invalid.

[c4-judge]

alex-ppg marked the issue as unsatisfactory: Invalid