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

8 stars 7 forks source link

Parmanent Ownership #192

Closed c4-submissions closed 9 months ago

c4-submissions commented 10 months ago

Lines of code

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

Vulnerability details

Impact

Each owner of the multisig wallet is permanent. A mechanism to update a list of owners is not present in the contract. This can be catastrophic when any of the owner's private key is compromised.

Proof Of Concept

Lets imagine a scenario where for example if any of the owners or worst case senario (but unlikely event) all the owners lose control of their multisig-assigned accounts or disappear (die, for example), then all the funds are lost or stuck in the wallet.

Tools Used

Manual Review

Recommended Mitigation Steps

Implement a mechanism to manage and update ownership of the multisig wallet. This mechanism can require a certain number of threshold to be met to update any of the owner.

Assessed type

Other

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 primary issue

raymondfam commented 10 months ago

Intended design.

alex-ppg commented 9 months ago

This is related to the Gnosis Safe implementation and is incorrect as a whole given that owners can be properly maintained within it.

c4-judge commented 9 months ago

alex-ppg marked the issue as unsatisfactory: Invalid