hats-finance / Common--Stableswap-0xd4d9a2772202ce33b24901d3fc94e95a84b37430

Apache License 2.0
0 stars 0 forks source link

Missing deadline check allows outdated slippage and allows pending transactions to be unexpectedly executed. #32

Open hats-bug-reporter[bot] opened 1 month ago

hats-bug-reporter[bot] commented 1 month ago

Github username: -- Twitter username: -- Submission hash (on-chain): 0x50b220940b491e2343b55f72ef9e95bdbdca2d18645917e18db52213c0b35f64 Severity: medium

Description: Description\ In StablePoolContract, the swap and liquidity functions don’t have a deadline check.

This allows outdated slippage and pending transactions to be unexpectedly executed.

Attack Scenario\ AMMs provide their users with an option to limit the execution of their pending actions, such as swaps or adding and removing liquidity.

The most common solution is to include a deadline timestamp as a parameter (for example see Uniswap V2 and Uniswap V3).

If such an option is not present, users can unknowingly perform bad trades:

Alice wants to swap 100 tokens for 1 AZERO and later sell the 1 AZERO for 50 DAI.

The transaction is submitted to the mempool, however, Alice chose a transaction fee that is too low for validators to be interested in including her transaction in a block (or the transaction fee increased after her submission).

The transaction stays pending in the mempool for extended periods, which could be hours, days, weeks, or even longer.

When the average gas fee dropped far enough for Alice's transaction to become interesting again for validators to include it, her swap will be executed.

In the meantime, the price of AZERO could have drastically changed.

She will still get 1 AZERO but the DAI value of that output might be significantly lower.

She has unknowingly performed a bad trade due to the pending transaction she forgot about.

An even worse way this issue can be maliciously exploited is through MEV:

The swap transaction is still pending in the mempool. Average fees are still too high for validators to be interested in it.

The price of tokens has gone up significantly since the transaction was signed, meaning Alice would receive a lot more AZERO when the swap is executed.

But that also means that her maximum slippage value is outdated and would allow for significant slippage.

A MEV bot detects the pending transaction. Since the outdated maximum slippage value now allows for high slippage, the bot sandwiches Alice, resulting in significant profit for the bot and significant loss for Alice.

Recommendation\ Add a deadline input in the swap and liquidity functions.

JanKuczma commented 1 month ago

Thank you for your submission.

Alice wants to swap 100 tokens for 1 AZERO and later sell the 1 AZERO for 50 DAI.

Do you mean 2 separate swaps? That is in 2 separate transactions? What " 100 tokens"? This smart contract is meant for stable tokens (like pegged to a common fiat currency OR pegged to each other) meaning that a pool AZERO x DAI would not make sense. This makes this submission invalid.

The most common solution is to include a deadline timestamp as a parameter (for example see Uniswap V2 and Uniswap V3).

There's no need for it in the stableswap invariant pools, as proven by many protocols using this type of invariant (for example CurveFi, RefFI, PancakeSwap).