`SubAccount` signers can backdoor the safe to execute any transaction

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-10-brahma/blob/a6424230052fc47c4215200c19a8eef9b07dfccc/contracts/src/core/TransactionValidator.sol#L181-L198 https://github.com/code-423n4/2023-10-brahma/blob/a6424230052fc47c4215200c19a8eef9b07dfccc/contracts/interfaces/external/IGnosisSafe.sol#L14-L21

Vulnerability details

The security checks made in the SafeModerator#checkAfterExecution are not enough to protect the integrity of the Safe/Brahma Account and allows the signer to bypass any policy commitment validation and opens a backdoor to access the wallet account.

Impact

The checkAfterExecution() of a Brahma SubAccount Gnosis Safe is intended to uphold important invariants after each signer transaction is completed but it lacks the check if a new module has being added. In the case such module is added it opens a backdoor trough the call execTransactionFromModule. The later will bypass any security restriction whatsoever.

Proof of Concept

This is the code which tries to protect the security integrity of a SubAccount Gnosis Safe wallet/account:

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

As it can be seen it does not check if the latest transaction has added new module/s.

And by adding a new module it can call Gnosis#execTransactionFromModule which Allows a Module to execute a Safe transaction without any further confirmations. :

https://github.com/code-423n4/2023-10-brahma/blob/a6424230052fc47c4215200c19a8eef9b07dfccc/contracts/interfaces/external/IGnosisSafe.sol#L14-L21

Tools Used

Manual review

Recommended Mitigation Steps

Check that no new modules have been introduced to the safe, using the getModulesPaginated() that Gnosis Wallet provide. Take a hash of all modules at the beginning and then in the checkAfterExecution callback retake the hash of all existing modules and verify that is not changed.

If this is not an option maybe and changing modules is a must then add a registry for whitelisted (by the Console Account owner) modules.

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

[c4-judge]

alex-ppg marked the issue as duplicate of #412

[c4-judge]

alex-ppg marked the issue as unsatisfactory: Invalid