Brahma - ZanyBonzy's results

Brahma Console is a custody and DeFi execution environment.

General Information

Platform: Code4rena

Start Date: 13/10/2023

Pot Size: $31,250 USDC

Total HM: 4

Participants: 51

Period: 7 days

Judge: 0xsomeone

Id: 295

League: ETH

Brahma

Findings Distribution

Researcher Performance

Rank: 43/51

Findings: 1

Award: $14.47

Analysis:
grade-b

🌟 Selected for report: 0

🚀 Solo Findings: 0

Awards

14.466 USDC - $14.47

Labels

analysis-advanced
grade-b
sufficient quality report
edited-by-warden
A-23

External Links

Brahma Console

Brahma Console v2 is a platform that helps people use decentralized finance (DeFi) more easily and safely. It does this by providing a layer of automation and risk management on top of smart contract wallets. It provides sub-accounts, which isolate user interactions to reduce risk. The Console is built on the principle of non-custodiality, which means that users shouldn't have to give up control of their funds to execute transactions.

It consists of four major account types:

  1. Console account - a Gnosis Safe that is owned by a number of users, and can be safeguarded by a safeModeratorOverridable if the owners want to;
  2. Sub-account -also a Gnosis Safe owned by the console account, controlled by operators, and has the console account as a safe module and a SafeModerator as a safeguard;
  3. Operator account - an account that acts as a delegate of a sub-account. Its rights are restricted by safeModerator and controlled by the console account;
  4. Executor account - an external account that makes module transactions on the sub-account via th executorplugin which needs to be enabled before use.

System Overview

Scope

Core Contracts
  1. AddressProvider.sol - is the gatekeeper of the system's authorized contracts and registries. It ensures that only the right addresses are allowed to interact with the system, and that all changes to the addresses are properly authorized and implemented;
  2. AddressProvideService.sol - is an abstract contract that provides a common interface for all core contracts to interact with the AddressProvider contract. It does this by providing a host of helper functions to help in these interactions;
  3. SafeDeployer.sol - does the actual work of deploying and registering console accounts (with or without policies) and sub-accounts with policies. It sets up the required states - safeguards, fallback handler, and module for these accounts;
  4. SafeModerator.sol - is the safeguard for a sub-account. It validates transaction before and after execution by making sure that the sub-account's state remain as is and is policy compliant. It does this through the TransactionValidator contract;
  5. SafeModeratorOverridable.sol - is the safeguard for a console account that has safeguard enabled (Its not compulsory). It has the same functions as the safeModerator contract. On top of this, it also does a check to confirm if an owner decides override the it as a safeguard and/or the ConsoleFallbackHandler as the fallback handler.
  6. TransactionValidator.sol - is the contract called by the safeguards to validate state/policies before and after executions. It is also called by an executor through the executorPlugin for sub acoounts module executions.
  7. PolicyValidator.sol - validates the Trusted Validator's signature against the policies. It ensures policy compliance based on EIP712 sugnatures before executing transactions.
  8. ExecutorPlugin - acts as a gatekeeper that ensures that only authorized executors can submit valid execution requests on console accounts. It also verifies that the requests are valid and that the executor has the necessary permissions to execute them.
  9. SafeEnabler.sol - makes delegatecalls to the ModuleManager and GuardManager to set up modules and safeguard states. This helps the accounts bypass the selfAuthorized checks that exist on the actual Safe.
  10. Consolefallbackenabler.sol - is a fallback handler for Safe accounts. It performs the same functions as the Safe's CompatibilityFallbackHandler, but it also ensures that policy validation is performed for ConsoleAccounts and sub-accounts that have policy validation enabled;
Registry contracts
  1. WalletRegistry.sol - manages registrations and ownership relationships of/between wallets and their sub-accounts
  2. PolicyRegistry.sol - allows authorized entities to set and update policy commitments for specific accounts.
  3. ExecutorRegistry.sol - manages the registration and removal of executor addresses by owners for their desired sub-accounts.
Library contracts
  1. SafeHelper.sol - is a helper library that provides essential functions for interacting with Safe. This includes executing transactions, generating calldata, obtaining necessary storage slots, and parsing data.
  2. TypeHasHelper.sol - builds struct hashes for generating EIP712 digests which are required for signature validations.
Roles
  1. Trusted Validator - is a trusted third party account that can ensures that transactions on sub-accounts and console accounts are valid. It's signature needs to be verified in PolicyValidator before execution.
  2. Guardian - is in charge of all emergency actions to ensure contract security.
  3. Governace - is brahma owned, configures addresses, funds accounts and performs a host of other privileged functions.

Architecture review

Console and Sub account Transactions
  1. execTransaction is called by a console account without policies and safeguard in the GnosisSafe.sol contract. The desired call datas execute without any safety checks;

  2. execTransaction is called by a console account with policies and safeguard. Here, the execTransaction function performs the needed safety checks before and after executing the transactions;

    • The execTransaction calls checkTransaction function in the SafeModeratorOverridable.sol;
    • The SafeModeratorOverridable.sol's checkTransaction function calls the validatePreTransactionOverridable function in the TransactionValidator.sol;
    • In the TransactionValidator.sol contract, validatePreTransactionOverridable checks if the guard isn't being removed with the help of the _isConsoleBeingOverriden. This checks to see if the required guard removal conditions are met. As, the guard isn't being removed, the _validatePolicySignature is called.
    • The _validatePolicySignature calls the isPolicySignatureValid in the PolicyValidator.sol contract. This validates the policyHash and the Trusted Validator Signature.
    • After the check, the desired calldata is executed.
    • After the calldata is successfully executed, execTransaction function calls the checkAfterExecution function in SafeModeratorOverridable.sol contract. This calls the validatePostTransactionOverridable in the TransactionValidator.sol contract.

    Transaction function flow

    execTransaction → checkTransaction → validatePreTransactionOverridable ==> _isConsoleBeingOverriden (No) ==> _validatePolicySignature → isPolicySignatureValid → execTransaction (desired calldatas are executed) → checkAfterExecution → validatePostTransactionOverridable

    Transaction contract flow

    GnosisSafe → SafeModeratorOverridable → TransactionValidator <==> TransactionValidator <==> TransactionValidator → PolicyValidator → GnosisSafe → SafeModeratorOverridable → TransactionValidator

  3. execTransaction is called by a console account with policies and safeguard, while also disabling the safeguard - This occurs almost as above with the added caveat that the safeguard is overriden. The action of overriding the safeguard skips the policy validation, and disables the guard by setting guard address to 0. However unlike stated in the readme, after guard is disabled. It doesn't seem to perform the checkAfterExecution. This is because before performing checkAfterExecution, there's a check to see if guard address isn't 0. If 0, the checkAfterExecution is skipped.

    Transaction function flow

    execTransaction → checkTransaction → validatePreTransactionOverridable ==> _isConsoleBeingOverriden (Yes) == _validatePolicySignature =→ isPolicySignatureValid → execTransaction (desired calldatas are executed, setGuard(address(0)))

    Transaction contract flow

    GnosisSafe → SafeModeratorOverridable → TransactionValidator <==> TransactionValidator → TransactionValidator =→ PolicyValidator → GnosisSafe

  4. execTransactionFromModuleReturnData is called by the console account via the sub accouunt using the ModuleManager.sol. The function executes the desired calldata and returns returnData to ConsoleAccount.

  5. execTransaction is called by operator through the sub-account - This tranaction occurs just like console account exectransaction (see â„–2) above. The only difference is that instead of the checks being conducted by the SafeModeratorOverridable.sol contract, it is conducted by the SafeModerator.sol instead.

    • The execTransaction calls checkTransaction function in the SafeModerator.sol;
    • The SafeModerator.sol checkTransaction function calls the validatePreTransaction function in the TransactionValidator.sol;
    • The TransactionValidator.sol contract validates the policy signature through the _validatePolicySignature. which calls the isPolicySignatureValid in the PolicyValidator.sol contract. This validates the policyHash and the Trusted Validator Signature.
    • After the check, the desired calldata is executed.
    • After the calldata is successfully executed, execTransaction function calls the checkAfterExecution function inSafeModerator.sol contract. This calls the validatePostTransaction in the TransactionValidator.sol contract. This checks the security config by calling the _checkSubAccountSecurityConfig function. It makes sure guard, fallbackhandlers and the console has not be disabled as module.

    Transaction function flow

    execTransaction → checkTransaction → validatePreTransaction ==> _validatePolicySignature → isPolicySignatureValid → execTransaction (desired calldatas are executed) → checkAfterExecution → validatePostTransaction ==> _checkSubAccountSecurityConfig

    Transaction contract flow

    GnosisSafe → SafeModerator → TransactionValidator <==> TransactionValidator → PolicyValidator → GnosisSafe → SafeModerator → TransactionValidator <==> TransactionValidator

  6. executeTransaction is called the executor on the ExecutorPlugin.sol enabled as module on the sub-account.

    • The Executor calls executeTransaction, this validates the signer executor by calling the _validateExecutionRequest, where it's determined if the executor is valid for the sub-account by checking with the ExecutorRegistry.sol, validates the executor's signature and validates the sub-account's policy by calling the validatePreExecutorTransaction function in the TransactionValidator.sol function.
    • Through the validatePreExecutorTransaction function, the policy signature is validated by the _validatePolicySignature. which calls the isPolicySignatureValid in the PolicyValidator.sol contract.
    • After a successful check, the ExecutorPlugin.sol executes transaction on the sub-account by calling _executeTxnAsModule function.
    • After execution, the ExecutorPlugin calls validatePostExecutorTransaction function on TransactionValidator. to check that SafeModerator hasn't been removed as guard on SubAccount and ConsoleAccount hasn't been removed as module on SubAccount.

    Transaction function flow

    executeTransaction → _validateExecutionRequest → validatePreExecutorTransaction ==> _validatePolicySignature → isPolicySignatureValid → executeTransaction (desired calldatas are executed)(_executeTxnAsModule) → validatePostExecutorTransaction → _checkSubAccountSecurityConfig

    Transaction contract flow

    ExecutorPlugin <==> ExecutorPlugin (checks with ExecutorRegistry) → TransactionValidator <==> TransactionValidator → PolicyValidator → ExecutorPlugin → TransactionValidator <==> TransactionValidator

Codebase quality analysis

The codebase is of high quality, well-structured, and would-be large functions are carefully divided into separate contracts. Required functions are either directly called or delegate called when needed. The codebase is well-commented, and its documentation is easy to follow. There are also unit tests for the contracts, which is very commendable. Error handling is fine. The codebase uses a mixture of custom, require, and revert errors. It's better to stick to one format, preferable custom errors as they're gas effiecient. In a host of cases, the errors weren't very descriptive, some without messages, some with cryptic messages like "GS102". An error also appears to be unused. Events were also well-handled, some events didn't fully emit needed parameters, some didn't have a lot of declarations or were well-handled. In general, NatSpec and the Style guide were followed, not 100%, but just enough to make the contract look well-organized. We recommend using linters and static analysis tools that can help note and fix these issues before deployment.

Systemic risks & Centralization risks

The risk of centralization seems to be limited to the Brahma-owned governance and guardian. Any malicious actions (intended or not) can pose a risk to the protocol and its users. Some contracts inherit third-party contracts like OpenZeppelin, Solidity, GnosisSafe contracts. Any issue found in these inheritances might pose a threat to the protocol. One thing to also note is that although wallets and sub-accounts can be added, they cannot be removed. Not sure if that is intended by the sponsor, but it might cause a potential issue in case of some problem to keep the console secure.

Conclusion

We started the audit process by thoroughly reviewing the provided documentation, tests, noting various important parts, and asking the sponsors questions. We used static analysis tools and linters to audit the codebase, noted the bot reports, and went about with the manual code inspection. We carefully inspected each section of the codebase, tested possible attack vectors, focusing both on the sponsor's interested areas of attack (through imported wallets) and areas we deemed important. We took notes of our findings, tested them out, and prepared our analysis. Overall, the console contract looks to be well-written and doesn't have a lot of serious issues. We recommend the team goes through these, make appropriate modifications where needed before deployment.

Time spent:

36 hours

#0 - c4-pre-sort

2023-10-22T21:21:11Z

raymondfam marked the issue as sufficient quality report

#1 - alex-ppg

2023-10-27T13:10:59Z

This report feels more like a rudimentary analysis of individual contracts rather than a solid overview of the Brahma system from a security perspective.

#2 - c4-judge

2023-10-27T13:11:12Z

alex-ppg marked the issue as grade-b

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax © 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter