Omniscia Tangible Audit
LayerZeroRebaseTokenUpgradeable Manual Review Findings
LayerZeroRebaseTokenUpgradeable Manual Review Findings
LZR-01M: Incorrect Consumption of Cross-Chain Transfers (Satellite)
Type | Severity | Location |
---|---|---|
Logical Fault | LayerZeroRebaseTokenUpgradeable.sol:L181 |
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:
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 particularindex
value and instead updates the index if thenonce
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 previousnonce
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.