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

2 stars 2 forks source link

0xeix - UnPause struct has no check for the pause_authority #1

Open sherlock-admin3 opened 1 month ago

sherlock-admin3 commented 1 month ago

0xeix

Medium

UnPause struct has no check for the pause_authority

Summary

Constraints in the UnPause struct do not check for the pause_authority making tx revert.

Vulnerability Detail

In the current implementation of the UnPause struct inside of pause_unpause.rs instructions, there is no check implemented to make sure the pause_authority that's responsible for the pause / unpause calls can actually call it:

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

pub struct UnPause<'info> {
    #[account(mut,
        constraint =
            wooconfig.authority == authority.key() ||
            wooconfig.woopool_admin_authority.contains(authority.key)
    )]
    pub wooconfig: Account<'info, WooConfig>,

    pub authority: Signer<'info>,
}

As you can see, the given constraint only checks whether the authority is the signer of the transaction and whether woopool_admin_auhority contains the authority key. However, this deviates from the expected behavior as pause_authority has to be able to call the function as well, similar to the Pause struct:

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

 #[account(mut,
        constraint =
            wooconfig.authority == authority.key() ||
            wooconfig.woopool_admin_authority.contains(authority.key) ||
            wooconfig.pause_authority.contains(authority.key)
    )]
    pub wooconfig: Account<'info, WooConfig>,

Impact

The transaction of the pause_authority will revert due to the access control check.

Code Snippet

Provided above.

Tool used

Manual Review

Recommendation

Implement wooconfig.pause_authority.contains(authority.key) in the Unpause struct.

toprince commented 1 month ago

Not valid for this one. This is by design, puase authority can puase the pool, but only admin can unpause the pool.