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

0 stars 0 forks source link

Helpful Jetblack Snake - Missing permission control in create_oracle and create_pool. #54

Open sherlock-admin2 opened 13 hours ago

sherlock-admin2 commented 13 hours ago

Helpful Jetblack Snake

Medium

Missing permission control in create_oracle and create_pool.

Summary

Missing permission control in create_oracle and create_pool.

Vulnerability Detail

#[derive(Accounts)]
pub struct CreateWooracle<'info> {
    pub wooconfig: Box<Account<'info, WooConfig>>,
    pub token_mint: Account<'info, Mint>,

    #[account(
        init,
        payer = admin,
        space = 8 + Wooracle::INIT_SPACE,
        seeds = [
            WOORACLE_SEED.as_bytes(),
            wooconfig.key().as_ref(),
            token_mint.key().as_ref(),
            feed_account.key().as_ref(),
            price_update.key().as_ref()
            ],
        bump,
    )]
    wooracle: Account<'info, Wooracle>,
    #[account(mut)]
@>>    admin: Signer<'info>,
    system_program: Program<'info, System>,
    /// CHECK: This is the Pyth feed account
    feed_account: AccountInfo<'info>,
    // Add this account to any instruction Context that needs price data.
    // Warning:
    // users must ensure that the account passed to their instruction is owned by the Pyth pull oracle program.
    // Using Anchor with the Account<'info, PriceUpdateV2> type will automatically perform this check.
    // However, if you are not using Anchor, it is your responsibility to perform this check.
    price_update: Account<'info, PriceUpdateV2>,

    quote_token_mint: Account<'info, Mint>,
    /// CHECK: This is the Quote token's pyth feed account
    quote_feed_account: AccountInfo<'info>,
    // Add this account to any instruction Context that needs price data.
    // Warning:
    // users must ensure that the account passed to their instruction is owned by the Pyth pull oracle program.
    // Using Anchor with the Account<'info, PriceUpdateV2> type will automatically perform this check.
    // However, if you are not using Anchor, it is your responsibility to perform this check.
    quote_price_update: Account<'info, PriceUpdateV2>,
}

pub fn handler(ctx: Context<CreateWooracle>, maximum_age: u64) -> Result<()> {
    ctx.accounts.wooracle.wooconfig = ctx.accounts.wooconfig.key();
@>>    ctx.accounts.wooracle.authority = ctx.accounts.admin.key();
    ctx.accounts.wooracle.token_mint = ctx.accounts.token_mint.key();
    ctx.accounts.wooracle.feed_account = ctx.accounts.feed_account.key();
    ctx.accounts.wooracle.price_update = ctx.accounts.price_update.key();

   //----skip
}

From the code, it is evident that create_wooracle lacks permission control, allowing anyone to create an oracle.

#[derive(Accounts)]
pub struct CreatePool<'info> {
    pub wooconfig: Box<Account<'info, WooConfig>>,
    pub token_mint: Account<'info, Mint>,
    pub quote_token_mint: Account<'info, Mint>,

    #[account(mut)]
@>>    pub authority: Signer<'info>,

    #[account(
        init,
        payer = authority,
        space = 8 + WooPool::INIT_SPACE,
        seeds = [
          WOOPOOL_SEED.as_bytes(),
          wooconfig.key().as_ref(),
          token_mint.key().as_ref(),
          quote_token_mint.key().as_ref()
        ],
        bump)]
    pub woopool: Box<Account<'info, WooPool>>,

    #[account(
        init,
        payer = authority,
        token::mint = token_mint,
        token::authority = woopool
      )]
    pub token_vault: Box<Account<'info, TokenAccount>>,

    #[account(
        has_one = wooconfig,
@>>        has_one = authority,
        has_one = token_mint,
        has_one = quote_token_mint
    )]
    wooracle: Account<'info, Wooracle>,

    #[account(address = token::ID)]
    pub token_program: Program<'info, Token>,
    pub system_program: Program<'info, System>,
}

From the above code, it is clear that the permission control for create_pool requires its authority to match wooracle.authority. However, since anyone can create an oracle, an attacker could create an oracle and then create a pool based on that oracle. This breaks the statement made in the README. "Functions need admin authority: claim_fee claim_rebate_fee create_oracle create_pool create_rebate_pool deposit set_pool_admin set_pool_state (all handlers in this file) set_woo_admin set_woo_state(all handlers in this file)"

Impact

There are two further impacts:

1.When the protocol has been running for a period of time, such as 6 months, and wants to add other token pairs into the swap (e.g., WETH-USDT), the real administrator may not be able to create the WETH oracle because the WETH oracle was created by someone else.

2. An attacker could create oracles and pools for other token pairs (e.g., WETH-USDT) and set the oracle’s price and bounds to manipulate prices. Even if there’s a comparison with Pyth’s prices, it could still pass. As a result, the attacker could trade with the manipulated prices against pools (e.g., USDT, USDC) and steal funds from the pools.

Code Snippet

https://github.com/sherlock-audit/2024-08-woofi-solana-deployment/blob/main/WOOFi_Solana/programs/woofi/src/instructions/admin/create_wooracle.rs#L42

https://github.com/sherlock-audit/2024-08-woofi-solana-deployment/blob/main/WOOFi_Solana/programs/woofi/src/instructions/admin/create_pool.rs#L8

Tool used

Manual Review

Recommendation

Set the admin parameter in CreateWooracle to admin = wooconfig.authority.

toprince commented 10 hours ago

Impact 1 is valid, it is low impact. Impact 2 is not valid, please verify if it can swap with correct pool.