sherlock-audit / 2024-08-woofi-solana-deployment-judging

0 stars 0 forks source link

Fast Sand Millipede - get_price.rs - Missing Ownership Validation on Oracle Accounts #11

Open sherlock-admin4 opened 15 hours ago

sherlock-admin4 commented 15 hours ago

Fast Sand Millipede

High

get_price.rs - Missing Ownership Validation on Oracle Accounts

Summary

The contract does not verify that the price_update and quote_price_update accounts used to fetch oracle price data are owned by the Pyth oracle program.

This creates a vulnerability where an attacker could provide impersonator accounts that mimic the structure of legitimate oracle accounts but deliver manipulated price data.

Vulnerability Detail

The contract interacts with the following accounts for price updates but lacks ownership checks:

pub struct GetPrice<'info> {
    #[account(
        has_one = wooconfig,
        has_one = price_update,
        has_one = quote_price_update,
    )]
    pub oracle: Account<'info, Wooracle>,
    -->
    pub price_update: Account<'info, PriceUpdateV2>,
    pub quote_price_update: Account<'info, PriceUpdateV2>,
}

Impact

Data Manipulation: Attackers could create fake price_update and quote_price_update accounts and inject manipulated price data into the contract.

This can lead to:

Code Snippet

https://github.com/sherlock-audit/2024-08-woofi-solana-deployment/blob/main/WOOFi_Solana/programs/woofi/src/instructions/get_price.rs#L19-L20

Tool used

Manual Review

Recommendation

To mitigate this issue, implement ownership validation on the price_update and quote_price_update accounts to ensure they belong to the Pyth oracle program. In Anchor, this can be done by adding an owner check in the account definition.

Like this:

#[account(
    owner = pyth_solana_receiver_sdk::ID // Replace with the actual Pyth program ID
)]
pub price_update: Account<'info, PriceUpdateV2>,

#[account(
    owner = pyth_solana_receiver_sdk::ID // Replace with the actual Pyth program ID
)]
pub quote_price_update: Account<'info, PriceUpdateV2>,

This ensures that both the price_update and quote_price_update accounts are owned by the Pyth oracle program, preventing the use of manipulated accounts.

toprince commented 13 hours ago

Need further investigation.

  1. Due to pyth's document, PriceUpdateV2 will check this for us.
  2. Get Price is only for reading, will not have big impact.
  3. When doing swap, the feed is checked by constraints in oracle.

So currently, in my opinion, it is invalid. Need more proof.