code-423n4 / 2024-08-superposition-validation

0 stars 0 forks source link

Protocol earnings are permanently lost/locked #54

Open c4-bot-6 opened 2 months ago

c4-bot-6 commented 2 months ago

Lines of code

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

Vulnerability details

Impact

It will be impossible to withdraw the accrued protocol fees, so funds are permanently locked inside the contract.

Proof of Concept

The collect_protocol function is designed to transfer the FUSDC and pool token protocol fees to a recipient:

    #[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> {
        assert_eq_or!(
            msg::sender(),
            self.seawater_admin.get(),
            Error::SeawaterAdminOnly
        );

        let (token_0, token_1) = self
            .pools
            .setter(pool)
            .collect_protocol(amount_0, amount_1)?;

->      erc20::transfer_to_addr(recipient, pool, U256::from(token_0))?;
->      erc20::transfer_to_addr(recipient, FUSDC_ADDR, U256::from(token_1))?;

        #[cfg(feature = "log-events")]
        evm::log(events::CollectProtocolFees {
            pool,
            to: recipient,
            amount0: token_0,
            amount1: token_1,
        });

        // transfer tokens
        Ok((token_0, token_1))
    }

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

The issue is that the token address and recipient are inverted in order, so the previous calls will always revert, as they use the recipient as the token address instead:

    pub fn transfer_to_addr(token: Address, recipient: Address, amount: U256) -> Result<(), Error> {
        safe_transfer(token, recipient, amount)
    }

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

Tools Used

Manual Review

Recommended Mitigation Steps

Consider fixing the argument order:

    #[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> {
        assert_eq_or!(
            msg::sender(),
            self.seawater_admin.get(),
            Error::SeawaterAdminOnly
        );

        let (token_0, token_1) = self
            .pools
            .setter(pool)
            .collect_protocol(amount_0, amount_1)?;

-       erc20::transfer_to_addr(recipient, pool, U256::from(token_0))?;
-       erc20::transfer_to_addr(recipient, FUSDC_ADDR, U256::from(token_1))?;

+       erc20::transfer_to_addr(pool, recipient, U256::from(token_0))?;
+       erc20::transfer_to_addr(FUSDC_ADDR, recipient, U256::from(token_1))?;

        #[cfg(feature = "log-events")]
        evm::log(events::CollectProtocolFees {
            pool,
            to: recipient,
            amount0: token_0,
            amount1: token_1,
        });

        // transfer tokens
        Ok((token_0, token_1))
    }

Assessed type

Token-Transfer

af-afk commented 2 months ago

https://github.com/fluidity-money/long.so/commit/b210e943ffa8c90f27a09b2ed529ac3ae0f84dad