Frontrunning createPair() may lead to DOS

[hats-bug-reporter[bot]]

Github username: @djxploit Twitter username: -- Submission hash (on-chain): 0x911f0bcf688ccf0985b106eac946b5e347061b10d0c3ce524e7b53f908e998b8 Severity: high

Description: Description\ A user may frontrun createPair function in PairFactoryUpgradeable.sol , to predict the salt and DOS users from creating pairs.

As can be seen in https://github.com/Satsyxbt/Fenix/blob/main/contracts/dexV2/PairFactoryUpgradeable.sol#L143, a pair is created by using the cloneDeterministic function, by using the salt as keccak256(abi.encodePacked(token0, token1, stable)) , which are given as arguments to the createPair functions.

Thus if an attacker frontruns the createPair function, he would be able to determine the salt, and create the pair. This will revert the legitimate createPair call

Attack Scenario\

1. Attacker monitors the mempool
2. Attacker sees the legitimate createPair call
3. He extracts the arguments from the call, and calls the cloneDeterministic(implementation, keccak256(abi.encodePacked(token0, token1, stable))) himself with the extracted arguments.
4. Now the legitimate address will revert as there is already a deployed contract at the predetermined address

Attachments

1. Proof of Concept (PoC) File

   function createPair(address tokenA, address tokenB, bool stable) external virtual override returns (address pair) {
       if (!isPublicPoolCreationMode) {
           _checkRole(PAIRS_CREATOR_ROLE);
       }
   
       if (tokenA == tokenB) {
           revert IdenticalAddress();
       }
   
       (address token0, address token1) = tokenA < tokenB ? (tokenA, tokenB) : (tokenB, tokenA);
       if (token0 == address(0)) {
           revert AddressZero();
       }
   
       if (getPair[token0][token1][stable] != address(0)) {
           revert PairExist();
       }
   
       pair = Clones.cloneDeterministic(implementation, keccak256(abi.encodePacked(token0, token1, stable)));
   
       address feesVaultForPool = IFeesVaultFactory(communityVaultFactory).createVaultForPool(pair);
   
       IPair(pair).initialize(defaultBlastGovernor, token0, token1, stable, feesVaultForPool);
   
       if (isRebaseToken[token0]) {
           IBlastERC20RebasingManage(pair).configure(token0, configurationForBlastRebaseTokens[token0]);
       }
   
       if (isRebaseToken[token1]) {
           IBlastERC20RebasingManage(pair).configure(token1, configurationForBlastRebaseTokens[token1]);
       }
   
       getPair[token0][token1][stable] = pair;
       getPair[token1][token0][stable] = pair; // populate mapping in the reverse direction
       allPairs.push(pair);
       isPair[pair] = true;
   
       emit PairCreated(token0, token1, stable, pair, allPairs.length);
   }

2. Revised Code File (Optional)

[BohdanHrytsak]

Thank you for the submission.

This behaviour is the expected and standard way to create predictable addresses for pairs/pools, in case of using a more complex salt it will not provide a cheap and easy way to get the address of a pair.

I also want to note that there is no benefit for an attacker to cause a pair to be created, as this will lead to the expected result anyway. And also, the user who actually wanted to create the pair will pay much less gas for the transaction if it fails, even though the pair will be created by the attacker