Salt not incorporated into address generation; allows address pre-computation.

[c4-submissions]

Lines of code

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

Vulnerability details

Impact

The salt provided for account creation is only used for nonce generation. An attacker could pre-compute addresses for a victim's safe owners and front-run deployment. The salt should also be incorporated into the address generation.

Proof of Concept

The salt usage for nonce generation in the _createSafe function: https://github.com/code-423n4/2023-10-brahma/blob/c217699448ffd7ec0253472bf0d156e52d45ca71/contracts/src/core/SafeDeployer.sol#L219-L245

function _createSafe(address[] calldata _owners, bytes memory _initializer, bytes32 _salt)
        private
        returns (address _safe) {

  // Generate nonce
  uint256 nonce = _genNonce(ownersHash, _salt); 

  // Use nonce to create safe
  IGnosisProxyFactory(gnosisProxyFactory).createProxyWithNonce(
     gnosisSafeSingleton, 
     _initializer,
     nonce
  );

}

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

function _genNonce(bytes32 _ownersHash, bytes32 _salt) private returns (uint256) {

  return uint256(keccak256(abi.encodePacked(_ownersHash, ownerSafeCount[_ownersHash]++, _salt, VERSION)));

}

As you can see, the _salt is only used as an input to generate the nonce. It is not incorporated into the final safe address itself.

This allows an attacker to pre-compute addresses by brute forcing salts offline, then deploying safes using those salts to claim the pre-computed addresses with victims' owners.

The use of salt can be exploited. Here is why:

The key functions involved in safe creation are _createSafe and _genNonce:

function _createSafe(/*...*/) private returns (address) {

  uint256 nonce = _genNonce(_ownersHash, _salt);

  // Create safe using nonce
  IGnosisProxyFactory.createProxyWithNonce(
    /*...*/,
    nonce
  );

}

function _genNonce(bytes32 _ownersHash, bytes32 _salt) private returns (uint256) {

  return uint256(keccak256(abi.encodePacked(
    _ownersHash, 
    ownerSafeCount[_ownersHash]++, 
    _salt,
    VERSION
  )));

}

As you can see, the _salt provided by the caller is only used as an input to generate the nonce. The nonce is then passed to the ProxyFactory's createProxyWithNonce to deploy the safe contract itself.

The _salt does not directly affect the generated safe address - it is only used for the nonce.

This allows an attacker to pre-compute safe addresses for a victim's owner list by brute forcing different _salt values offline to generate the corresponding nonces and addresses.

For example:

• Attacker gets victim's owner list [A, B, C]
• Attacker iterates 1 trillion random salts locally, generating the corresponding nonces and predicted safe addresses for [A, B, C].
• Attacker finds a collision address matching the victim's predicted safe address.
• Attacker uses the salt that generates that nonce to deploy the safe first.

This attack works because the _salt does not sufficiently participate in generating the final safe address - it only affects the nonce.

This demonstrates how an attacker could pre-compute addresses by brute forcing salts offline.

1. Victim has safe owners [A, B, C] which they will use to deploy a safe.

2. Attacker learns these owners will be used.

3. Attacker writes a script that:

• Iterates through 1 trillion random 32 byte salt values S1, S2..., S1trillion

• For each salt Si:

  • Calls _genNonce(keccak256(encode([A, B, C])), Si)

  • Stores the resulting nonce Ni

  • Calculates the safe address Ai that would result from createProxyWithNonce(nonce = Ni)

4. This creates a mapping of (Si -> Ai) for 1 trillion salts.

5. When the victim deploys their real safe with salt S1million, the attacker looks up the pre-computed address A1million and deploys it first.

This concrete example demonstrates how an attacker could realistically pre-compute 1 trillion+ addresses offline by brute forcing salts. They could then front-run the victim's real deployment by looking up the address for their salt.

Tools Used

Manual

Recommended Mitigation Steps

_salt should be included in the address generation. For example, by passing it to create2:

function _createSafe({/*...*/}) private returns (address _safe) {

  bytes32 salt = keccak256(abi.encodePacked(_salt, _owners));

  _safe = address(uint160(uint(keccak256(abi.encodePacked(
    bytes1(0xff),
    address(this),
    salt,
    keccak256(abi.encodePacked(
      type(Proxy).creationCode,
      uint256(_owners.length),
      _owners
    ))
  )))));

}

This incorporates the caller's salt into the final safe address in a way that can't be brute forced.

Assessed type

Invalid Validation

[c4-pre-sort]

raymondfam marked the issue as low quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #17

[c4-judge]

alex-ppg marked the issue as not a duplicate

[alex-ppg]

As the salt_ is part of the hash of the actual nonce utilized for the Gnosis Safe deployment, it is indeed incorporated into the Gnosis Safe's address generation mechanism. As such, this exhibit is invalid.

[c4-judge]

alex-ppg marked the issue as unsatisfactory: Invalid

[emerald7017]

Thank you @alex-ppg for the clarity, I clearly understand now.