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

No related function to set fee_protocol #8

Open
c4-bot-2 opened this issue Sep 1, 2024 · 4 comments
Open

No related function to set fee_protocol #8

c4-bot-2 opened this issue Sep 1, 2024 · 4 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 M-12 primary issue Highest quality submission among a set of duplicates 🤖_03_group AI based duplicate group recommendation satisfactory satisfies C4 submission criteria; eligible for awards selected for report This submission will be included/highlighted in the audit report sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") sufficient quality report This report is of sufficient quality

Comments

@c4-bot-2
Copy link
Contributor

c4-bot-2 commented Sep 1, 2024

Lines of code

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

Vulnerability details

Impact

There are no related functions to set fee_protocol, which prevents the protocol from accumulating protocol fees.

Proof of Concept

The pool contract defines the protocol fee rate fee_protocol, but there is no function to set it.

#[solidity_storage]
pub struct StoragePool {
    ...
    fee_protocol: StorageU8,
    fee_growth_global_0: StorageU256,
    fee_growth_global_1: StorageU256,
    protocol_fee_0: StorageU128,
    protocol_fee_1: StorageU128,
    ...
}

The contract also defines the collect_protocol and collect_protocol_7540_F_A_9_F functions to collect fees. However, since the protocol fee rate cannot be set, the protocol will never accumulate any protocol fees.

github:link
pkg/seawater/src/lib.rs

    #[allow(non_snake_case)]
    pub fn collect_protocol_7540_F_A_9_F(
        &mut self,
        pool: Address,
        amount_0: u128,
        amount_1: u128,
        recipient: Address,
    ) -> Result<(u128, u128), Revert> {
        ...
    }

github: link
pkg/seawater/src/pool.rs

    pub fn collect_protocol(
        &mut self,
        amount_0: u128,
        amount_1: u128,
    ) -> Result<(u128, u128), Revert> {
        ...
    }

Tools Used

Manual Review

Recommended Mitigation Steps

Add the relevant functions to enable protocol fees.

pkg/seawater/src/pool.rs

+    pub fn set_fee_protocol(&mut self, new_fee_protocol: U256) {
+        self.fee_protocol.set(new_fee_protocol);
+    }

pkg/seawater/src/lib.rs

+    #[allow(non_snake_case)]
+    pub fn set_fee_protocol(
+        &mut self,
+        pool: Address,
+        new_fee_protocol: U256,
+    ) -> Result<(), Revert> {
+        assert_eq_or!(
+            msg::sender(),
+            self.seawater_admin.get(),
+            Error::SeawaterAdminOnly
+        );
+
+        self.pools.setter(pool).set_fee_protocol(new_fee_protocol);
+
+        Ok(())
+    }

Assessed type

Other

@c4-bot-2 c4-bot-2 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 1, 2024
c4-bot-2 added a commit that referenced this issue Sep 1, 2024
@c4-bot-11 c4-bot-11 added the 🤖_03_group AI based duplicate group recommendation label Sep 13, 2024
@howlbot-integration howlbot-integration bot added primary issue Highest quality submission among a set of duplicates sufficient quality report This report is of sufficient quality labels Sep 16, 2024
@af-afk af-afk added the sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") label Sep 17, 2024
@af-afk
Copy link

af-afk commented Sep 23, 2024

@alex-ppg
Copy link

The submission and its duplicates have correctly identified that there is no mechanism to set the protocol fee in the system, causing fees to never accumulate in the current implementation.

I believe a severity of medium is appropriate given that its only harm is prospective earnings for the protocol itself.

@c4-judge
Copy link
Contributor

alex-ppg marked the issue as selected for report

@c4-judge c4-judge added the selected for report This submission will be included/highlighted in the audit report label Sep 23, 2024
@c4-judge
Copy link
Contributor

alex-ppg marked the issue as satisfactory

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 M-12 primary issue Highest quality submission among a set of duplicates 🤖_03_group AI based duplicate group recommendation satisfactory satisfies C4 submission criteria; eligible for awards selected for report This submission will be included/highlighted in the audit report sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") sufficient quality report This report is of sufficient quality
Projects
None yet
Development

No branches or pull requests

6 participants