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

8 stars 7 forks source link

Signed data may be usable cross-chain #335

Closed c4-submissions closed 1 year ago

c4-submissions commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-10-brahma/blob/a6424230052fc47c4215200c19a8eef9b07dfccc/contracts/src/libraries/TypeHashHelper.sol#L64-L77 https://github.com/code-423n4/2023-10-brahma/blob/a6424230052fc47c4215200c19a8eef9b07dfccc/contracts/src/libraries/TypeHashHelper.sol#L23-L31

Vulnerability details

Impact

The function validatePreTransactionOverridable(), which Validates a txn on guard before execution, for Brahma console accounts.takes one parameter "txParams" which is of type SafeTransactionParams Struct, if we look at that struct members : struct SafeTransactionParams { Enum.Operation operation; address from; address to; address payable refundReceiver; address gasToken; address msgSender; uint256 value; uint256 safeTxGas; uint256 baseGas; uint256 gasPrice; bytes data; bytes signatures; }

there is no chainId identifier, this is also the case with the following functions: -function validatePreTransaction(SafeTransactionParams memory txParams) // which Validates a txn on guard before execution, for subAccounts. -function validatePreExecutorTransaction( address, /msgSender / address from, bytes32 transactionStructHash, bytes memory signatures ) // which Validates a module txn before execution.

And as the Brahma Console is multi-chain ( Target Chains:Ethereum,Optimism,Base,Avalanche, C Chan,Polygon Mainnet,Arbitrum,Polygon zkEVM,Binance Smart Chain,Fantom), This issue can lead to many problems such as:

  1. The signature can be replayed on other EVM-compatible chains on which Brahma Console protocol is deployed.
  2. Users can transfer their assets - by mistake- to another chain where they may be lost forever.

Proof of Concept

In "ethereum.stackexchange.com" there are many questions asking about a way to return their assets that they mistakenly sent to the wrong Gnosis Safe on another chain, here is an example: https://ethereum.stackexchange.com/questions/117781/how-to-recover-funds-mistakenly-sent-to-polygon-instead-of-eth-gnosis-safe-1-1?rq=1 this proves that it is already an issue that needs to be handled in Gnosis Safe and as Brahma Console stated in the docs " The SafeDeployer contract facilitates the deployment of Gnosis Safe accounts and configuring them as console accounts" Then most likely there will be the same issue unless it is mitigated beforehand.

of course, the mentioned issue is about innocent mistake, the same vulnerability can be used by a malicious attacker to steal the protocol users.

Tools Used

Manual review

Recommended Mitigation Steps

Assessed type

Other

c4-pre-sort commented 1 year ago

raymondfam marked the issue as low quality report

c4-pre-sort commented 1 year ago

raymondfam marked the issue as duplicate of #143

c4-pre-sort commented 1 year ago

raymondfam marked the issue as duplicate of #398

c4-judge commented 12 months ago

alex-ppg marked the issue as unsatisfactory: Invalid