The `authorised_enablers` can disable pools

[howlbot-integration[bot]]

Lines of code

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

Vulnerability details

According to the documentation, the authorised_enablers should only be allowed to create new pools and enable them:

    // authorised enablers to create new pools, and enable them
    authorised_enablers: StorageMap<Address, StorageBool>,

lib.rs#L120-L121

However, the current enable_pool_579_D_A658 function implementation allows senders with the authorised_enablers role to not only enable the pool but also disable it.

Impact

Because liquidity cannot be removed when the pool is locked, allowing senders with the authorised_enablers role to disable pools is a critical access control issue. This could allow a malicious sender to lock the liquidity of all users until the seawater admin restricts the malicious enabler.

Proof of Concept

The authorized enabler calls the enable_pool_579_D_A658 function with a pool address and enabled parameter set to false (see comments):

    pub fn enable_pool_579_D_A658(&mut self, pool: Address, enabled: bool) -Result<(), Revert{
        assert_or!(
            self.seawater_admin.get() == msg::sender() // @audit false
                || self.emergency_council.get() == msg::sender() // @audit false
                || self.authorised_enablers.get(msg::sender()), // @audit true
            Error::SeawaterAdminOnly
        ); // @audit the assertion passed (false || false || true)

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

        // @audit proceeds with disabling the pool
        self.pools.setter(pool).set_enabled(enabled);
        Ok(())
    }

lib.rs#L1169-L1187

Tools Used

Manual review.

Recommended Mitigation Steps

Add proper checks to prevent authorized enablers from disabling pools. For instance, add the following check at the beginning of the enable_pool_579_D_A658 function:

        if self.authorised_enablers.get() == msg::sender()
            && self.seawater_admin.get() != msg::sender()
            && enabled
        {
            // Enabler cannot disable!
            return Err(Error::SeawaterEnablerCannotDisable.into());
        }

Assessed type

Access Control

[c4-judge]

alex-ppg changed the severity to QA (Quality Assurance)

[c4-judge]

alex-ppg marked the issue as grade-c

[c4-judge]

alex-ppg marked the issue as grade-b