`cloneDeterministic` can cause DOS

[hats-bug-reporter[bot]]

Github username: -- Submission hash (on-chain): 0x8c87f3c9cea38884a3bba80eccb64354584f319812e828f957dacf133fc18cc3 Severity: medium

Description: Description\ DOS of the function createHATPaymentSplitter

Attack Scenario\ In the function createHATPaymentSplitter we are using cloneDeterministic and using _payees and _shares as salt which were passed by the user.

        result = Clones.cloneDeterministic(implementation, keccak256(abi.encodePacked(_payees, _shares)));

so lets say that Alice tries to call this function and use A and B as function perameters so Alice send this transaction and the transaction is in mempool, so while the transaction is in mempool and as the salts are passed by the user so bob sees this transactions and front-run alice transaction and creates a new contract at that address, so in this way he will forceFully revert Alice transaction as bob contract already exist on the predetermined address.

This attack can be repeated again and again resulting in DoS for the function.

Attachments

2. Revised Code File (Optional) consider using msg.sender as a salt perameter

       result = Clones.cloneDeterministic(implementation, keccak256(abi.encodePacked(_payees, _shares, msg.sender)));

[jellegerbrandy]

This is a very weird kind of grieving attack with no gain at all to the attacker. In addition, your fix will not help against this attack. Lastly, we are not necessarily unhappy if people pay gas costs to deploy our contracts (although it could be annoying or confusing if they also call initialize)

[Nabeel-javaid]

This is a very weird kind of grieving attack with no gain at all to the attacker. In addition, your fix will not help against this attack. Lastly, we are not necessarily unhappy if people pay gas costs to deploy our contracts (although it could be annoying or confusing if they also call initialize)

Respectfully, there is no gain to the attacker doesn't means that he will not do it and maybe my recommendation is wrong but the issue is 100% right

[jellegerbrandy]

Sorry, I have perhaps not been completely clear here. What I should have said is that the there is no "gain at all to the attacker" because there is no damage at al to the protocol.

createHATPaymentSplitter is a completely deterministic function and it's results do not depend on who sends the transaction. If Bob sees a createHatPaymentSplitter call in the in the mempool and decides to execute it, that is fine - it will be the sam econtract in the same state that Ann wanted to deploy. The only difference is that Bob pays the gas instead of Ann. There is no damage to Ann as far as I can see.

[Nabeel-javaid]

yeah you are completely right, Bob will pay the gas fee instead of Ann, but in this way Ann transaction will revert so Ann will try again to call createHATPaymentSplitter but what if again Bob front-run Ann transaction? Ann transaction will revert again, and this could be the start of DOS of createHATPaymentSplitter for Ann or for anyone else cause everytime Ann or someone else tries to call createHATPaymentSplitter bob can front-run the transaction and the transaction will revert for the original user.