Missing a check in `_checkSubAccountSecurityConfig` that no new modules have been introduced to the safe

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-10-brahma/blob/dd0b41031b199a0aa214e50758943712f9f574a0/contracts/src/core/TransactionValidator.sol#L181-L199

Vulnerability details

Impact

Adding a new module opens up a backdoor to the SubAccount, allowing the new module to execute any action on the SubAccount.

Proof of Concept

The checkAfterExecution function calls validatePostTransaction in the TransactionValidator.sol file, which in turn invokes the _checkSubAccountSecurityConfig function. However, in the _checkSubAccountSecurityConfig function, there is a missing check to ensure that no new modules have been introduced to the safe. When modules execute transactions on a Gnosis safe, the guard safety callbacks are not triggered. Consequently, any new module introduced is free to execute any action it wishes on the safe, creating a serious backdoor threat to the security of the safe.

    function _checkSubAccountSecurityConfig(address _subAccount) internal view {
        address guard = SafeHelper._getGuard(_subAccount);
        address fallbackHandler = SafeHelper._getFallbackHandler(_subAccount);

        // Ensure guard has not been disabled
        if (guard != AddressProviderService._getAuthorizedAddress(_SAFE_MODERATOR_HASH)) revert InvalidGuard();

        // Ensure fallback handler has not been altered
        if (fallbackHandler != AddressProviderService._getAuthorizedAddress(_CONSOLE_FALLBACK_HANDLER_HASH)) {
            revert InvalidFallbackHandler();
        }

        address ownerConsole =
            WalletRegistry(AddressProviderService._getRegistry(_WALLET_REGISTRY_HASH)).subAccountToWallet(_subAccount);

        // Ensure owner console as a module has not been disabled
        if (!IGnosisSafe(_subAccount).isModuleEnabled(ownerConsole)) revert InvalidModule();
    }

Tools Used

Manual Review

Recommended Mitigation Steps

Check that no new modules have been introduced to the safe, using the getModulesPaginated()

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 #39

[c4-judge]

alex-ppg marked the issue as not a duplicate

[c4-judge]

alex-ppg marked the issue as duplicate of #412

[c4-judge]

alex-ppg marked the issue as unsatisfactory: Invalid