c4-submissions commented 11 months ago

Lines of code

https://github.com/code-423n4/2023-10-brahma/blob/dd0b41031b199a0aa214e50758943712f9f574a0/contracts/src/core/SafeDeployer.sol#L168-L205

Vulnerability details

According to the Contest Overview,

Executor - An account authorized to make module transactions on a subAccount via ExecutorPlugin, ExecutorPlugin needs to be enabled as module on subAccount.

For ExecutorPlugin to make module transactions, it needs to be enabled as a module on the subaccount. During subaccount creation, _setupSubAccount() sets the fallback handler and guard on the subaccount, but fails to set the ExecutorPlugin as module.

  function _setupSubAccount(address[] memory _owners, uint256 _threshold, address _consoleAccount)
    private
    view
    returns (bytes memory)
{
    address safeEnabler = AddressProviderService._getAuthorizedAddress(_SAFE_ENABLER_HASH);
    Types.Executable[] memory txns = new Types.Executable[](2);

    // Enable Brhma Console account as module on sub Account
    txns[0] = Types.Executable({
        callType: Types.CallType.DELEGATECALL,
        target: safeEnabler,
        value: 0,
        data: abi.encodeCall(IGnosisSafe.enableModule, (_consoleAccount))
    });

    // Enable guard on subAccount
    txns[1] = Types.Executable({
        callType: Types.CallType.DELEGATECALL,
        target: safeEnabler,
        value: 0,
        data: abi.encodeCall(IGnosisSafe.setGuard, (AddressProviderService._getAuthorizedAddress(_SAFE_MODERATOR_HASH)))
    });

    return abi.encodeCall(
        IGnosisSafe.setup,
        (
            _owners,
            _threshold,
            AddressProviderService._getAuthorizedAddress(_GNOSIS_MULTI_SEND_HASH),
            abi.encodeCall(IGnosisMultiSend.multiSend, (SafeHelper._packMultisendTxns(txns))),
            AddressProviderService._getAuthorizedAddress(_CONSOLE_FALLBACK_HANDLER_HASH),
            address(0),
            0,
            address(0)
        )
    );
}

Impact

This leads to a situation where subaccounts are created but lack ExecutorPlugin set as a module, this prevents executors from executing transactions as a module.

Tools Used

Manual Review

Recommended Mitigation Steps

A call to set ExecutorPlugin as a module should be added to the _setupSubAccount() function to ensure that every new subaccount has ExecutorPlugin enabled.