sherlock-audit / 2024-09-orderly-network-solana-contract-judging

0 stars 0 forks source link

Sunny Syrup Worm - Inadequate User Verification Allows Unauthorized Token Redirection #97

Open sherlock-admin4 opened 4 days ago

sherlock-admin4 commented 4 days ago

Sunny Syrup Worm

High

Inadequate User Verification Allows Unauthorized Token Redirection

Summary

A verified withdrawal message can be maliciously redirected to an attacker's account due to an inadequate user account check in OAppLzReceive, allowing funds intended for a legitimate receiver to be sent to an unauthorized user.

Root Cause

When the ledger processes a withdrawal through the SolConnector::withdraw function, the message is sent via LayerZero to the Orderly vault in Solana.
In Solana, the message is verified by the endpoint::verify function, and upon successful verification, the PacketVerifiedEvent is emitted, making the message executable.
A malicious user listening to PacketVerifiedEvent for the oapp receiver can then deliver the message to solana_vault::lz_receive with the message as verified in layerZero endpoint but with their account as the user instead of the receiver.

    #[account()]
    pub user: AccountInfo<'info>,

In the constructed withdrawal message in SolConnector, the intended receiver is defined here:

// AccountTypes
struct AccountWithdrawSol {
    bytes32 accountId;
    bytes32 sender;
    bytes32 receiver;  //<-----  @
    bytes32 brokerHash;
    bytes32 tokenHash;
    uint128 tokenAmount;
    uint128 fee;
    uint256 chainId;
    uint64 withdrawNonce;
}

Due to missing constraint checks, the tokens are sent from the vault wallet to the user wallet instead:

    fn transfer_token_ctx(&self) -> CpiContext<'_, '_, '_, 'info, Transfer<'info>> {
        let cpi_accounts = Transfer {
            from: self.vault_deposit_wallet.to_account_info(),
            to: self.user_deposit_wallet.to_account_info(),
            authority: self.vault_authority.to_account_info(),
        };
        let cpi_program = self.token_program.to_account_info();
        CpiContext::new(cpi_program, cpi_accounts)
    }

Internal pre-conditions

None

External pre-conditions

None

Attack Path

  1. The ledger issues a cross-chain withdrawal to solana via SolConnector::withdraw function
  2. After the message has been verified, the attacker calls solana_vault::lz_receive with the verified message, but with his account as the user account.

Impact

During a withdrawal, tokens intended for the receiver are at risk of being hijacked and redirected to unauthorized user accounts.

PoC

https://github.com/sherlock-audit/2024-09-orderly-network-solana-contract/blob/a40ed80ce4a196bc81bfa6dfb749c19b92c623b0/solana-vault/packages/solana/contracts/programs/solana-vault/src/lib.rs#L69-L71

Mitigation

Add a get_receiver_address function to AccountWithdrawSol to verify the intended recipient:

    pub fn get_receiver_address(encoded: &[u8]) -> Result<Pubkey> {
        // Decode the LzMessage to get the payload
        let message = LzMessage::decode(encoded)?;

        // Decode the payload
        let withdraw_params = AccountWithdrawSol::decode_packed(&message.payload)?;

        // Return the receiver address as a Pubkey
        Ok(Pubkey::new_from_array(withdraw_params.receiver))
    }

Then update OAppLzReceive::user to:

    #[account(address = AccountWithdrawSol::get_receiver_address(&params.message)?)]
    pub user: AccountInfo<'info>,
sherlock-admin2 commented 9 hours ago

The protocol team fixed this issue in the following PRs/commits: https://github.com/OrderlyNetwork/solana-vault/pull/4/commits/a9f56db5e63562df9eb6a39803f3df12b7959032