Solana Staking

When claiming rewards from Solana, the send_claim program is expected to be used where the user first submits a proof, then the SendClaim function encodes the user’s rewards and merkle root into the payload.

However, an attacker may bypass send_claim and use the send_request program instead to claim rewards. SendRequest does not validate that the payload type is not ClaimRewardSolana. The attacker is therefore able to set their desired claim amount and easily obtain a merkle root to match the current active root of the desired distribution.

On the Ledger chain, MerkleDistributor handles reward claims from Solana differently, without requiring a merkle proof, as it expects the check to be performed on Solana. However, by bypassing send_claim the attacker is able to claim any arbitrary amount from the distribution, draining the LedgerOCCManager of all ORDER tokens.

In send_request: 58 validate that the payload type is not ClaimRewardSolana .

Orderly Team: The issue was resolved in commit cd0eaa8.

Block Confirmation Mismatch

In layerzero.config.ts, messages sent from Solana to Orderly require 10 block confirmations. However, on the Orderly chain, the receive configuration requires 32 block confirmations (as seen in the ReceiveUln302 contract: Explorer Link, using the getUlnConfig function with OApp address 0x68835941c7C300bFEF44D1D68b83798791901eB8 and remoteId 30168).

This mismatch causes the sending OApp (Solana) to proceed with only 10 confirmations, while the receiving OApp (Orderly) rejects any message with fewer than 32 confirmations. As a result, messages will be blocked until either:

1. The sending OApp increases the outbound block confirmation requirement to 32, or
2. The receiving OApp decreases the inbound confirmation threshold to 10.

NOTE: Increasing the outbound block confirmation will also mitigate against any re-org risk.

DVN Mismatch

Similarly, layerzero.config.ts sets the required DVN to a LayerZero Labs address (4VDjp6XQaxoZf5RGwiPU9NR1EXSZn2TP4ATMmiSzLfhb). However, on the Orderly chain, the required DVN points to a dead DVN (0x690b1857EaA8c55850547d7C22148C0B99a71dCd). This discrepancy means that:

1. The sending OApp (Solana) pays the LayerZero Labs DVN to verify packets,
2. But the receiving OApp (Orderly) relies on a dead DVN, which performs no verification.

As a result, messages will be blocked until the receiving OApp updates its configuration to remove or replace the dead DVN.

1. Ensure that block confirmation settings match on both the sending and receiving OApps.
2. Update the DVN configuration to ensure that both the sending and receiving OApps reference the same,

active DVN.

Orderly Team: Acknowledged. 11

When users claim rewards both from EVM chains and Solana, they pay backward fee to cover the message fees from Orderly network back to theirs. However, some distributionIds are rewarding only esORDER.

When these tokens are claimed, they are being staked in the system and no backwards message is sent. In result, users who claim their esORDER rewards pay an unfair fee.

You can track which distributionId is rewarding esORDER on the vault sides and don't charge fee if the user specified distributionId matches it.

Orderly Team: Acknowledged.

With the ILedgerOapp(ledgerOappAddr).ledgerOappSend(occMessage); call from the OCCManager, ETH value is not sent to the LedgerOApp.

Because native value is not forwarded, and Orderly does not subsidize cross-chain messages, _lzSend will revert due to lack of messaging fee provided (error NotEnoughNative) and all messages back to Solana through the OApp will be permanently DoS'd.

Forward native value to the LedgerOApp so that the ledgerOappSend function call can succeed. This is achieved by:

1. Making the function ledgerOappSend payable
2. In LedgerOCCManager, calculate and forward the native fee.

Orderly Team: The issue was resolved in commit 05fe8af.

Proof of concept: PoC

lz_receive is called by the executor after a user sends a request with the ClaimUsdcRevenue payload type to the Ledger.

In the LzReceive struct, it is expected that the receiver_token_account is already initialized by the user. However, there is a possibility that this account is not initialized. If that happens, the executor's call to lz_receive will always revert with error “AccountNotInitialized”, causing a DOS and loss of fees.

Initialize the account if it is not already initialized with init_if_needed.

Orderly Team: The issue was resolved in commit 28077ba.

When sending a message from Solana to Orderly, the anticipated gas cost for executing lzReceive must be specified. Currently, this is determined by get_enforced_options, which provides a single gas value based on SEND or SEND_AND_CALL.

However, in LedgerOApp, lzReceive processes various payload types, each requiring different gas amounts. Using a single gas value results in users either: a) overpaying for simple operations, or b) underpaying, causing transactions to fail due to insufficient gas.

Consider implementing a mapping of payload type to destination gas in the Solana proxy, similar to the existing payloadType2DstGas mapping in OCCManager.sol.

Orderly Team: Acknowledged.

According to the LZ docs, when sending messages to Solana, the specified addExecutorLzReceiveOption parameters are compute_units and lamports.

Currently, LedgerOCCManager.buildOCCLedgerMsg() will use the same addExecutorLzReceiveOption(_oftGas, 0). This may result in unexpected behavior because the gas and compute_units are not the same.

In addition, you should send at least 1500000 lamports as mentioned in the docs.

Consider passing different values when sending to Solana.

Orderly Team: The issue was resolved in commit 0401f8e.

InTransferAdmin:apply the event emits the newly set admin as the old_admin, because it uses ctx.accounts.proxy_config.admin after is has already been updated. Consequently, the AdminRoleTransferred emits inaccurate information.

Cache the old admin and use that cached value in the event instead.

Orderly Team: The issue was resolved in commit 5915655.

When users stake their ORDER from Solana, they can pass arbitrary data in the compose message. Each of these fields are validated except chainedEventId and payload. The chainedEventId parameter is not used if the messages comes from Solana, so that's okay.

The payload however is further passed to OmnichainLedgerV1.ledgerRecvFromVault(). Even though it's not used there, for additional safety it's better to not use it.

Use empty bytes for the payload.

Orderly Team: The issue was resolved in commit 4c9f0c8.

In the current implementation, the admin is the single point of failure in the apply() function inside of the transfer_admin.rs instruction due to the single-step transfer.

Set pending admin first and then the pending admin has to accept the ownership

Orderly Team: Acknowledged.

In send_request.rs, the constraint backward_fee.order_backward_fee < msg_fee.native_fee @ProxyError:InsufficientMessagingFee is commented out and remains unused. This is inconsistent with the send_claim.rs instruction, where the constraint is actively enforced.

Currently, no overflow risk is present due to the overflow-checks = true setting in Cargo.toml. However, if this setting were to change in the future, the absence of this constraint could introduce an overflow risk.

Consider uncommenting and enforcing the constraint in send_request.rs to maintain consistency with send_claim.rs and mitigate potential overflow risks if overflow checks are ever disabled in future configurations.

Orderly Team: The issue was resolved in commit c6a597a.

EVM $ORDER has 18 decimals precision, but the token on Solana has 10 decimals. This means users will suffer small losses on token bridging.

Document this so the users are aware.

Orderly Team: Acknowledged.

When sending a message from Solana to Orderly, if ORDER tokens are expected in the backward message, an order_backward_fee is charged. However, this fee is applied uniformly across all payload types, without accounting for potential differences in gas requirements.

On the Orderly chain, the LedgerOApp contract maps payload types to gas options, meaning that different payload types may require varying amounts of gas.

This mismatch can lead to inconsistencies between the backward fee charged on Solana and the actual gas sent back from Orderly to Solana.

Consider implementing a mapping of payload types to backward fees on Solana instead of applying a single order_backward_fee across all payload types. Otherwise, document this behavior.

Orderly Team: Acknowledged.

When users initiate withdrawal requests or they claim their vestings or rewards, the amount they are eligible to may be 0. In this case, a cross-chain message won't be executed, but they will still have paid to it in a form of a backwards fee.

No code changes needed, but document this so the users are careful.

Orderly Team: Acknowledged.

In the function _unstakeOrderNow, the penalty for early unstaking is calculated as:

orderAmountForCollect = (_amount * UNSTAKE_NOW_COLLECT_PERCENT) / 100; // 5% of
withdrawn amount

However, this calculation rounds down due to Solidity's integer division behavior, which benefits the user instead of the protocol.

To ensure the penalty always rounds up in favor of the protocol, modify the calculation using ceiling division:

orderAmountForCollect = (_amount * UNSTAKE_NOW_COLLECT_PERCENT + 99) / 100;

Orderly Team: Acknowledged.

The SubmitProof:apply() function in send_claim.rs doesn't check if the proxy is currently paused or not. This allows users to submit proofs during a paused state and can result in unexpected behaviors.

Consider disabling proof submissions if the proxy is paused.

Orderly Team: The issue was resolved in commit 205027f.

For EVM chains, the first message sent cross-chain has a chainedEventId of 0. In contrast, the first cross-chain message sent from SOL has a solanaChainEventId of 1 due to the pre-increment: srcEid = solanaEid + solanaChainEventId.

This difference may be unexpected for Orderly's off-chain systems.

Be aware of this difference and adjust the SOL eventId as necessary for off-chain systems to function appropriately.

Orderly Team: The issue was resolved in commit 3f4524b.

Transactions sent via the Solana proxy to the Orderly network may not execute in the same order they were submitted. This unordered execution can lead to unexpected results.

For example, consider the following scenario:

A user intends to first execute a withdrawOrder transaction, since the unlockTimestamp has been reached, and subsequently initiate a new CreateOrderUnstakeRequest.

The user submits the withdrawOrder request via send_request on the Solana proxy, immediately followed by a CreateOrderUnstakeRequest.

However, due to the non-sequential execution of transactions on the Orderly network, the CreateOrderUnstakeRequest might execute before the withdrawOrder.

If this happens, the unlockTimestamp is extended by an additional unstakeLockPeriod, causing the subsequent withdrawOrder transaction to fail.

When the withdrawOrder transaction fails due to the extended lock period, the fees previously paid for its submission through the Solana proxy are forfeited.

Clearly document this behavior.

Orderly Team: Acknowledged.

The proxy_config's delegate has critical controls over the OApp's configuration. To maximize safety, it is recommended for the delegate to be a multi-sig.

Consider using a multi-sig for the OApp's delegate.

Orderly Team: Acknowledged.

When sending crosschain requests through the solana-proxy, users specify the maximum fee they are ready to pay in both native tokens and lz token.

pub native_fee: u64, pub lz_token_fee: u64

However, quote_claim.rs and quote_send.rs hardcode pay_in_lz_token to false when it quotes the endpoint. Because of this, the quoting mechanism won't work for lz token payments even though the solana proxy allows such messages.

Allow the user to specify whether they want to pay with lz token.

Orderly Team: The issue was resolved in commit 784be09.

The require statement would never revert as the check is already performed in the if condition in the function ledgerSendToVault().

Consider removing the require statement.

Orderly Team: The issue was resolved in commit 53f8eea.

For staking from Solana, the OFT is transferred with the composed message carrying the OCCVaultMessage to be processed on the Ledger Chain.

The compose message is intended to be created by Orderly's frontend, however any user can specify a compose message and custom front ends could be built.

This opens the user to potential issues such as if the composed message contents are malformed and reverts upon lzCompose execution, the user would have lost their funds since the OFT was burnt on src chain.

Clearly document these risks to users.

Orderly Team: Acknowledged.

The internal state in the solana-proxy and OFTStore would be reverted to previous state in case if the Solana chain restarts.

Implement an check to verify the LastRestartSlot state to detect outdated configuration.

Orderly Team: Acknowledged.

The CancelAllVestingRequest payload is no longer supported, but it was left in the payload enum, so the payload types won't be shifted and the system will remain backwards compatible.

However, the code still allows sending such request from the vault sides and the users who initiate them will pay fees for a 100% reverting transaction.

Consider not allowing CancelAllVestingRequests to be sent.

Orderly Team: The issue was resolved in commit 4136571.

The rate limiter's implementation utilizes a basic token bucket model without transaction weighting, making it vulnerable to large transaction dominance.

An attacker can submit a small number of resource-intensive transactions, consuming the rate limit and preventing other users from submitting transactions, leading to potential DoS and resource starvation.

To prevent large transaction dominance, implement a tiered token bucket system. Create separate buckets for distinct transaction size ranges (e.g., small, medium, large).

This ensures dedicated capacity for all transaction sizes, preventing large transactions from monopolizing resources and ensuring fairness.

Orderly Team: Acknowledged.

In the LedgerOCCManager's withdrawTo function, the owner can withdraw ether to a specified address. This is currently done using a low-level transfer function.

However, transfer() only forwards 2300 gas, which is insufficient for the recipient to execute any non-trivial logic in a receive() or fallback function, such as when recipient is a multisig wallet.

Consider using the low-level call function instead of the transfer function.

Orderly Team: The issue was resolved in commit be89754.

The LedgerOApp can be paused by calling the pause() function. However, _lzReceive() doesn't use the whenNotPaused modifier which makes it possible for the proxy to receive messages while it's paused.

In most cases, the proxy on the Solana chain should be paused as well, but in case the protocol team wants to pause only receiving messages on Orderly Network, it won't work.

In addition, even if both proxies are paused, any messages sent before the pause, but still not received, can be executed.

Be aware of this behavior and consider implementing the whenNotPaused modifier on lzReceive().

Orderly Team: Resolved.

The storage layout for LedgerOCCManager is:

• LzTestData (no gap)
• LedgerAccessControl (5 gap slots)
• OCCAdapterDatalayout (50 gap slots)
• LedgerOCCManager (45 gap slots)

LzTestData does not have a storage gap, so caution should be taken not to introduce variables for future upgrades which would lead to collision issues.

Do not introduce new variables to LzTestData for future upgrades.

Orderly Team: Acknowledged.

The current implementation of the proxy initialization functionality is susceptible to front-running attacks as there is no any checks for the signer address.

It has to be ensured that there is no possibility for malicious users to front-run the critical functionality of the protocol. As there is no sufficient validation, the function can be called by any account.

Consider adding the ProgramData account and check for the signer to be the expected upgrade_authority from the program_data.

Orderly Team: Acknowledged.

The LedgerOApp contract on the Ledger chain has its own chainId2Eid and eid2ChainId mappings. If the mappings are updated, the update is not reflected in the mappings within of the LedgerOCCManager contract.

This may lead to inconsistent endpoint<>chain id's across the two contracts. Furthermore, the chainId2Eid is not even utilized beyond the setter function setChainId2Eid.

Consider having the LedgerOApp query the LedgerOCCManager's eid <> chain id mappings to have one source of truth.

Orderly Team: The issue was resolved in commit f1108aa.

Currently, Pause is used on lz_receive in both the Solana proxy and Solana OFT. Suppose a user sends order tokens from the Ledger to Solana. When the user initiates the call on the Ledger the send is not paused; however, when the Executor calls lz_receive on Solana OFT, it is paused.

This means that the user does not receive the order tokens on both chains, causing them to become stuck in the middle. It also causes every Executor call to revert.

Ensure a retry system is implemented and documented.

Orderly Team: Acknowledged.

When users claim their USDC revenue, they will be credited usdcRevenueAmount on the source chain. This amount is in the same decimal precision for all chains (most likely 6 decimals).

However, there are some LayerZero supported networks, like BNB, for example, where the USDC token has different decimals (on BNB it's 18). This will cause a severe loss of funds for these users.

Convert the amount of USDC received to its local decimals amount when it's received in ProxyLedger.vaultRecvFromLedger()

Orderly Team: Acknowledged.

The set_peer_config.rs file includes functionality to update the rate limiter. However, the rate limiter is not implemented in any of the Solana proxy instructions, making this update function redundant.

Review whether the update_rate_limiter function is necessary.

Orderly Team: Acknowledged.

In the ledgerOappSend function, gas for the destination chain (Solana) is retrieved from the payloadType2LzOptions mapping and appended to options. However, if payloadType2LzOptions is unset, it returns a value of zero for gas.

This results in a stuck message on Solana, as no gas is paid for the executor to process the transaction. This behavior differs from the design in the LedgerOCCManager contract, which handles EVM flows differently.

In the buildOCCLedgerMsg function, if _dstGas = 0, a default value of 200000 is assigned to prevent execution failures (see LedgerOCCManager.sol#L130.

Furthermore, the likelihood of payloadType2LzOptions being unset is high because:

• The live LedgerOCCManager contract on the Orderly chain has payloadType2DstGas unset for all

payload types.

• No test files or scripts indicate that the protocol intends to set payloadType2LzOptions.

Modify the ledgerOappSend function to assign a default gas value if typeOptions.gas = 0

Orderly Team: The issue was resolved in commit 46dbcf5.

The LedgerOApp contract includes a receive function, which allows it to accept native ETH. However, it lacks a withdrawal function, meaning any excess ETH that accumulates cannot be recovered when the contract is no longer in use. This could lead to stranded funds.

Other related contracts, such as ProxyLedger and LedgerOCCManager, already include a withdrawTo function, suggesting that LedgerOApp should have one for consistency. Similarly, the proxy_token_account is designed to hold protocol-owned USDC, which users claim.

However, there is no function to withdraw surplus USDC, potentially leaving funds locked in the contract. Since LedgerOCCManager includes a withdrawOrderTo function for ORDER token withdrawals, it would make sense for the Solana proxy to also provide a mechanism to withdraw USDC.

A withdrawTo function should be added to LedgerOApp to allow for the recovery of excess ETH. Additionally, a withdrawUSDCTo function should be implemented in proxy_token_account to ensure surplus USDC can be withdrawn when necessary.

Orderly Team: The issue was resolved in commit b670398.