Open c4-bot-10 opened 6 months ago
Consider QA, worst impact is temporally DoS. Leaving for sponsor review.
0xRobocop marked the issue as primary issue
0xRobocop marked the issue as sufficient quality report
enthusiastmartin (sponsor) disputed
Although the rounding issue migh be correct, it has minimal impact on protocol.
But the DoS is incorrect, Stableswap is not "protected" by circuit breaker.
Rounding down (i.e. in favor of the user) in this scenario might not be ideal conceptually, but the impact is very low in practice. With 12 decimals, we are talking about differences of roughly 10^-12, which is (way) smaller than the transaction fee of ~1.6 HDX. Still a good QA recommendation
OpenCoreCH changed the severity to QA (Quality Assurance)
OpenCoreCH marked the issue as grade-a
Lines of code
https://github.com/code-423n4/2024-02-hydradx/blob/603187123a20e0cb8a7ea85c6a6d718429caad8d/HydraDX-node/math/src/stableswap/math.rs#L92
Vulnerability details
Bug Description
Step 1: Circumvent Fee
The Stableswap AMM imposes a Poolfee on each trade. This fee can be set by the Authority in the range of $0\% <= fee <= 100\%$. The fee is imposed on each trade, and in case of a call to
sell()
, will be deducted from the amount going out. In the case of a buy, it will be deducted from the amount going in.For this issue, we will take a closer look at the functionality of the
sell()
function. When a sell call happens, the output is calculated by thecalculate_out_given_in_with_fee()
function.As one can see, this calls the
calculate_fee_amount()
with rounding down as the rounding direction.Unfortunately, this leads to the issue that low amounts being swapped will lead to the fee rounding down to 0, resulting in no fee being paid on the swap. Even for bigger swaps, the fee is rounded down on the call to sell() while it is rounded up on the call to buy(). This makes the sell() function overall cheaper to call for any swap where rounding of the fee will occur.
So a malicious user can split one big swap into many smaller swaps, to circumvent the protocol fee and get a cheaper swap. The maximum swap amount for this to work will be:
$$amountToWithdraw < 1/feePercentage$$
For a fee of 0.01%, this would for example be:
$$amountToWithdraw < 1/0.0001$$
$$amountToWithdraw < 10000$$
Step2: DOS Swaps
The HydraDx protocol also implements a circuit breaker that will halt trading and liquidity transfers if a preset value of transfers is crossed in a single block. To ensure that users can not artificially inflate the value of total swaps in a block, a fee is imposed. Unfortunately, as seen above, users can circumvent this fee. So a user can swap small amounts often enough without incurring a fee (besides the gas fee) and artificially inflate the total swaps. As soon as the threshold is passed, all other swaps will be DOSd.
To understand the viability of this attack path one also needs to understand the core functionality of the StableSwap. On normal AMMs A user would still lose funds on each swap, as the price is newly calculated each time. For StableSwap AMMs, as long as the reserves don't differentiate too much, the exchange rate stays constant at 1:1 for the tokens. So the users only "loss" on the swaps would be the fee, which can be circumvented.
Impact
The issue allows a malicious user to bundle his swap into multiple smaller swaps to circumvent the pool fee. This in turn allows the attacker to artificially trigger the circuit breaker.
Proof of Concept
The following testcase shows an example where the attacker is able to curcumvent the fee:
The test can be run by adding it to the
pallets/stableswap/src/tests/trades.rs
file.Tools Used
Manual Review
Recommended Mitigation Steps
The issue can be mitigated by changing the rounding direction so that the calculation always rounds up on the fee.
Assessed type
DoS