Open dkuppitz opened 1 year ago
Upon careful consideration of the audit findings related to the LayerZeroRebaseTokenUpgradeable::_sendAck
function, I recognize the concerns regarding the potential for non-consecutive rebase operation consumption and the subsequent issues that may arise with pending cross-chain transfers. I propose the following measures to address this issue:
Refinement of Rebase Assertion Logic on the Main Chain: We will eliminate the assertion on the main chain for message nonce validation within the _sendAck
function. By doing so, we'll prevent any interruption in the operation flow due to rebase operations occurring out of sequence, which may be the case with the current linear consumption pattern.
Adjustment of Rebase Index Handling on Satellite Chains: The function _setRebaseIndex
can be adjusted to allow for a non-reverting execution path on satellite chains. Such a provision will enable the system to accommodate temporary desynchronization between the rebase indexes of the main and satellite chains without causing transaction failures.
These modifications ensure that the system’s resilience is maintained in the face of potential discrepancies. Furthermore, it is anticipated that any misalignment in the rebase index would be transient, given the implementation of off-chain monitors designed to correct any divergence between the main chain and its satellites. Moreover, as the redemption of underlying assets is not permitted on satellite chains and the transfer mechanism operates based on shares rather than absolute amounts, the integrity of value transfer and redemption processes remains intact. The recipient will always obtain the correct amount upon redemption on the main chain, preserving the system's functional integrity and user trust.
I believe these improvements will mitigate the risk outlined in the audit, without compromising on the protocol’s efficiency or security.
Following a thorough analysis of the issue identified as LZR-01M, we have implemented several key updates to address the potential for perpetual retry-failure and ensure robust cross-chain communication and state consistency.
Summary of Implemented Changes:
Removal of Nonce Assertion on Main Chain:
_sendAck
function on the main chain that verified the message nonce against the _rebaseNonce()
.Enhanced Rebase Index Handling on Satellite Chains:
_setRebaseIndex
function to handle the rebase index updates in a way that avoids transaction failures due to temporary desynchronization between main and satellite chains.Optimized Rebase Nonce Verification Logic:
Contract Initialization Refinement:
_isMainChain
flag based on the provided mainChainId
at deployment time, ensuring that this characteristic does not change post-deployment.refreshRebaseIndex
only if the chain is the main chain, and setRebaseIndex
is called with a predefined initial value otherwise.Gas Optimization:
Added Documentation:
Additional Safeguards and Mechanisms:
Off-Chain Monitoring Integration:
Guaranteed Redemption Integrity:
Rationale for Changes:
These enhancements are intended to provide a more resilient and fault-tolerant mechanism for cross-chain rebase synchronization. By allowing the satellite chains to catch up with the main chain's state without reverting transactions, the risk of pending operations failing indefinitely is mitigated. The update ensures that all cross-chain transfers are processed orderly and efficiently while preserving the non-blocking nature of the system.
Furthermore, the new measures put in place for monitoring and correcting rebase index synchronization bolster the system's reliability and ensure equitable user experiences across all chains.
Resolved in ee32873673f6c0dd601315a48cce572b46e624bf.
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 theNonblockingLzAppUpgradeable::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:
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 theLzAppUpgradeable
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.