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

Missing access control in create_pool_D650_E2_D0() allows for unauthorized pool creation #9

Closed
howlbot-integration bot opened this issue Nov 4, 2024 · 3 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 edited-by-warden primary issue Highest quality submission among a set of duplicates 🤖_primary AI based primary recommendation sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue sufficient quality report This report is of sufficient quality unsatisfactory does not satisfy C4 submission criteria; not eligible for awards

Comments

@howlbot-integration
Copy link

Lines of code

https://github.com/code-423n4/2024-10-superposition/blob/7ad51104a8514d46e5c3d756264564426f2927fe/pkg/seawater/src/lib.rs#L120-L121
https://github.com/code-423n4/2024-10-superposition/blob/7ad51104a8514d46e5c3d756264564426f2927fe/pkg/seawater/src/lib.rs#L802-L834

Vulnerability details

Proof of Concept

The following is given:

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

However, the create_pool_D650_E2_D0() function does not check if the caller has the necessary permissions, meaning any actor could potentially create a pool, leading to unauthorized pool creation.

    pub fn create_pool_D650_E2_D0(
        &mut self,
        pool: Address,
        price: U256,
        fee: u32,
    ) -> Result<(), Revert> {
        
        let tick_spacing = match fee {
            0 => Ok(1),
            500 => Ok(10),
            3000 => Ok(60),
            10_000 => Ok(200),
            _ => Err(Error::BadFee)
        }?;

        let max_liq_per_tick = tick_math::tick_spacing_to_max_liq(tick_spacing)?;

        self.pools
            .setter(pool)
            .init(price, fee, tick_spacing, max_liq_per_tick)?; 

        // Get the decimals for the asset so we can log its decimals for the indexer
        let _decimals = erc20::decimals(pool)?;

        evm::log(events::NewPool {
            token: pool,
            fee,
            decimals: _decimals,
            tickSpacing: tick_spacing,
        });

        Ok(())
    }

The function lacks a check against the authorised_enablers map, which is intended to restrict pool creation to authorized addresses.

Impact

This could result in unauthorized or malicious pool creations. Since no verification is in place, it exposes the contract to potential misuse, such as the creation of pools with invalid parameters or for tokens not intended to be supported by the system.

Recommended Mitigation Steps

Introduce an access control check at the start of the function to verify if the caller is authorized.

+       assert_or!(
+           self.authorised_enablers.get(msg::sender()),
+           Error::Unauthorized
+       );

Assessed type

Access Control

@howlbot-integration howlbot-integration bot added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value 🤖_primary AI based primary recommendation bug Something isn't working edited-by-warden primary issue Highest quality submission among a set of duplicates sufficient quality report This report is of sufficient quality labels Nov 4, 2024
howlbot-integration bot added a commit that referenced this issue Nov 4, 2024
@af-afk af-afk added the sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue label Nov 7, 2024
@af-afk
Copy link

af-afk commented Nov 7, 2024

This is intended functionality! For this reason, the pool is disabled by default.

@alex-ppg
Copy link

The submission attempts to outline an issue around access control for pool creations, however, the pools created are disabled by default as the Sponsor has established and thus it is acceptable for them to be permissionlessly created.

@c4-judge c4-judge added the unsatisfactory does not satisfy C4 submission criteria; not eligible for awards label Nov 13, 2024
@c4-judge
Copy link

alex-ppg marked the issue as unsatisfactory:
Invalid

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 edited-by-warden primary issue Highest quality submission among a set of duplicates 🤖_primary AI based primary recommendation sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue sufficient quality report This report is of sufficient quality unsatisfactory does not satisfy C4 submission criteria; not eligible for awards
Projects
None yet
Development

No branches or pull requests

3 participants