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

8 stars 7 forks source link

It is possible to deploy address(0) as owners of a Console account and subAccount #322

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/dd0b41031b199a0aa214e50758943712f9f574a0/contracts/src/core/SafeDeployer.sol#L90-L92 https://github.com/code-423n4/2023-10-brahma/blob/dd0b41031b199a0aa214e50758943712f9f574a0/contracts/src/core/SafeDeployer.sol#L82-L103

Vulnerability details

Impact

address(0) is allowed to be an owner of a console account and subAccount

Proof of Concept

In both deployConsoleAccount function and deploySubAccount function in the SafeDeploy contract, there is no check against passing address(0) as owners or part of the owners.

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

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

Tools Used

Manual review

Recommended Mitigation Steps

I suggest a check against having address(0) as an 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 duplicate of #17

c4-judge commented 10 months ago

alex-ppg marked the issue as not a duplicate

alex-ppg commented 10 months ago

This check is taken care of by the Gnosis Safe deployer directly, alongside threshold restrictions and other such rudimentary sanitizations against the deployment of a Gnosis Safe.

c4-judge commented 10 months ago

alex-ppg marked the issue as unsatisfactory: Invalid