Omniscia Tangible Audit

LayerZeroRebaseTokenUpgradeable Manual Review Findings

LayerZeroRebaseTokenUpgradeable Manual Review Findings

LZR-01M: Incorrect Consumption of Cross-Chain Transfers (Satellite)

Description:

The LayerZeroRebaseTokenUpgradeable::_sendAck function will consume a cross-chain transfer in a satellite chain by updating the current rebase index.

A misbehaviour arises from this system as it is linear; should a rebase operation be consumed while cross-chain transfers are pending, these pending operations will never succeed. This issue is bound to be identified in a production environment due to the usage of the NonblockingLzAppUpgradeable system and specifically due to the NonblockingLzAppUpgradeable::retryMessage function.

Impact:

The current LayerZero cross-chain relay system implemented in LayerZeroRebaseTokenUpgradeable::_sendAck for satellite chains is prone to perpetual retry-failure, causing the non-blocking LayerZero application implementation to store messages that will never succeed thus leading to fund loss.

Example:

src/LayerZeroRebaseTokenUpgradeable.sol
175function _sendAck(uint16 srcChainId, bytes memory, uint64, bytes memory payload) internal override {
176 (, bytes memory toAddressBytes, Message memory message) = abi.decode(payload, (uint16, bytes, Message));
177
178 if (isMainChain()) {
179 assert(message.nonce <= _rebaseNonce());
180 } else {
181 _setRebaseIndex(message.rebaseIndex, message.nonce);
182 }
183
184 address to = toAddressBytes.toAddress(0);
185 uint256 amount = _creditTo(srcChainId, to, message.shares);
186
187 emit ReceiveFromChain(srcChainId, to, amount);
188}

Recommendation:

Given that by its nature, the CrossChainRebaseTokenUpgradeable is a blocking system that should consume all transactions in the order they are submitted, we strongly advise the Tangible team to consider implementing the normal version of the LzAppUpgradeable version.

Alternatively, we advise retries of past nonce values to be correctly re-tried as otherwise they will never be relayed correctly. For example, they could simply be relayed back to the main chain where they are bound to succeed.

Alleviation (47fbf62bbbf2409ff0baf9a18a2945466cb2a576):

The Tangible team analysed this exhibit in great length within its dedicated issue and carried out multiple albeit small changes in the codebase to fully alleviate it.

In detail, the code was updated in the following ways\:

  • The CrossChainRebaseTokenUpgradeable::_setRebaseIndex function no longer mandates a particular index value and instead updates the index if the nonce supplied is greater-than-or-equal to the current nonce
  • The LayerZeroRebaseTokenUpgradeable::_sendAck function reduced its validation and no longer mandates a message from a previous nonce for the main-chain, permitting temporary desynchronizations between chains
  • The rebase index logic has been split to two dedicated functions; one for the main chain and one for satellite chains, simplifying the overall rebase process

As a result of these changes, we consider the original attack vector fully rectified as the consumption of transactions cannot be halted in the current implementation and will always succeed.