Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

authorised_enablers role Inconsistencies in Pool Management. #203

Open
c4-bot-7 opened this issue Sep 13, 2024 · 0 comments
Open

authorised_enablers role Inconsistencies in Pool Management. #203

c4-bot-7 opened this issue Sep 13, 2024 · 0 comments
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working 🤖_04_group AI based duplicate group recommendation sufficient quality report This report is of sufficient quality

Comments

@c4-bot-7
Copy link
Contributor

Lines of code

https://github.com/code-423n4/2024-08-superposition/blob/main/pkg/seawater/src/lib.rs#L120

Vulnerability details

In the current implementation, the role of authorised_enablers has inconsistencies:

 // authorised enablers to create new pools, and enable them
 authorised_enablers: StorageMap<Address, StorageBool>,
  • Disabling Pools: While the role description of authorised_enablers suggest they are only authorized to enable pools, however they can also disable them. This dual functionality is not consistent with the intended scope of the role.

  • Pool Creation: The authorised_enablers are not allowed to create new pools, as this responsibility is exclusively reserved for the admin. However, the role suggests they have broader control over pool creation.

Impact

Role Confusion and Security Risks: The dual functionality of enabling and disabling pools by authorised_enablers introduces confusion about the intended roles and permissions. This can lead to unauthorized or unexpected actions.

Proof of Concept

@>> // authorised enablers to create new pools, and enable them
@>> authorised_enablers: StorageMap<Address, StorageBool>,
  • Creating Pools
@>> /// Creates a new pool. Only usable by the seawater admin.
@>> /// Requires the caller to be the seawater admin. Requires the pool to not exist.
    #[allow(non_snake_case)]
    pub fn create_pool_D650_E2_D0(
        &mut self,
        pool: Address,
        price: U256,
        fee: u32,
        tick_spacing: u8,
        max_liquidity_per_tick: u128,
    ) -> Result<(), Revert> {
        assert_eq_or!(
            msg::sender(),
@>>         self.seawater_admin.get(),
            Error::SeawaterAdminOnly
        );
  • Pool Enable / Disable
    pub fn enable_pool_579_D_A658(&mut self, pool: Address, enabled: bool) -> Result<(), Revert> {
        assert_or!(
            self.seawater_admin.get() == msg::sender()
                || self.emergency_council.get() == msg::sender()
@>>             || self.authorised_enablers.get(msg::sender()),
            Error::SeawaterAdminOnly
        );

        if self.emergency_council.get() == msg::sender()
            && self.seawater_admin.get() != msg::sender()
            && enabled
        {
            // Emergency council can only disable!
            return Err(Error::SeawaterEmergencyOnlyDisable.into());
        }

        self.pools.setter(pool).set_enabled(enabled);
        Ok(())
    }

Tools Used

Manual Review

Recommended Mitigation Steps

Implement functionality according to the role specification.

Assessed type

Access Control

@c4-bot-7 c4-bot-7 added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working labels Sep 13, 2024
c4-bot-5 added a commit that referenced this issue Sep 13, 2024
@c4-bot-12 c4-bot-12 added the 🤖_04_group AI based duplicate group recommendation label Sep 13, 2024
howlbot-integration bot added a commit that referenced this issue Sep 16, 2024
@howlbot-integration howlbot-integration bot added the sufficient quality report This report is of sufficient quality label Sep 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working 🤖_04_group AI based duplicate group recommendation sufficient quality report This report is of sufficient quality
Projects
None yet
Development

No branches or pull requests

2 participants