Arbitrary external addresses in payload could allow for griefing of unexpecting relayers

[hats-bug-reporter]

Github username: @PlamenTSV
Twitter username: @p_tsanev
Submission hash (on-chain): 0x01a0c73b68d2d0ed943a72e32453131af922d7b7848765294737f542700a8ad9
Severity: low

Description:
Description
The CCI function sendCrossChainLiquidity calls the IME function submitMessage, allowing the caller to specifying any arbitrary parameters, in the case of IME even allowing for an incentive/bounty that's outside of the allowed limit. This could allow the creation of a bounty that when processed could intentionally call external addresses that grief the relayer through gas.

Attack Scenario
We start from sendCrossChainLiquidity, then our fromVault address would be that of msg.sender, toApplication will be the CCI. Additionally, we would be able to specify the toVault and toApplication to any address we want, which can too be contracts. Our message identifier will surely be valid (we can append dirty calldata just to be sure), so we will get this message signed successfully and put an incentivizing bounty on it.
When a relayer sees this(or the structure detects so) bounty, he will naturally try to process it, providing the gas it needs. Thus we reach 2 instances of gas consumption:

1. _handleMessage -> ICrossChainReceiver(toApplication).receiveMessage{gas: maxGas}. The problem here is that we cannot burn more than maxGas, but still if we create a gas eating function at the toApplication we can easily make a let's say, unbound loop and hit OOG, making the relayer waste his funds.
2. We can choose to just instantly return an ack here, thus leading the relayer into processing this ack and getting to the CCI's receiveAck function, where we have either ICatalystV1Vault(fromVault).onSendLiquiditySuccess or ICatalystV1Vault(fromVault).onSendLiquidityFailure. But since fromVault is our address, we can choose to force the OOG here. Right here there is no gas limit on the external call, so we can easily eat up the entire block gaslimit and grief the relayer even greater.

I do not see (and am led to believe) there is no way to detect bounties which revert, so relayers have no way of being protected againts gas-draining packets. They can only detect if their incentive is good enough for process. There is also no way to clear such failing bounties, so there would always be relayers getting griefed from attempting to process them.

Attachments

1. Proof of Concept (PoC) File

2. Revised Code File (Optional)

As a recommendation, I believe in the CCI sendCrossChainLiquidity and sendCrossChainAsset should be protected by access only from vaults. An admin function which can delete bounties that are failing can deem useful. A sanity check to see if the provided from and to addresses in the messages point to real instances of vaults and CCIs.

[reednaa]

Yes, you can use all of the gas but the relayer will be paid for it. There is no way for you to use gas which the relayer isn't being paid for. (except catalystdao/GeneralisedIncentives#8)

It is true that if an application uses ALL of the allocated gas, then the relayer has to evaluate the bounty carefully.

• If an application uses less than all of the gas, some of the gas associated with verification is also paid for by the application. (we are recording gas before verification)
• If an application uses all of the gas, none of the gas associated with verification is paid for by the application.

This is also the case on the ack side.

A relayer can lazily evaluate packages by using the fact that verification cost is roughly constant. Thus the gas used by the application is verficationCost + maxGasDelivery + fixedTXcost and the bounty is maxGasDelivery * gasPriceDelivery.

A better designed relayer can simulate the packages and figure out how much gas the application spends. This can be done by forking the network and calling the application by faking the Incentive contract. This also allows you to get the ack and you can do a similar simulation on the source chain.

Let me know if I misunderstood something.

[PlamenTSV]

So in order to accidentally avoid wasting gas on a malicious packet, relayers should verify each packet on a forked enviroment?
Even if that was the case, I believe this behavior should have been given to us as to avoid such speculations.
Being able to set the external addresses screams vulnerability, this is why I reported this in the first place.

[reednaa]

The latter is an example of how to optimise for what you described not a "solution". The former is the solution.

You havn't convinced me that the current implementation and documentation is insufficient given common sense. (see everything but last portion of my previous comment)

[reednaa]

Specifically:
If a relayer assumes they will get paid for all gas an application spends but nothing else then this wouldn't be an issue.
We actually provide a slighly stronger incentive:
A relayer is paid for all gas an application spends AND if there is unspent gas they are also paid for verification.