Wormhole Consistency Levels set to zero in the publishMessage

[hats-bug-reporter[bot]]

Github username: @ShaheenRehman Twitter username: 0x_Shaheen Submission hash (on-chain): 0xd91e12515e3a0202c52712e61b79c299b3a51f4338700a636bfa2bb65dc25a73 Severity: low

Description: Description\ The GeneralisedIncentives integrates Wormhole to publish & verify messages.

wormhole.publishMessage's important parameter, consistency_Level or finality is set to zero.

It should be never zero, it is different on every chain, it should be chain specific.

The consistency level controls how many blocks since the transaction originally occurred the guardian network should wait before the message begins the validation process. This can be helpful for security reasons, it proviedes defense against reorgs and rollbacks."

        WORMHOLE.publishMessage{value: costOfsendPacketInNativeToken}(
            0,
            abi.encodePacked(
                destinationChainIdentifier,
                message
            ),
            0   // Finality = complete.
        );

Code Snippets\ https://github.com/catalystdao/GeneralisedIncentives/blob/2448d77e412216283ed75d8c3cbaa1270657f7b5/src/apps/wormhole/IncentivizedWormholeEscrow.sol#L73

Mitigation\ Add consistency_level/finality base on every chain's finality: https://github.com/wormhole-foundation/docs.wormhole.com/blob/835d1e813d62e7dfd1f15928e0894c1443faf5bd/docs/reference/constants.md#consistency-levels

[reednaa]

From your docs: If a value is passed that is not in the set above it's assumed to mean finalized

[ShaheenRehman]

yes sir, correct, but that's not safe. Chain reorgs happen on good chains like Polygon: https://protos.com/polygon-hit-by-157-block-reorg-despite-hard-fork-to-reduce-reorgs/

In the Wormhole's example contracts, they make sure that wormhole Finality is set above zero: https://github.com/wormhole-foundation/wormhole-scaffolding/blob/87f3dce3f610aed5912248ee33b0a3e684285f50/evm/src/01_hello_world/HelloWorld.sol#L27

[reednaa]

How would you protect against deep reorgs? The line you highlight is 2 years old. They likely changed the way it is used since then and for some of the chains they seem to recommend 0.

[ShaheenRehman]

i guess reorgs happens more than the deep reorgs. so protection for reorgs is more important. Just because we cannot protect against one thing, doesn't mean we should overlook the other issue and not use protection for it.

And tbh I'm not really sure about the wormholeFinality is important anymore or not. What I can do, is contact a wormhole developer and ask him about this. I'll share the details with you soon. Thanks!

[reednaa]

I am willing to grant this a low severity if a credible source argues that setting it to 0 is not good enough.

[ShaheenRehman]

Hi Sir @reednaa, as promised I contacted a wormhole dev and asked him about the wormholeFinality is important anymore or not, and this is what he said:

its not great. I think all the token bridges have it set to 1 or 15

Full chat: https://discord.com/channels/856079283104251944/857225885231611935/1199757143293829262 (To access the chat, you have to join wormhole discord and have a developer role)

I think it's quite clear that the wormholeFinality should be chain-specific, it's still recommended by wormhole, and hardcoding it to zero is bad. Hope this helps. Thanks!

[reednaa]

I will discuss this internally.

I am leaning won't fix since it functions as intended on EVM (which this audit targets), the proposed fix seems to be either setting it to 1 or 15 (which wouldn't actually do anything to how the protocol functions).

The reason why I dislike having a chain-specific wormholeFinality is that it introduces way more human errors than it solves. If they decide to make an EVM chain that breaks this scheme I would rather make a deployment specifically for that chain rather than make the finality custom.

[ShaheenRehman]

The reason why I dislike having a chain-specific wormholeFinality is that it introduces way more human errors than it solves.

Sir I don't think so this is true, is there any incident of that? I would love to learn about this more.

I just checked codeslaw and noticed that almost every contract deployed (using wormhole.publishMessage) is using some sort of finality: https://www.codeslaw.app/search?chain=ethereum&q=publishMessage

So, I don't think so it will become problematic for the protocol.

If you dislike chain-specific finality number, then I guess 1-15 is a safer option, as the wormhole developer said.

[reednaa]

Sir I don't think so this is true, is there any incident of that? I would love to learn about this more.

Deployment mistakes happen. That is why so many people are submitting issues with no check for non-address(0). While I don't agree that these are severe setting a wrong finality could be worse since it is more likely to go undetected. (since it doesn't have any short term side effects).

If I wanted to set it to 201 but instead set it to 200, for a lot of chains it could be bad. A very similar result could happen for if it shouldn't have been set to 0.

---

We are discussing internally if we want to change it to 1 or 15 and how that qualifies for a bounty. Note, the only time this change (currently) matters is for a contract written in Rust for Solana. For any of the EVM chains 0 implies final. (as explained by Barnji in your chat)

[reednaa]

We have decided to classify this issue as Low. Our decision is based on the following arguments:

• The issue does not present itself in any way, as both documented by Wormhole’s docs and by a Wormhole developer. Thus it has no material impact on the contracts.
• It is not universal that 0 implies full finality and non-EVM implementation could mistakenly copy 0 over. For Solana it would cause problems.
• Wormhole recommends 1 or 15.

According to these arguments, the issue is low. Internally we have determined that the proposed fix will introduce more risk than it removes. Instead we will set it to 1 or 15.

[ShaheenRehman]

The Fix looks good. Thanks! Shaheen