code-423n4 / 2024-02-hydradx-findings

1 stars 0 forks source link

An asset in stablepool can be DoS #162

Closed c4-bot-6 closed 6 months ago

c4-bot-6 commented 6 months ago

Lines of code

https://github.com/code-423n4/2024-02-hydradx/blob/603187123a20e0cb8a7ea85c6a6d718429caad8d/HydraDX-node/pallets/stableswap/src/lib.rs#L551

Vulnerability details

Impact

The attacker uses A token in the same pool to add liquidity and remove B tokens, leaving B tokens missing in the pool.

Proof of Concept

In stablepool, liquidity providers get shares for any token they add:

    #[require_transactional]
    fn do_add_liquidity(
        who: &T::AccountId,
        pool_id: T::AssetId,
        assets: &[AssetAmount<T::AssetId>],
    ) -> Result<Balance, DispatchError> {
        ....
        T::Currency::deposit(pool_id, who, share_amount)?;
        ....
    }

By T::Currency::free_balance(pool_id, who); can got user's share balance:

    let current_share_balance = T::Currency::free_balance(pool_id, who);

Users can use share balance to retrieve the asset provided when adding liquidity

    #[transactional]
    pub fn remove_liquidity_one_asset(
        origin: OriginFor<T>,
        pool_id: T::AssetId,
        asset_id: T::AssetId,
        share_amount: Balance,
        min_amount_out: Balance,
    ) -> DispatchResult {
        ....
        T::Currency::withdraw(pool_id, &who, share_amount)?;
        T::Currency::transfer(asset_id, &pool_account, &who, amount)?;
        ......
    }

However, the user can use share balance to retrieve any token in the pool.

Therefore, the attacker can deposit one token to get the share balance, and then withdraw another token. The attacker can attack one token and make one cannot be exchanged.

Tools Used

vscode, manual

Recommended Mitigation Steps

Liquidity providers are not allowed to remove arbitrary tokens through share balance, A token can only be withdrawn if A token is added.

Assessed type

DoS

c4-pre-sort commented 6 months ago

0xRobocop marked the issue as insufficient quality report

OpenCoreCH commented 6 months ago

By design / no vulnerability presented

c4-judge commented 6 months ago

OpenCoreCH marked the issue as unsatisfactory: Invalid