code-423n4 / 2023-10-brahma-findings

8 stars 7 forks source link

Everyone can disable policy of any brahama console account #459

Closed c4-submissions closed 10 months ago

c4-submissions commented 10 months ago

Lines of code

https://github.com/code-423n4/2023-10-brahma/blob/a6424230052fc47c4215200c19a8eef9b07dfccc/contracts/src/core/ConsoleOpBuilder.sol#L68

Vulnerability details

Impact

Everyone can disable policy of any brahama console account

if you look at the function disablePolicyOnConsole it designed to disable the policy and set guards to 0 which is important decision of any account but the problem is everyone can disable random people account cause there is No validation of caller at all at any kind.

badactor can disable every accounts policy when they want and set the gnosis guards to 0 address

  /**
     * @notice Provides multicall bytecode for disabling a policy on brahma console account
     * @dev Disables policy, by setting guard to address(0) on Safe, setting fallback handler to address(0) on Safe
     * @param account address of brahma console account
     * @return callData for multicall execution
     */

Proof of Concept

https://github.com/code-423n4/2023-10-brahma/blob/a6424230052fc47c4215200c19a8eef9b07dfccc/contracts/src/core/ConsoleOpBuilder.sol#L68

 /**
     * @notice Provides multicall bytecode for disabling a policy on brahma console account
     * @dev Disables policy, by setting guard to address(0) on Safe, setting fallback handler to address(0) on Safe
     * @param account address of brahma console account
     * @return callData for multicall execution
     */
    function disablePolicyOnConsole(address account) external pure returns (bytes memory) {
        Types.Executable[] memory txns = new Types.Executable[](2);

        txns[0] = Types.Executable({
            callType: Types.CallType.CALL,
            target: account,
            value: 0,
            data: abi.encodeCall(IGnosisSafe.setGuard, (address(0)))
        });

        txns[1] = Types.Executable({
            callType: Types.CallType.CALL,
            target: account,
            value: 0,
            data: abi.encodeCall(IGnosisSafe.setFallbackHandler, (address(0)))
        });

        return abi.encodeCall(IGnosisMultiSend.multiSend, (SafeHelper._packMultisendTxns(txns)));
    }

Tools Used

vscode

Recommended Mitigation Steps

Assessed type

Access Control

0xad1onchain commented 10 months ago

Invalid, all this function does is provide calldata to execute the operation, not actually execute it

0xad1onchain commented 10 months ago

Also the given calldata can only be executed by main console account and would revert if any one else executes it

c4-pre-sort commented 10 months ago

raymondfam marked the issue as low quality report

c4-pre-sort commented 10 months ago

raymondfam marked the issue as duplicate of #144

c4-pre-sort commented 10 months ago

raymondfam marked the issue as not a duplicate

c4-pre-sort commented 10 months ago

raymondfam marked the issue as primary issue

alex-ppg commented 10 months ago

As the Sponsor has specified, this function simply crafts an ABI-encoded payload and does not really execute anything. Additionally, the relevant smart contract is out-of-scope per the official C4 contest scope.

c4-judge commented 10 months ago

alex-ppg marked the issue as unsatisfactory: Out of scope