cergyk - BlockSpecimenProofChain::submitBlockSpecimen block number may be incorrectly estimated for some chains

[sherlock-admin2]

cergyk

medium

BlockSpecimenProofChain::submitBlockSpecimen block number may be incorrectly estimated for some chains

Summary

The strategy of estimating a block number on target chain used to determine thresholds, would be inadapted in some cases such as variable block time chains, or in even in the case of Moonbeam, in the case of a chain halt.

Vulnerability Detail

On the 3 Apr 2023 Moonbeam experienced an outage due to an upgrade: https://moonbeam.network/blog/block-production-interruption-root-cause/

This caused the block 3291300 to be produced approx 4 hours, 12 minutes, and 24 after 3291299 was produced instead of target 12 seconds.

This would break the constant block time estimation used by Covalent here: https://github.com/sherlock-audit/2023-11-covalent/blob/main/cqt-staking/contracts/BlockSpecimenProofChain.sol#L342-L344

And potentially prevent the correct submission of some block specimens.

Impact

Some block specimens may not be produced

Code Snippet

Tool used

Manual Review

Recommendation

Fix does not seem trivial, please consider having a more flexible approach to block number estimation on target chain

[sherlock-admin2]

1 comment(s) were left on this issue during the judging contest.

takarez commented:

invalid

[nevillehuang]

Invalid, this would consitute external admin error synonymous to a sequncer outage, and so invalid

[CergyK]

Escalate

I think this is a bit different from the textbook sherlock rule, since the protocol core functionality is to attest block on some chains from other chains; To do this, it attempts to estimate block numbers from other chains and if it does so wrongly these blocks may be DOSed.

The case of Moonbeam is especially relevant, since covalent focuses on ETH mainnet and Moonbeam for its deployment

[sherlock-admin]

Escalate

I think this is a bit different from the textbook sherlock rule, since the protocol core functionality is to attest block on some chains from other chains; To do this, it attempts to estimate block numbers from other chains and if it does so wrongly these blocks may be DOSed.

The case of Moonbeam is especially relevant, since covalent focuses on ETH mainnet and Moonbeam for its deployment

You've created a valid escalation!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

[midori-fuse]

I believe this is a low.

Firstly, if a chain outage does happen, admin can easily make submissions more flexible through setBlockHeightSubmissionsThreshold() and other admin-only functions.

Secondly, quoting Sherlock rules:

Chain re-org and network liveness related issues are not considered valid. Exception: If an issue concerns any kind of a network admin (e.g. a sequencer), can be remedied by a smart contract modification, the protocol team considers external admins restricted and the considered network was explicitly mentioned in the contest README, it may be a valid medium. It should be assumed that any such network issues will be resolved within 7 days, if that may be possible.

Issue is thus not a valid medium for the following reasons:

• The external admin is considered trusted. The root cause of the Moonbeam incident was invalid admin input, which does not fall into the scope of this contest.
• Even if we make a case for this and consider the admin restricted, the submission failed to prove this can be remedied by a smart contract modification.
• This comment has proven that admin can adjust the chain parameters in such a chain halt event. A trusted admin, in the event of the network upgrade, must make a timely announcement to all its users. The quoted article showed that Moonbeam admin made an announcement. The halt only became an incident due to invalid admin input.

[nevillehuang]

Agree with @midori-fuse, should remain low severity.

[Czar102]

Firstly, this is not a network liveness issue – the "outage" may theoretically be 1 minute and the impact would still be there, and that wouldn't cause any transactions not to be able to execute on that network in a timely manner.

Secondly, I believe it's possible to change this setting or effectively disable it for variable block time networks, so the code is compatible with these networks, too.

In case of Moonbeam, I think it's possible to remediate this sanity check not passing by modifying the parameters using setSecondsPerBlock and setBlockHeightSubmissionsThreshold functions. @CergyK would you agree?

Planning to consider this issue an informational one and reject the escalation.

[Czar102]

Result: Low Unique

[sherlock-admin2]

Escalations have been resolved successfully!

Escalation status:

• CergyK: rejected