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

0 stars 0 forks source link

Radiant Punch Dalmatian - Incorrect implementation of quote instruction leads users to pay extra fees for deposits #103

Open sherlock-admin4 opened 4 days ago

sherlock-admin4 commented 4 days ago

Radiant Punch Dalmatian

Medium

Incorrect implementation of quote instruction leads users to pay extra fees for deposits

Summary

The USDC deposits on solana_vault consists of two steps:

  1. User calls quote instruction with the deposit message and parameters. OAppQuote returns the LZ messaging fees for sending the deposit message.
  2. User calls deposit with the MessagingFee returned by quote instruction and the deposit parameters.
    • The deposit function calls endpoint::send function with the deposit message and the MessagingFee parameters provided by the user.

The quote function uses a different LZ message options than the options used by the deposit. As a result, the quote returns higher fees than the fees required by the deposit function leading users to pay more fees.

Root Cause

The quote function incorrectly uses enforced_options.send_and_call options to compute the fees while the deposit, which actually sends the deposit message, uses enforced_options.send options.

The quote function incorrectly uses enforced_options.send_and_call options:

https://github.com/sherlock-audit/2024-09-orderly-network-solana-contract/blob/main/solana-vault/packages/solana/contracts/programs/solana-vault/src/instructions/oapp_instr/oapp_quote.rs#L40-L50

The params.message will be the deposit message and it is not None and the params.options will be empty. The options are computed in the enforced_options.combine_options function:

https://github.com/sherlock-audit/2024-09-orderly-network-solana-contract/blob/main/solana-vault/packages/solana/contracts/programs/solana-vault/src/state/oapp_state/enforced_options.rs#L25-L36

The EnforcedOptions::combine_options returns the send_and_call options. The send_and_call options include the compose message options additional to the send and hence require more LZ fees than the enforced_options.send options.

The deposit instruction uses the return value of EnforcedOptions::get_enforced_options function as options:

https://github.com/sherlock-audit/2024-09-orderly-network-solana-contract/blob/main/solana-vault/packages/solana/contracts/programs/solana-vault/src/instructions/vault_instr/deposit.rs#L141-L147

The get_enforced_options function returns self.send options as the composed_msg parameter is set to None:

https://github.com/sherlock-audit/2024-09-orderly-network-solana-contract/blob/main/solana-vault/packages/solana/contracts/programs/solana-vault/src/state/oapp_state/enforced_options.rs#L17-L23

Internal pre-conditions

No pre-conditions are required

External pre-conditions

No pre-conditions are required

Attack Path

Happens by itself. User uses the frontend to perform deposits.

Impact

The users will lose funds in the form of LZ fees for every deposit on solana_vault by paying higher fees than required.

PoC

No response

Mitigation

Update the quote function to use None for compose_msg argument in the call to EnforcedOptions::combine_options.