Brahma - sorrynotsorry's results

Lines of code

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

Vulnerability details

Impact

The assets might be stolen

Proof of Concept

The SafeDeployer contract internally utilizes _createSafe function when the deploySubAccount and deployConsoleAccount functions are called.

_createSafe serves for creating a new Gnosis Safe. Here's the NATSPEC of the function;

Contract: SafeDeployer.sol

207:     /**
208:      * @notice Internal function to create a new Gnosis Safe.
209:      * @dev SafeDeployer calls createProxyWithNonce to deploy a new Gnosis Safe. This also contains initializer bytes
210:      *  which are used during creation to setup the safe with owners and threshold. An actor can precompute the salt
211:      *  for a given set of owners and deploy the safe. We choose to not consider that safe as a valid safe and deploy a new
212:      *  safe. In case the actor chooses to deploy multiple precomputed safes with bumped nonces, the transaction will run out
213:      *  of gas and user can retry with a new random salt
214:      *  To generate deterministic addresses for a given set of owners, the order of owner addresses and threshold should be same
215:      * @param _owners list of owners addresses
216:      * @param _salt salt to be used during creation of safe, to generate nonce
217:      * @return _safe The address of the created Gnosis Safe.
218:      */
219:     function _createSafe(address[] calldata _owners, bytes memory _initializer, bytes32 _salt)

Accordingly, the function uses user provided data to create the salt for the deterministic address creation. And it continues to try to create the Safe account until the the TX runs out of the gas if the deterministic address is already taken. The _genNonce function increments the ownerSafeCount mapping which is served for generating nonce based on owners and user provided salt.

However, this flow is problematic due to proxy creation being public at the Gnossis Safe contracts.

• Let's say a user provided their address in _owners calldata and their salt on blockA and created their console account via deployConsoleAccount
• The user initiates bridging their funds from their other console account on other chain at blockA+n which is irreversible.
• A re-org occurs at the receiving/existing network with a depth of m blocks where m > n and the states roll back m blocks,
• Accordingly, the state changed and the console account is said to never exist while the funds are sent to the account.
• An actor listening to bridging and safe deployment events (ProxyCreation) tries out providing same salt on Gnosis SafeProxyFactory contract via calling createProxyWithNonce and calls setup and grants the ownership of the console account that the funds are flowing from other network (or with the setup initiliazer data, just wanted to refer to the Gnossis contract) This leads to the funds being under the custody of the actor.

The likeliness of the reorgs is dependent on the network. The state of the chains in the flow-originating chain might change due to their consensus mechanisms or fraudulent blocks known as reorgs.

While the Polygon chain suffers the reorg issue every day, the hard fork for reorg mitigation also didn't resolve the issue. They had the largest reorg after the hard fork for 157 blocks.

It still continues to have reorgs as can be tracked on this page also confirming the frequency.;

Optimistic rollups (Optimism/Arbitrum) are also suspect to reorgs since if someone finds a fraud the blocks will be reverted, even though the user receives a confirmation and already performed a cross-chain message.

Tools Used

Manual Review

Recommended Mitigation Steps

Not sure but a vault could be utilized to register these bridging transactions rather than sending the funds to the console account itself at the first deployment. OR Brahma can run a script for each console/subaccount deployment call that will try out the same TX as msg.sender being the user address for m blocks in order to back up the user.

Assessed type

Other