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

0 stars 0 forks source link

Strong Magenta Loris - Missing Initialization Check in initialize Method #49

Open sherlock-admin3 opened 13 hours ago

sherlock-admin3 commented 13 hours ago

Strong Magenta Loris

High

Missing Initialization Check in initialize Method

Summary

The initialize method in the RebateManager contract lacks a crucial check to prevent it from being called multiple times. As such, it allows an attacker to re-initialize the contract.

Vulnerability Detail

The current initialize method doesn't verify whether the RebateManager has already been initialized. This means any user can call this method at any time to overwrite the existing configuration. By re-calling initialize, an attacker can change the authority, quote_token_mint, and token_vault to values they control. This effectively hands over control of the contract and its assets to the attacker.

Impact

The contract can be reinitialized and as such, an attacker can become the new authority, gaining full control over administrative functions.

Code Snippet

https://github.com/sherlock-audit/2024-08-woofi-solana-deployment/blob/main/WOOFi_Solana/programs/rebate_manager/src/state/rebate_manager.rs#L61-L72

Tool used

Manual Review

Recommendation

Introduce a check at the beginning of the initialize method to ensure it can only be executed once. One common approach is to verify that a specific field is unset (e.g., authority is still Pubkey::default()) before proceeding.

    pub fn initialize(
        &mut self,
        authority: Pubkey,
        quote_token_mint: Pubkey,
        token_vault: Pubkey,
    ) -> Result<()> {
        require!(
            self.authority == Pubkey::default(),
            ErrorCode::AlreadyInitialized
        );

        self.authority = authority;
        self.quote_token_mint = quote_token_mint;
        self.token_vault = token_vault;

        Ok(())
    }

By adding this check, any attempt to re-initialize an already initialized RebateManager will fail.

toprince commented 10 hours ago

The initialize fn only called by anchor create. So already avoided this issue?