SafeEnabler's `setGuard()` function can be called by anyone and anytime

[c4-submissions]

Lines of code

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

Vulnerability details

Impact

setGuard() function inside the SafeEnabler contract can be called by anyone.

Proof of Concept

The contract SafeEnabler is responsible for holding the bytecode to enable modules and guards for a Gnosis Safe. But this is a little bit changed in Brahma contracts to bypass the initialization procedure:

*  bypasses Safe selfAuthorized check which disallows setting up guard during initialization
*  Refer https://github.com/safe-global/safe-contracts/blob/186a21a74b327f17fc41217a927dea7064f74604/contracts/base/ModuleManager.sol#L32C5-L32C5

Now if we look at the function setGuard() we will see that there is just one check:

    function setGuard(address guard) public {
        _onlyDelegateCall();

        bytes32 slot = _GUARD_STORAGE_SLOT;
        // solhint-disable-next-line no-inline-assembly
        assembly {
            sstore(slot, guard)
        }
        emit ChangedGuard(guard);
    }

As we can see the function's visibility is set to be public and the only check is it should be called using delegatecall. It also lacks an initializer modifier to prevent it being called after initialization. Anyone thus can call this function and set their addresses as the guard in a gnosis safe.

Tools Used

Manual

Recommended Mitigation Steps

Consider putting a restriction to authorized people or put an initializer modifier if you wish not to be called after initialization period

Assessed type

Invalid Validation

[c4-pre-sort]

raymondfam marked the issue as low quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #104

[c4-judge]

alex-ppg marked the issue as unsatisfactory: Invalid