oappConfig init can be called multiple times to take control of the admin

Summary

In oapp_config.rs, init can be called multiple times to override the OAppConfig admin.

Root Cause

In oapp_config.init(), there is no access control check to ensure that this function can only be called once. Anyone can call the function again to take over the admin.

impl OAppConfig {
    // todo: optimize
    pub fn init(
        &mut self,
        endpoint_program: Option<Pubkey>,
        admin: Pubkey,
        accounts: &[AccountInfo],
        oapp_signer: Pubkey,
    ) -> Result<()> {
 >      self.admin = admin;
        self.endpoint_program = if let Some(endpoint_program) = endpoint_program {
            endpoint_program
        } else {
            ENDPOINT_ID
        };

        // register oapp
        oapp::endpoint_cpi::register_oapp(
            self.endpoint_program,
            oapp_signer,
            accounts,
            &[OAPP_SEED, &[self.bump]],
            RegisterOAppParams {
                delegate: self.admin,
            },
        )
    }
}

Internal pre-conditions

External pre-conditions

Attack Path

Attacker calls init() with a new set of parameters, setting himself as admin.

Impact

Attacker can take control of most of the state of the protocol

PoC

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/oapp_config.rs#L15

Mitigation

Add a bool field to the OAppConfig struct:

#[account]
pub struct OAppConfig {
    pub endpoint_program: Pubkey,
    pub bump: u8,
    pub admin: Pubkey,
    pub usdc_hash: [u8; 32],
    pub usdc_mint: Pubkey,
    pub initialized: bool, // New flag to track initialization status
}

In the init function, check if self.initialized is true. If it is, return an error. Otherwise, proceed with initialization and set initialized to true.

impl OAppConfig {
    pub fn init(
        &mut self,
        endpoint_program: Option<Pubkey>,
        admin: Pubkey,
        accounts: &[AccountInfo],
        oapp_signer: Pubkey,
    ) -> Result<()> {
        // Check if already initialized
        if self.initialized {
            return Err(ErrorCode::AlreadyInitialized.into());
        }

        self.admin = admin;
        self.initialized = true;  // Set initialized to true

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

Droll Cider Armadillo - oappConfig init can be called multiple times to take control of the admin #113