From bdeac71844e6e7a607eecb54e5ae5cf01f6dbb7c Mon Sep 17 00:00:00 2001 From: mootz12 Date: Tue, 27 Feb 2024 15:15:06 -0500 Subject: [PATCH] backstop: minimize inflation attack exploit (#196) --- backstop/src/backstop/deposit.rs | 69 ++++++++++- backstop/src/backstop/pool.rs | 4 - backstop/src/backstop/withdrawal.rs | 64 ++++++++++- backstop/src/constants.rs | 3 + backstop/src/errors.rs | 2 + .../tests/test_backstop_inflation_attack.rs | 107 ++++++++++++++++++ .../tests/test_pool_inflation_attack.rs | 2 +- 7 files changed, 242 insertions(+), 9 deletions(-) create mode 100644 test-suites/tests/test_backstop_inflation_attack.rs diff --git a/backstop/src/backstop/deposit.rs b/backstop/src/backstop/deposit.rs index b5d7423b..33067042 100644 --- a/backstop/src/backstop/deposit.rs +++ b/backstop/src/backstop/deposit.rs @@ -1,4 +1,6 @@ -use crate::{contract::require_nonnegative, emissions, storage, BackstopError}; +use crate::{ + constants::MIN_INITIAL_SHARES, contract::require_nonnegative, emissions, storage, BackstopError, +}; use sep_41_token::TokenClient; use soroban_sdk::{panic_with_error, Address, Env}; @@ -20,6 +22,9 @@ pub fn execute_deposit(e: &Env, from: &Address, pool_address: &Address, amount: backstop_token_client.transfer(from, &e.current_contract_address(), &amount); let to_mint = pool_balance.convert_to_shares(amount); + if to_mint == 0 || (pool_balance.shares == 0 && to_mint < MIN_INITIAL_SHARES) { + panic_with_error!(e, &BackstopError::InvalidShareMintAmount); + } pool_balance.deposit(amount, to_mint); user_balance.add_shares(to_mint); @@ -35,6 +40,7 @@ mod tests { use crate::{ backstop::execute_donate, + constants::SCALAR_7, testutils::{create_backstop, create_backstop_token, create_mock_pool_factory}, }; @@ -208,4 +214,65 @@ mod tests { execute_deposit(&e, &samwise, &pool_0_id, 100); }); } + + #[test] + #[should_panic(expected = "Error(Contract, #1005)")] + fn test_execute_deposit_zero_share_mint() { + let e = Env::default(); + e.budget().reset_unlimited(); + e.mock_all_auths_allowing_non_root_auth(); + + let backstop_address = create_backstop(&e); + let bombadil = Address::generate(&e); + let samwise = Address::generate(&e); + let frodo = Address::generate(&e); + let pool_0_id = Address::generate(&e); + let pool_1_id = Address::generate(&e); + + let (_, backstop_token_client) = create_backstop_token(&e, &backstop_address, &bombadil); + backstop_token_client.mint(&samwise, &100_0000000); + backstop_token_client.mint(&frodo, &100_000_000_0000000); + + let (_, mock_pool_factory_client) = create_mock_pool_factory(&e, &backstop_address); + mock_pool_factory_client.set_pool(&pool_0_id); + mock_pool_factory_client.set_pool(&pool_1_id); + + // initialize pool 0 with funds + some profit + e.as_contract(&backstop_address, || { + execute_deposit(&e, &frodo, &pool_0_id, SCALAR_7); + execute_donate(&e, &frodo, &pool_0_id, 10_000_000 * SCALAR_7); + }); + + e.as_contract(&backstop_address, || { + execute_deposit(&e, &samwise, &pool_0_id, SCALAR_7); + }); + } + + #[test] + #[should_panic(expected = "Error(Contract, #1005)")] + fn test_execute_deposit_small_initial_mint() { + let e = Env::default(); + e.budget().reset_unlimited(); + e.mock_all_auths_allowing_non_root_auth(); + + let backstop_address = create_backstop(&e); + let bombadil = Address::generate(&e); + let samwise = Address::generate(&e); + let frodo = Address::generate(&e); + let pool_0_id = Address::generate(&e); + let pool_1_id = Address::generate(&e); + + let (_, backstop_token_client) = create_backstop_token(&e, &backstop_address, &bombadil); + backstop_token_client.mint(&samwise, &100_0000000); + backstop_token_client.mint(&frodo, &100_0000000); + + let (_, mock_pool_factory_client) = create_mock_pool_factory(&e, &backstop_address); + mock_pool_factory_client.set_pool(&pool_0_id); + mock_pool_factory_client.set_pool(&pool_1_id); + + e.as_contract(&backstop_address, || { + execute_donate(&e, &frodo, &pool_0_id, SCALAR_7); + execute_deposit(&e, &samwise, &pool_0_id, SCALAR_7 / 10 - 1); + }); + } } diff --git a/backstop/src/backstop/pool.rs b/backstop/src/backstop/pool.rs index a6abd99b..e54cebc0 100644 --- a/backstop/src/backstop/pool.rs +++ b/backstop/src/backstop/pool.rs @@ -57,8 +57,6 @@ pub fn require_is_from_pool_factory(e: &Env, address: &Address, balance: i128) { } } -/// TODO: Duplicated from pool/pool/status.rs. Can this be moved to a common location? -/// /// Calculate the threshold for the pool's backstop balance /// /// Returns true if the pool's backstop balance is above the threshold @@ -134,8 +132,6 @@ impl PoolBalance { /// Deposit tokens and shares into the pool /// - /// If this is the first time - /// /// ### Arguments /// * `tokens` - The amount of tokens to add /// * `shares` - The amount of shares to add diff --git a/backstop/src/backstop/withdrawal.rs b/backstop/src/backstop/withdrawal.rs index 29e4ee4d..4bf739f0 100644 --- a/backstop/src/backstop/withdrawal.rs +++ b/backstop/src/backstop/withdrawal.rs @@ -1,6 +1,6 @@ -use crate::{contract::require_nonnegative, emissions, storage}; +use crate::{contract::require_nonnegative, emissions, storage, BackstopError}; use sep_41_token::TokenClient; -use soroban_sdk::{unwrap::UnwrapOptimized, Address, Env}; +use soroban_sdk::{panic_with_error, unwrap::UnwrapOptimized, Address, Env}; use super::Q4W; @@ -56,6 +56,9 @@ pub fn execute_withdraw(e: &Env, from: &Address, pool_address: &Address, amount: user_balance.dequeue_shares_for_withdrawal(e, amount, true); let to_return = pool_balance.convert_to_tokens(amount); + if to_return == 0 { + panic_with_error!(e, &BackstopError::InvalidTokenWithdrawAmount); + } pool_balance.withdraw(e, to_return, amount); storage::set_user_balance(e, pool_address, from, &user_balance); @@ -75,7 +78,7 @@ mod tests { }; use crate::{ - backstop::{execute_deposit, execute_donate}, + backstop::{execute_deposit, execute_donate, execute_draw}, testutils::{ assert_eq_vec_q4w, create_backstop, create_backstop_token, create_mock_pool_factory, }, @@ -363,6 +366,7 @@ mod tests { assert_eq!(backstop_token_client.balance(&samwise), tokens); }); } + #[test] #[should_panic(expected = "Error(Contract, #8)")] fn test_execute_withdrawal_negative_amount() { @@ -413,4 +417,58 @@ mod tests { execute_withdraw(&e, &samwise, &pool_address, -42_0000000); }); } + + #[test] + #[should_panic(expected = "Error(Contract, #1006)")] + fn test_execute_withdrawal_zero_tokens() { + let e = Env::default(); + e.mock_all_auths_allowing_non_root_auth(); + + let backstop_address = create_backstop(&e); + let pool_address = Address::generate(&e); + let bombadil = Address::generate(&e); + let samwise = Address::generate(&e); + let frodo = Address::generate(&e); + + let (_, backstop_token_client) = create_backstop_token(&e, &backstop_address, &bombadil); + backstop_token_client.mint(&samwise, &150_0000000); + backstop_token_client.mint(&frodo, &150_0000000); + + let (_, mock_pool_factory_client) = create_mock_pool_factory(&e, &backstop_address); + mock_pool_factory_client.set_pool(&pool_address); + + e.ledger().set(LedgerInfo { + protocol_version: 20, + sequence_number: 200, + timestamp: 10000, + network_id: Default::default(), + base_reserve: 10, + min_temp_entry_ttl: 10, + min_persistent_entry_ttl: 10, + max_entry_ttl: 2000000, + }); + + // setup pool with queue for withdrawal and allow the backstop to incur a profit + e.as_contract(&backstop_address, || { + execute_deposit(&e, &frodo, &pool_address, 1_0000001); + execute_deposit(&e, &samwise, &pool_address, 1_0000000); + execute_queue_withdrawal(&e, &samwise, &pool_address, 1_0000000); + execute_draw(&e, &pool_address, 1_9999999, &frodo); + }); + + e.ledger().set(LedgerInfo { + protocol_version: 20, + sequence_number: 200, + timestamp: 10000 + 21 * 24 * 60 * 60 + 1, + network_id: Default::default(), + base_reserve: 10, + min_temp_entry_ttl: 10, + min_persistent_entry_ttl: 10, + max_entry_ttl: 2000000, + }); + + e.as_contract(&backstop_address, || { + execute_withdraw(&e, &samwise, &pool_address, 1_0000000); + }); + } } diff --git a/backstop/src/constants.rs b/backstop/src/constants.rs index 26e7f9ce..7c7b77f5 100644 --- a/backstop/src/constants.rs +++ b/backstop/src/constants.rs @@ -1,5 +1,8 @@ /// Fixed-point scalar for 7 decimal numbers pub const SCALAR_7: i128 = 1_0000000; +/// The minimum number of shares that can be created +pub const MIN_INITIAL_SHARES: i128 = SCALAR_7 / 10; + // The approximate deployment date of the backstop module TODO: pick one pub const BACKSTOP_EPOCH: u64 = 1441065600; diff --git a/backstop/src/errors.rs b/backstop/src/errors.rs index 8eaec76e..e5d3966c 100644 --- a/backstop/src/errors.rs +++ b/backstop/src/errors.rs @@ -22,4 +22,6 @@ pub enum BackstopError { InvalidRewardZoneEntry = 1002, InsufficientFunds = 1003, NotPool = 1004, + InvalidShareMintAmount = 1005, + InvalidTokenWithdrawAmount = 1006, } diff --git a/test-suites/tests/test_backstop_inflation_attack.rs b/test-suites/tests/test_backstop_inflation_attack.rs new file mode 100644 index 00000000..56ae1c0c --- /dev/null +++ b/test-suites/tests/test_backstop_inflation_attack.rs @@ -0,0 +1,107 @@ +#![cfg(test)] + +use soroban_sdk::{testutils::Address as _, vec, Address, Error, Symbol}; +use test_suites::{ + pool::default_reserve_metadata, + test_fixture::{TestFixture, TokenIndex, SCALAR_7}, +}; + +// This test showcases an inflation attack during a backstops initialization stage. This is a +// high risk inflation attack for the attacker, and requires significant capital to execute. +// If the donation does not happen before the victim deposits, the attacker will lose the majority +// of the capital they donated to the backstop. +// +// However, since the attack is feasible, this test exists to ensure it's effects are minimized +// and common pitfalls are avoided. +#[test] +fn test_backstop_inflation_attack() { + let mut fixture = TestFixture::create(false); + + let whale = Address::generate(&fixture.env); + let sauron = Address::generate(&fixture.env); + let pippen = Address::generate(&fixture.env); + + // create pool with 1 new reserve + fixture.create_pool(Symbol::new(&fixture.env, "Teapot"), 0, 6); + + let xlm_config = default_reserve_metadata(); + fixture.create_pool_reserve(0, TokenIndex::XLM, &xlm_config); + let pool_address = fixture.pools[0].pool.address.clone(); + + // setup backstop and update pool status + fixture.tokens[TokenIndex::BLND].mint(&whale, &(5_001_000 * SCALAR_7)); + fixture.tokens[TokenIndex::USDC].mint(&whale, &(121_000 * SCALAR_7)); + fixture.lp.join_pool( + &(400_000 * SCALAR_7), + &vec![&fixture.env, 5_001_000 * SCALAR_7, 121_000 * SCALAR_7], + &whale, + ); + + // execute inflation attack against pippen + let starting_balance = 200_000 * SCALAR_7; + fixture.lp.transfer(&whale, &sauron, &starting_balance); + fixture.lp.transfer(&whale, &pippen, &starting_balance); + + // 1. Attacker deposits a small amount as the initial depositor + + // contracts stop very small initial deposits + let bad_init_deposit = + fixture + .backstop + .try_deposit(&sauron, &pool_address, &(SCALAR_7 / 10 - 1)); + assert_eq!( + bad_init_deposit.err(), + Some(Ok(Error::from_contract_error(1005))) + ); + + let sauron_deposit_amount = SCALAR_7 / 10; + let sauron_shares = fixture + .backstop + .deposit(&sauron, &pool_address, &sauron_deposit_amount); + + // 2. Attacker donates a large amount to the backstop before the victim can perform a deposit + let inflation_amount = 100_000 * SCALAR_7; + fixture + .backstop + .donate(&sauron, &pool_address, &inflation_amount); + + // contracts stop any zero share deposits + let mut deposit_amount = 1000; + let bad_deposit_result = fixture + .backstop + .try_deposit(&pippen, &pool_address, &deposit_amount); + assert_eq!( + bad_deposit_result.err(), + Some(Ok(Error::from_contract_error(1005))) + ); + + // user can still be in a situation where they get adversely affected by the inflation attacks + // but to a small extent + deposit_amount = SCALAR_7; + let pippen_shares = fixture + .backstop + .deposit(&pippen, &pool_address, &deposit_amount); + assert_eq!(pippen_shares, 9); // actual is 9.99... + + // 3. Attacker and victim withdraw funds + fixture + .backstop + .queue_withdrawal(&sauron, &pool_address, &sauron_shares); + fixture + .backstop + .queue_withdrawal(&pippen, &pool_address, &pippen_shares); + + // wait enough time so all shares can be withdrawn + fixture.jump(21 * 24 * 60 * 60 + 1); + + fixture + .backstop + .withdraw(&sauron, &pool_address, &sauron_shares); + fixture + .backstop + .withdraw(&pippen, &pool_address, &pippen_shares); + + // pippen loses less than 10% of initial deposit due to rounding + assert!(fixture.lp.balance(&sauron) < starting_balance + SCALAR_7 / 10); + assert!(fixture.lp.balance(&pippen) > starting_balance - SCALAR_7 / 10); +} diff --git a/test-suites/tests/test_pool_inflation_attack.rs b/test-suites/tests/test_pool_inflation_attack.rs index 9d7a1553..efa2a142 100644 --- a/test-suites/tests/test_pool_inflation_attack.rs +++ b/test-suites/tests/test_pool_inflation_attack.rs @@ -58,7 +58,7 @@ fn test_pool_inflation_attack() { fixture.jump_with_sequence(5); // 2. Attacker frontruns victim's deposit by depositing a large amount of underlying - // to force the victim's minted shares to be zero. + // to try and force an error in minting B tokens let inflation_amount = 100 * SCALAR_7; fixture.tokens[TokenIndex::XLM].transfer( &sauron,