IOP _ ThunderNFT 34636 - [Smart Contract - Critical] The amount is set to when creating the Executio

Submitted on Sun Aug 18 2024 21:31:54 GMT-0400 (Atlantic Standard Time) by @Schnilch for IOP | ThunderNFT

Report ID: #34636

Report type: Smart Contract

Report severity: Critical

Target: https://github.com/ThunderFuel/smart-contracts/tree/main/contracts-v1/libraries

Impacts:

Permanent freezing of NFTs

Description

Brief/Intro

When an order is executed, the ExecutionResult includes, among other details, how many NFTs the buyer receives. The amount parameter, however, is not taken from the order but is instead set to 1. This could result in only one token being sold while the remaining tokens remain stuck in the contract.

Vulnerability Details

The execution result is generated by the function s1 (see 1. reference) in the strategy with the function execute_order (see 2. reference). As can be seen in the function s1, the amount is simply set to 1 there, and the amount specified in the order is not used. Afterwards, this execution result is returned by the strategy to the Thunder Exchange. The Thunder Exchange uses the execution_result in the functions _execute_sell_taker_order (see 3. reference) and _execute_buy_taker_order (see 4. reference) to send the buyer their tokens based on this result. However, the buyer only receives one token because the amount is always set to 1, but they pay the full price for all tokens offered by the seller. The remaining tokens then get stuck in the contract.

Impact Details

Tokens are stuck in the contract if the seller sells a larger amount of tokens than 1, for example with an ERC1155 token (Normal NFTs should be unique and an amount of 1 would be correct in this case). In addition, the buyer also pays too much because he pays the full price for the actual amount of tokens but only receives one.

References

https://github.com/ThunderFuel/smart-contracts/blob/260c9859e2cd28c188e8f6283469bcf57c9347de/contracts-v1/libraries/src/execution_result.sw#L16-L34
https://github.com/ThunderFuel/smart-contracts/blob/260c9859e2cd28c188e8f6283469bcf57c9347de/contracts-v1/execution_strategies/strategy_fixed_price_sale/src/main.sw#L146
https://github.com/ThunderFuel/smart-contracts/blob/260c9859e2cd28c188e8f6283469bcf57c9347de/contracts-v1/thunder_exchange/src/main.sw#L407-L411
https://github.com/ThunderFuel/smart-contracts/blob/260c9859e2cd28c188e8f6283469bcf57c9347de/contracts-v1/thunder_exchange/src/main.sw#L387-L391

Proof of concept

Proof of Concept

Since the POC is in rust some setup is required.

Execute cargo new thunder-tests in the smart-contracts folder
Add the following dependencies to the Cargo.toml in the thunder-tests folder:

[dev-dependencies]
fuels = { version = "0.62.0", features = ["fuel-core-lib"] }
tokio = { version = "1.12", features = ["rt", "macros"] }

Then paste the following code into thunder-tests/src/main.rs:

use std::str::FromStr;

use fuels::{
    accounts::wallet::Wallet, prelude::*, types::{
        Bits256, ContractId, Identity
    }
};

/////////////////////////////////////Setup/////////////////////////////////////
abigen!(
    Contract(
        name = "AssetManager",
        abi = "/home/schnilch/Fuel/thunder-nft/contracts-v1/asset_manager/out/debug/asset_manager-abi.json"
    ),
    Contract(
        name = "ExecutionManager",
        abi = "/home/schnilch/Fuel/thunder-nft/contracts-v1/execution_manager/out/debug/execution_manager-abi.json"
    ),
    Contract(
        name = "ThunderExchange",
        abi = "/home/schnilch/Fuel/thunder-nft/contracts-v1/thunder_exchange/out/debug/thunder_exchange-abi.json"
    ),
    Contract(
        name = "Pool",
        abi = "/home/schnilch/Fuel/thunder-nft/contracts-v1/pool/out/debug/pool-abi.json"
    ),
    Contract(
        name = "RoyaltyManager",
        abi = "/home/schnilch/Fuel/thunder-nft/contracts-v1/royalty_manager/out/debug/royalty_manager-abi.json"
    ),
    Contract(
        name = "Strategy",
        abi = "/home/schnilch/Fuel/thunder-nft/contracts-v1/execution_strategies/strategy_fixed_price_sale/out/debug/strategy_fixed_price_sale-abi.json"
    ),
    Contract(
        name = "ERC1155",
        abi = "/home/schnilch/Fuel/thunder-nft/contracts-v1/erc1155/out/debug/erc1155-abi.json"
    )
);


pub type Accounts = [WalletUnlocked; 5];

const STRATEGY_FEE: u64 = 50;
const ROYALTY_MANAGER_FEE_LIMIT: u64 = 500;
pub const BASE_ASSET: AssetId = AssetId::new([0u8; 32]);
pub const ERC1155_NFT_STR: &str = "53a2ce8ca7a1cecfd3c9256797d8edce464f4d6deef427cad7b68a32f4340b0c";

pub async fn get_wallets() -> Accounts {
    let mut wallets = launch_custom_provider_and_get_wallets(
        WalletsConfig::new(
            Some(5),             /* Single wallet */
            Some(1),             /* Single coin (UTXO) */
            Some(1_000_000_000), /* Amount per coin */
        ),
        None,
        None,
    )
    .await
    .unwrap();
    let owner = wallets.pop().unwrap();
    let alice = wallets.pop().unwrap();
    let bob = wallets.pop().unwrap();
    let user1 = wallets.pop().unwrap();
    let user2 = wallets.pop().unwrap();

    [owner, alice, bob, user1, user2]
}

//The following functions set up all required contracts and initialize them
pub async fn setup_asset_manager(owner: WalletUnlocked) -> (AssetManager<WalletUnlocked>, ContractId){
    let asset_manager_id = Contract::load_from(
        "/home/schnilch/Fuel/thunder-nft/contracts-v1/asset_manager/out/debug/asset_manager.bin",
        LoadConfiguration::default(),
    )
    .unwrap()
    .deploy(&owner, TxPolicies::default())
    .await
    .unwrap();
 
    let asset_manager = AssetManager::new(asset_manager_id.clone(), owner.clone());

    asset_manager.methods().initialize().call().await.unwrap();
    (asset_manager, asset_manager_id.into())
}

pub async fn setup_execution_manager(owner: WalletUnlocked) -> (ExecutionManager<WalletUnlocked>, ContractId) {
    let execution_manager_id = Contract::load_from(
        "/home/schnilch/Fuel/thunder-nft/contracts-v1/execution_manager/out/debug/execution_manager.bin",
        LoadConfiguration::default(),
    )
    .unwrap()
    .deploy(&owner, TxPolicies::default())
    .await
    .unwrap();
 
    let execution_manager = ExecutionManager::new(execution_manager_id.clone(), owner.clone());

    execution_manager.methods().initialize().call().await.unwrap();
    (execution_manager, execution_manager_id.into())
}

pub async fn setup_thunder_exchange(owner: WalletUnlocked) -> (ThunderExchange<WalletUnlocked>, ContractId){
    let thunder_exchange_id = Contract::load_from(
        "/home/schnilch/Fuel/thunder-nft/contracts-v1/thunder_exchange/out/debug/thunder_exchange.bin",
        LoadConfiguration::default(),
    )
    .unwrap()
    .deploy(&owner, TxPolicies::default())
    .await
    .unwrap();
 
    let thunder_exchange = ThunderExchange::new(thunder_exchange_id.clone(), owner.clone());

    thunder_exchange.methods().initialize().call().await.unwrap();
    (thunder_exchange, thunder_exchange_id.into())
}

pub async fn setup_pool(owner: WalletUnlocked, thunder_exchange: ContractId, asset_manager: ContractId) -> (Pool<WalletUnlocked>, ContractId){
    let pool_id = Contract::load_from(
        "/home/schnilch/Fuel/thunder-nft/contracts-v1/pool/out/debug/pool.bin",
        LoadConfiguration::default(),
    )
    .unwrap()
    .deploy(&owner, TxPolicies::default())
    .await
    .unwrap();
 
    let pool = Pool::new(pool_id.clone(), owner.clone());

    pool.methods().initialize(thunder_exchange, asset_manager).call().await.unwrap();
    (pool, pool_id.into())
}

pub async fn setup_royalty_manager(owner: WalletUnlocked) -> (RoyaltyManager<WalletUnlocked>, ContractId){
    let royalty_manager_id = Contract::load_from(
        "/home/schnilch/Fuel/thunder-nft/contracts-v1/royalty_manager/out/debug/royalty_manager.bin",
        LoadConfiguration::default(),
    )
    .unwrap()
    .deploy(&owner, TxPolicies::default())
    .await
    .unwrap();
 
    let royalty_manager = RoyaltyManager::new(royalty_manager_id.clone(), owner.clone());

    royalty_manager.methods().initialize().call().await.unwrap();
    (royalty_manager, royalty_manager_id.into())
}

pub async fn setup_strategy(owner: WalletUnlocked, thunder_exchange: ContractId) -> (Strategy<WalletUnlocked>, ContractId){
    let strategy_id = Contract::load_from(
        "/home/schnilch/Fuel/thunder-nft/contracts-v1/execution_strategies/strategy_fixed_price_sale/out/debug/strategy_fixed_price_sale.bin",
        LoadConfiguration::default(),
    )
    .unwrap()
    .deploy(&owner, TxPolicies::default())
    .await
    .unwrap();
 
    let strategy = Strategy::new(strategy_id.clone(), owner.clone());

    strategy.methods().initialize(thunder_exchange).call().await.unwrap();
    (strategy, strategy_id.into())
}

pub async fn setup_erc1155(owner: WalletUnlocked, nft_holder: Address) -> (ERC1155<WalletUnlocked>, ContractId) {
    let erc1155_id = Contract::load_from(
        "/home/schnilch/Fuel/thunder-nft/contracts-v1/erc1155/out/debug/erc1155.bin",
        LoadConfiguration::default(),
    )
    .unwrap()
    .deploy(&owner, TxPolicies::default())
    .await
    .unwrap();
 
    let erc1155 = ERC1155::new(erc1155_id.clone(), owner.clone());

    erc1155.methods().constructor(Identity::Address(owner.address().into())).call().await.unwrap();

    (erc1155, erc1155_id.into())
}

pub async fn post_setup(
    strategy: Strategy<WalletUnlocked>, 
    royalty_manager: RoyaltyManager<WalletUnlocked>,
    thunder_exchange: ThunderExchange<WalletUnlocked>,
    execution_manager: ExecutionManager<WalletUnlocked>,
    asset_manager: AssetManager<WalletUnlocked>,
    pool_id: ContractId,
    protocol_fee_recipient: Address,
    nft_holder: Address
) {
    strategy.methods().set_protocol_fee(STRATEGY_FEE).call().await.unwrap();
    royalty_manager.methods().set_royalty_fee_limit(ROYALTY_MANAGER_FEE_LIMIT).call().await.unwrap();
    
    thunder_exchange.methods().set_pool(pool_id).call().await.unwrap();
    thunder_exchange.methods().set_execution_manager(execution_manager.id()).call().await.unwrap();
    thunder_exchange.methods().set_royalty_manager(royalty_manager.id()).call().await.unwrap();
    thunder_exchange.methods().set_asset_manager(asset_manager.id()).call().await.unwrap();
    thunder_exchange.methods().set_protocol_fee_recipient(Identity::Address(protocol_fee_recipient)).call().await.unwrap();

    execution_manager.methods()
        .add_strategy(strategy.id())
        .call()
        .await
        .unwrap();

    asset_manager.methods()
        .add_asset(BASE_ASSET)
        .call()
        .await
        .unwrap();
}

async fn get_contract_instances() -> (
    Accounts,
    (AssetManager<WalletUnlocked>, ContractId), 
    (ExecutionManager<WalletUnlocked>, ContractId),
    (ThunderExchange<WalletUnlocked>, ContractId),
    (Pool<WalletUnlocked>, ContractId),
    (RoyaltyManager<WalletUnlocked>, ContractId),
    (Strategy<WalletUnlocked>, ContractId),
    (ERC1155<WalletUnlocked>, ContractId)
) {
    let accounts = get_wallets().await;
 
    let (asset_manager, asset_manager_id) = setup_asset_manager(accounts[0].clone()).await;
    let (execution_manager, execution_manager_id) = setup_execution_manager(accounts[0].clone()).await;
    let (thunder_exchange, thunder_exchange_id) = setup_thunder_exchange(accounts[0].clone()).await;
    let (pool, pool_id) = setup_pool(accounts[0].clone(), thunder_exchange_id, asset_manager_id).await;
    let (royalty_manager, royalty_manager_id) = setup_royalty_manager(accounts[0].clone()).await;
    let (strategy, strategy_id) = setup_strategy(accounts[0].clone(), thunder_exchange_id).await;
    let (erc1155, erc1155_id) = setup_erc1155(accounts[0].clone(), accounts[1].address().into()).await;

    post_setup(
        strategy.clone(),
        royalty_manager.clone(),
        thunder_exchange.clone(),
        execution_manager.clone(),
        asset_manager.clone(),
        pool_id,
        accounts[4].address().into(),
        accounts[1].address().into()
    ).await;
 
    (
        accounts,
        (asset_manager, asset_manager_id), 
        (execution_manager, execution_manager_id),
        (thunder_exchange, thunder_exchange_id),
        (pool, pool_id),
        (royalty_manager, royalty_manager_id),
        (strategy, strategy_id),
        (erc1155, erc1155_id)
    )
}
/////////////////////////////////////Setup End/////////////////////////////////////

/////////////////////////////////////POC/////////////////////////////////////
#[tokio::test]
async fn test_poc() {
    let (
        [_owner, alice, bob, _protocol_fee_recipient, _user],
        (_asset_manager, asset_manager_id), 
        (_execution_manager, execution_manager_id),
        (thunder_exchange, _thunder_exchange_id),
        (_pool, pool_id),
        (_royalty_manager, royalty_manager_id),
        (strategy, strategy_id),
        (erc1155, erc1155_id)
    ) = get_contract_instances().await;
    let ERC1155_NFT = AssetId::from_str(ERC1155_NFT_STR).unwrap();
    let thunder_exchange_alice = thunder_exchange.clone().with_account(alice.clone());
    let thunder_exchange_bob = thunder_exchange.clone().with_account(bob.clone());

    erc1155.methods() //10 ERC1155 tokens are minted for alice so she can sell them
        .mint(alice.address().into(), Bits256::zeroed(), 10)
        .append_variable_outputs(1)
        .call()
        .await
        .unwrap();

    //The order for alice so that she can sell her 10 tokens for a price of 800
    let maker_order_input = MakerOrderInput {
        maker: alice.address().into(),
        collection: erc1155_id.clone(),
        price: 800,
        amount: 10,
        nonce: 1,
        strategy: strategy_id,
        payment_asset: BASE_ASSET,
        expiration_range: 10000,
        token_id: Bits256::zeroed(),
        side: Side::Sell,
        extra_params: ExtraParams {
            extra_address_param: Address::zeroed(),
            extra_contract_param: ContractId::zeroed(),
            extra_u_64_param: 0
        }
    };
    
    //Alice places the sell order created above on the thunder exchange and sends the 10 ERC1155 tokens which are then in the contract
    thunder_exchange_alice.methods()
        .place_order(maker_order_input)
        .call_params(CallParameters::new(10, ERC1155_NFT, 1_000_000))
        .unwrap()
        .append_contract(execution_manager_id.into())
        .append_contract(strategy_id.into())
        .append_contract(pool_id.into())
        .append_contract(asset_manager_id.into())
        .call()
        .await
        .unwrap();
    println!("------------Alice sells 10 ERC1155 tokens--------------");
    println!("alice sell order: {:#?}", strategy.methods().get_maker_order_of_user(alice.address(), 1, Side::Sell).call().await.unwrap().value);
    
    //This is the taker order from bob with which he buys the 10 erc1155 tokens for 800.
    let taker_order = TakerOrder {
        side: Side::Buy,
        taker: bob.address().into(),
        maker: alice.address().into(),
        nonce: 1,
        price: 800,
        token_id: Bits256::zeroed(),
        collection: erc1155_id.clone(),
        strategy: strategy_id.clone(),
        extra_params: ExtraParams {
            extra_address_param: Address::zeroed(),
            extra_contract_param: ContractId::zeroed(),
            extra_u_64_param: 0
        }
    };

    //Bob executed the orders and sends his 800 tokens but he will only get one of the 1 of the 10 ERC1155 NFTs
    thunder_exchange_bob.methods()
        .execute_order(taker_order)
        .call_params(CallParameters::new(800, BASE_ASSET, 1_000_000))
        .unwrap()
        .append_contract(execution_manager_id.into())
        .append_contract(strategy_id.into())
        .append_contract(pool_id.into())
        .append_contract(asset_manager_id.into())
        .append_contract(royalty_manager_id.into())
        .append_variable_outputs(3)
        .call()
        .await
        .unwrap();

    //This shows that Bob received only one NFT and paid the full price. The other NFTs are now stuck in Thunder Exchange because the order is no longer there
    println!("bob balance ERC1155 NFT: {:#?}", bob.get_asset_balance(&ERC1155_NFT).await.unwrap());
    println!("bob balance BASE_ASSET: {:#?}", bob.get_asset_balance(&BASE_ASSET).await.unwrap()); //Bob had 1000000000 at the beginning, now he should only have 999999200
    println!("order (not existent): {:#?}", strategy.methods() //shows that the order was really deleted
        .get_maker_order_of_user(alice.address(), 1, Side::Sell)
        .call()
        .await
        .unwrap()
        .value
    );
}

Since normal NFTs only have an amount of 1 per NFT, an ERC1155 token will be used in this example. For this one must be created in the contracts-v1 folder:

Execute force new erc1155 in the contracts-v1 folder
Add these dependency to the Forc.toml of the erc1155 contract:

standards = { git = "https://github.com/FuelLabs/sway-standards", tag = "v0.4.3" }

Paste this code into contracts-v1/erc1155/src/main.sw (The code was mainly copied from here: https://docs.fuel.network/docs/sway/blockchain-development/native_assets/#multi-native-asset-example):

contract;
 
use standards::src5::{SRC5, State};
use standards::src20::SRC20;
use standards::src3::SRC3;
use std::{
    asset::{
        burn,
        mint_to,
    },
    call_frames::{
        msg_asset_id,
    },
    hash::{
        Hash,
    },
    context::this_balance,
    storage::storage_string::*,
    string::String
};
 
storage {
    total_assets: u64 = 0,
    total_supply: StorageMap<AssetId, u64> = StorageMap {},
    name: StorageMap<AssetId, StorageString> = StorageMap {},
    symbol: StorageMap<AssetId, StorageString> = StorageMap {},
    decimals: StorageMap<AssetId, u8> = StorageMap {},
    owner: State = State::Uninitialized,
}
 
// Native Asset Standard
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> {
        storage.name.get(asset).read_slice()
    }
    
    #[storage(read)]
    fn symbol(asset: AssetId) -> Option<String> {
        storage.symbol.get(asset).read_slice()
    }
 
    #[storage(read)]
    fn decimals(asset: AssetId) -> Option<u8> {
        storage.decimals.get(asset).try_read()
    }
}
 
// Mint and Burn Standard
impl SRC3 for Contract {
    #[storage(read, write)]
    fn mint(recipient: Identity, sub_id: SubId, amount: u64) {
        require_access_owner();
        let asset_id = AssetId::new(ContractId::this(), sub_id);
        log(asset_id);
        let supply = storage.total_supply.get(asset_id).try_read();
        if supply.is_none() {
            storage.total_assets.write(storage.total_assets.try_read().unwrap_or(0) + 1);
        }
        let current_supply = supply.unwrap_or(0);
        storage.total_supply.insert(asset_id, current_supply + amount);
        mint_to(recipient, sub_id, amount);
    }
    
    #[storage(read, write), payable]
    fn burn(sub_id: SubId, amount: u64) {
        require_access_owner();
        let asset_id = AssetId::new(ContractId::this(), sub_id);
        require(this_balance(asset_id) >= amount, "not-enough-coins");
        
        let supply = storage.total_supply.get(asset_id).try_read();
        let current_supply = supply.unwrap_or(0);
        storage.total_supply.insert(asset_id, current_supply - amount);
        burn(sub_id, amount);
    }
}
 
abi MultiAsset {
    #[storage(read, write)]
    fn constructor(owner_: Identity);
    
    #[storage(read, write)]
    fn set_name(asset: AssetId, name: String);
 
    #[storage(read, write)]
    fn set_symbol(asset: AssetId, symbol: String);
 
    #[storage(read, write)]
    fn set_decimals(asset: AssetId, decimals: u8);
}
 
impl MultiAsset for Contract {
    #[storage(read, write)]
    fn constructor(owner_: Identity) {
        require(storage.owner.read() == State::Uninitialized, "owner-initialized");
        storage.owner.write(State::Initialized(owner_));
    }
    
    #[storage(read, write)]
    fn set_name(asset: AssetId, name: String) {
        require_access_owner();
        storage.name.insert(asset, StorageString {});
        storage.name.get(asset).write_slice(name);
    }
 
    #[storage(read, write)]
    fn set_symbol(asset: AssetId, symbol: String) {
        require_access_owner();
        storage.symbol.insert(asset, StorageString {});
        storage.symbol.get(asset).write_slice(symbol);
    }
 
    #[storage(read, write)]
    fn set_decimals(asset: AssetId, decimals: u8) {
        require_access_owner();
        storage.decimals.insert(asset, decimals);
    }
}
 
#[storage(read)]
fn require_access_owner() {
    require(
        storage.owner.read() == State::Initialized(msg_sender().unwrap()),
        "not owner",
    );
}

Run forc build in all smart contract folders that are needed:
- contracts-v1/asset_manager
- contracts-v1/erc1155
- contracts-v1/execution_manager
- contracts-v1/execution_strategies/strategy_fixed_price_sale
- contracts-v1/interfaces
- contracts-v1/libraries
- contracts-v1/pool
- contracts-v1/royalty_manager
- contracts-v1/thunder_exchange
The POC can be started with the command cargo test test_poc -- --nocapture in the thunder-tests folder

PreviousIOP _ ThunderNFT 34630 - [Smart Contract - Critical] Incorrect Token Sale Amount NextIOP _ ThunderNFT 34642 - [Smart Contract - High] strategy de-listing causes sellers NFTs locked on T

Last updated 8 months ago

Was this helpful?