hats-finance / Catalyst-Exchange-0x3026c1ea29bf1280f99b41934b2cb65d053c9db4

Other
1 stars 3 forks source link

Permanent DoS attack vector on submit-message #71

Open hats-bug-reporter[bot] opened 5 months ago

hats-bug-reporter[bot] commented 5 months ago

Github username: -- Twitter username: https://twitter.com/windhustler Submission hash (on-chain): 0x445fd07583a9783f2adf5521af7128b21649b3a60fdd333ef59c3b23fdfc2123 Severity: high

Description: Description\ Using block.number to construct a unique message identifier opens a permanent DoS attack vector.

Attack Scenario\ IncentivizedWormholeEscrow contract overrides the _getMessageIdentifier function and uses:

to construct a unique message identifier. As block.number is the same for every transaction within a block this opens a DoS attack vector for all the users using this contract.

Let's imagine a user wants to send a cross-chain transaction from Ethereum -> Polygon. A front-running bot sees the user's transaction in the mempool and sends the exact same transaction, i.e.

to the same destinationIdentifier and with the same message. As bounty can be placed only for a unique message identifier the griever's transaction gets included in the block.

While the user's transactions reverts inside the _setBounty function: https://github.com/catalystdao/GeneralisedIncentives/blob/2448d77e412216283ed75d8c3cbaa1270657f7b5/src/IncentivizedMessageEscrow.sol#L535.

Each and every transaction can be reverted by a front-running bot. This is a permanent DoS attack vector.

As this is a messaging component, and this attack can make the user's not being able to send any messages I'm marking this as a high issue. Also, the griever can set the incentives to zero so the only cost for him is the transaction cost.

Recommended Mitigation Steps\ Use a monotonically increasing nonce to construct a unique message identifier inside the IncentivizedWormholeEscrow contract instead of block.number.

PlamenTSV commented 5 months ago

Dupe of #46

windhustler commented 5 months ago

https://github.com/hats-finance/Catalyst-Exchange-0x3026c1ea29bf1280f99b41934b2cb65d053c9db4/issues/46 doesn't mention IncentivizedWormholeEscrow contract and it's a specific implementation of _getMessageIdentifier which is the root cause of this issue.

windhustler commented 5 months ago

46 does mention the IncentivizedMessageEscrow contract. Your original submission only reports a single instance of the issue, so I'll leave it to the judges to decide.

reednaa commented 5 months ago

It is a duplicate of #46. Specially, my proposed fix to the issue cover this as well: msg.sender should be part of the identifier: https://github.com/catalystdao/GeneralisedIncentives/blob/2448d77e412216283ed75d8c3cbaa1270657f7b5/src/IncentivizedMessageEscrow.sol#L75-L80

@PlamenTSV 's issue is not written that well (because he kind of tries to argue for another attack). Sadly, we only grant issues for the first non-duplicate regardless of the description. (See my message in Discord for the limited exceptions).

If you disagree that it is a duplicate, please raise the issue to Hats.