The issue of not limiting the number of iterations in the code can lead to several security and performance implications:
Overloading the System: An attacker could potentially create a large number of Gnosis Safe instances in a single transaction, leading to system overload and gas exhaustion.
Denial of Service (DoS) Attacks: Creating an excessive number of Safe instances could result in a Denial of Service (DoS) attack, rendering the system unavailable to other users.
// Generate nonce based on owners and user provided salt
uint256 nonce = _genNonce(ownersHash, _salt);
do {
try IGnosisProxyFactory(gnosisProxyFactory).createProxyWithNonce(gnosisSafeSingleton, _initializer, nonce)
returns (address _deployedSafe) {
_safe = _deployedSafe;
} catch Error(string memory reason) {
// KEK
if (keccak256(bytes(reason)) != _SAFE_CREATION_FAILURE_REASON) {
// A safe is already deployed with the same salt, retry with bumped nonce
revert SafeProxyCreationFailed();
}
emit SafeProxyCreationFailure(gnosisSafeSingleton, nonce, _initializer);
nonce = _genNonce(ownersHash, _salt);
} catch {
revert SafeProxyCreationFailed();
}
} while (_safe == address(0));
Tools Used
Manual Review
Recommended
Limit the Number of Iterations: Add a check to limit the number of iterations in the code. This will prevent attackers from creating an excessive number of Safe instances in a single transaction.
Ensure Transaction Integrity: Define a maximum limit for the number of iterations and check whether this limit has been exceeded before proceeding with the creation of Safe instances.
Example:
function _createSafe(address[] calldata _owners, bytes memory _initializer, bytes32 _salt)
private
returns (address _safe)
{
address gnosisProxyFactory = AddressProviderService._getAuthorizedAddress(_GNOSIS_PROXY_FACTORY_HASH);
address gnosisSafeSingleton = AddressProviderService._getAuthorizedAddress(_GNOSIS_SINGLETON_HASH);
bytes32 ownersHash = keccak256(abi.encode(_owners));
// Generate nonce based on owners and user provided salt
uint256 nonce = _genNonce(ownersHash, _salt);
// Limit the number of retries to prevent excessive gas usage
+ uint256 maxRetries = 3; // Set your desired maximum number of retries
+ uint256 retries = 0;
+ while (retries < maxRetries) {
- do {
try IGnosisProxyFactory(gnosisProxyFactory).createProxyWithNonce(gnosisSafeSingleton, _initializer, nonce)
returns (address _deployedSafe)
{
_safe = _deployedSafe;
break; // Successfully created a safe, exit the loop
} catch Error(string memory reason) {
// Handle the error with a specific reason
if (keccak256(bytes(reason)) != _SAFE_CREATION_FAILURE_REASON) {
revert("SafeProxyCreationFailed");
}
emit SafeProxyCreationFailure(gnosisSafeSingleton, nonce, _initializer);
// Retry with a bumped nonce
nonce = _genNonce(ownersHash, _salt);
retries++;
} catch {
revert("SafeProxyCreationFailed");
}
}
+ if (_safe == address(0)) {
+ revert("Safe creation failed after maximum retries");
+ }
- } while (_safe == address(0));
}
Lines of code
https://github.com/code-423n4/2023-10-brahma/blob/main/contracts/src/core/SafeDeployer.sol#L229-L224
Vulnerability details
Impact
The issue of not limiting the number of iterations in the code can lead to several security and performance implications:
Proof of Concept
https://github.com/code-423n4/2023-10-brahma/blob/main/contracts/src/core/SafeDeployer.sol#L229-L224
Tools Used
Manual Review
Recommended
Example:
Assessed type
DoS