Closed c4-submissions closed 12 months ago
raymondfam marked the issue as low quality report
raymondfam marked the issue as duplicate of #368
alex-ppg marked the issue as not a duplicate
alex-ppg marked the issue as duplicate of #412
alex-ppg marked the issue as unsatisfactory: Invalid
Lines of code
https://github.com/code-423n4/2023-10-brahma/blob/main/contracts/src/core/TransactionValidator.sol#L181
Vulnerability details
Impact
The
_checkSubAccountSecurityConfig
function ensures that the guard and fallback handler have not been disabled or updated, and that the owner console as a module has not been disabled by any operators or executors. This helper function is used as a last resort if the trusted validator mistakenly signs malicious transactions, given that there is only one trusted validator in the system. However, it does not check if operators or executors has enabled new modules to the subaccount, which can execute any transactions without any validations. As a result, any new module introduced is free to execute whatever it wishes on the subaccount. It constitutes a serious backdoor threat. Hence, it is important to add the following invariant to the system:Subacount should not allow operators and executors to intruduce any modules on it.
Proof of Concept
https://github.com/code-423n4/2023-10-brahma/blob/main/contracts/src/core/TransactionValidator.sol#L181
Tools Used
None
Recommended Mitigation Steps
Check that no new modules have been introduced to the subaccount, using the getModulesPaginated() utility.
Assessed type
Access Control