One Shot: Reloaded

First Flight #48
Beginner FriendlyNFT
100 EXP
View results
Submission Details
Impact: medium
Likelihood: high
Invalid

A player can exit a battle using two Rapper tokens

rap_battle::go_on_stage_or_battle() does not check whether the defender and challenger are the same player + A player can exit a battle using two Rapper tokens

Description

  • The defender and the challenger should be two different players.

  • rap_battle.move does not provide a function to allow the defender to exit a battle.

  • rap_battle::go_on_stage_or_battle() does not check whether the defender and challenger are the same player.

  • A player can exit a battle using two Rapper by:

    • Entering a battle as a defender using their first Rapper token and with for example n CRED coins.

    • Then entering a battle as a challenger using their second Rapper token with a matching n CRED coins.

    • The player must have at least 2n CRED coins for this exploit.

public entry fun go_on_stage_or_battle(
player: &signer,
rapper_token: Object<Token>,
bet_amount: u64
) acquires BattleArena {
let player_addr = signer::address_of(player);
let arena = borrow_global_mut<BattleArena>(@battle_addr);
if (arena.defender == @0x0) {
assert!(arena.defender_bet == 0, E_BATTLE_ARENA_OCCUPIED);
arena.defender = player_addr;
arena.defender_bet = bet_amount;
let token_id = object::object_address(&rapper_token);
arena.defender_token_id = token_id;
let first_bet = coin::withdraw<cred_token::CRED>(player, bet_amount);
coin::merge(&mut arena.prize_pool, first_bet);
one_shot::transfer_record_only(token_id, player_addr, @battle_addr);
object::transfer(player, rapper_token, @battle_addr);
event::emit(OnStage {
defender: player_addr,
token_id,
cred_bet: bet_amount,
});
} else {
@> assert!(arena.defender_bet == bet_amount, E_BETS_DO_NOT_MATCH);
let defender_addr = arena.defender;
let chall_addr = player_addr;
let chall_token_id = object::object_address(&rapper_token);
one_shot::transfer_record_only(chall_token_id, chall_addr, @battle_addr);
object::transfer(player, rapper_token, @battle_addr);
let chall_coins = coin::withdraw<cred_token::CRED>(player, bet_amount);
coin::merge(&mut arena.prize_pool, chall_coins);
let defender_skill = one_shot::skill_of(arena.defender_token_id);
let challenger_skill = one_shot::skill_of(chall_token_id);
let total_skill = defender_skill + challenger_skill;
let rnd = timestamp::now_seconds() % (if (total_skill == 0) { 1 } else { total_skill });
let winner = if (rnd < defender_skill) { defender_addr } else { chall_addr };
event::emit(Battle {
challenger: chall_addr,
challenger_token_id: chall_token_id,
winner,
});
let pool = coin::extract_all(&mut arena.prize_pool);
if (winner == defender_addr) {
coin::deposit(defender_addr, pool);
one_shot::increment_wins(arena.defender_token_id);
one_shot::transfer_record_only(arena.defender_token_id, @battle_addr, defender_addr);
one_shot::transfer_record_only(chall_token_id, @battle_addr, defender_addr);
} else {
coin::deposit(chall_addr, pool);
one_shot::increment_wins(chall_token_id);
one_shot::transfer_record_only(arena.defender_token_id, @battle_addr, chall_addr);
one_shot::transfer_record_only(chall_token_id, @battle_addr, chall_addr);
};
arena.defender = @0x0;
arena.defender_bet = 0;
arena.defender_token_id = @0x0;
}
}

Risk

Likelihood:

  • Player1 is lucky enough to be minted two Rapper tokens by the module owner.

  • Player1 stakes at least one of their Rapper tokens to have at least 2 CRED coins.

  • Player1 enters the battle as a defender using their first Rapper token with 1 CRED coin.

  • Player1 enters the battle as a challenger using their second Rapper token with 1 CRED coin.

  • Player1 gets back both of their Rapper tokens and their 2 CRED coins.

Impact:

  • Player1 is able to exit their battle as a defender.

Proof of Concept

Add this test_init_module() wrapper function to cred_token.move so that we can call the init_module() function from our unit tests.

#[test_only]
public fun test_init_module(sender: &signer) {
init_module(sender);
}

Add the functions mint_rapper_fixed() and get_token_ids() to one_shot.move so that players can retrieve their rapper token_id for unit testing.

#[test_only]
public entry fun mint_rapper_fixed(module_owner: &signer, to: &signer)
acquires Collection, RapperStats, TokenIDs {
let owner_addr = signer::address_of(module_owner);
assert!(owner_addr == @battle_addr, 1 /* E_NOT_AUTHORIZED */);
// 🔧 Lazy-init if needed (unit tests don’t auto-run init_module)
if (!exists<Collection>(@battle_addr)) {
init_module(module_owner);
};
if (!exists<RapperStats>(@battle_addr)) {
// This case shouldn’t happen if init_module just ran, but keep it safe.
let stats_table = table::new<address, StatsData>();
let owner_table = table::new<address, u64>();
move_to(module_owner, RapperStats { stats: stats_table, owner_counts: owner_table });
};
// Safe to assume collection/stats exist now
let _ = borrow_global<Collection>(@battle_addr);
let tok_ref = token::create(
module_owner,
string::utf8(COLLECTION_NAME),
string::utf8(b"A new rapper enters the scene."),
string::utf8(b"Rapper"),
option::none(),
string::utf8(b""),
);
let token_obj = object::object_from_constructor_ref<token::Token>(&tok_ref);
let token_id = object::address_from_constructor_ref(&tok_ref);
let to_address = signer::address_of(to);
let stats_res = borrow_global_mut<RapperStats>(@battle_addr);
table::add(&mut stats_res.stats, token_id, StatsData {
owner: to_address,
weak_knees: true,
heavy_arms: true,
spaghetti_sweater: true,
calm_and_ready: false,
battles_won: 0,
});
// increment owner count
if (table::contains(&stats_res.owner_counts, to_address)) {
let cnt = table::borrow_mut(&mut stats_res.owner_counts, to_address);
*cnt = *cnt + 1;
} else {
table::add(&mut stats_res.owner_counts, to_address, 1);
};
event::emit(MintRapperEvent { minter: to_address, token_id });
object::transfer(module_owner, token_obj, to_address);
if (exists<TokenIDs>(to_address)) {
// TokenIDs already exists, perform upddate
let token_ids = borrow_global_mut<TokenIDs>(to_address);
vector::push_back(&mut token_ids.ids, token_id);
} else {
// first publish
let v = vector::empty<address>();
vector::push_back(&mut v, token_id);
let token_ids = TokenIDs{
ids: v
};
move_to(to,token_ids);
};
}
#[test_only]
#[view]
public fun get_token_ids(player_address: address): vector<address> acquires TokenIDs {
assert!(exists<TokenIDs>(player_address), E_NO_TOKEN_IDS);
let token_ids = borrow_global<TokenIDs>(player_address);
token_ids.ids
}

Add this test_init_module() wrapper function to rap_battle.move so that we can call the init_module() function from our unit tests.

#[test_only]
public fun test_init_module(sender: &signer) {
init_module(sender);
}

Create an additional tests/rap_battle_bug_exit_battle.movefile containing:

#[test_only]
module battle_addr::rap_battle_bug_exit_battle {
use std::signer;
use aptos_framework::account;
//use aptos_framework::object::{Self as object, Object};
use aptos_framework::object;
use battle_addr::cred_token::{Self as cred_token};
use battle_addr::one_shot::{Self as one_shot};
use battle_addr::streets::{Self as streets};
use battle_addr::rap_battle::{Self as rap_battle};
use aptos_framework::timestamp;
use aptos_token_v2::token::{Self as token};
use std::string;
use std::debug;
use std::vector;
use aptos_framework::coin;
fun p<T>(s: &T) {
debug::print(s);
}
fun p_vector_u8(s: vector<u8>) {
p(&string::utf8(s));
}
fun p_vector_address(s: &vector<address>) {
debug::print(s);
}
const SECS_PER_DAY: u64 = 86400;
#[test(player1 = @0x100, framework = @0x1)]
public fun test_rap_battle_exploit_randomness (
player1: &signer,
framework: &signer
) {
let (burn_cap, mint_cap) = aptos_framework::aptos_coin::initialize_for_test(framework);
timestamp::set_time_has_started_for_testing(framework);
let player1_address = signer::address_of(player1);
let module_owner = account::create_account_for_test(@battle_addr);
account::create_account_for_test(player1_address);
let player1_address = signer::address_of(player1);
cred_token::test_init_module(&module_owner);
rap_battle::test_init_module(&module_owner);
// mint two rapper tokens for player1
one_shot::mint_rapper_fixed(&module_owner, player1);
one_shot::mint_rapper_fixed(&module_owner, player1);
let balance1 = one_shot::balance_of(player1_address);
p_vector_u8(b"balance1");
p(&balance1);
assert!(balance1 == 2, 1);
let player1_rapper_ids = one_shot::get_token_ids(player1_address);
p_vector_u8(b"player1_rapper_ids");
p_vector_address(&player1_rapper_ids);
let player1_token_id0 = vector::borrow(&player1_rapper_ids, 0);
p_vector_u8(b"player1_token_id0");
p(player1_token_id0);
let player1_token_object0 =
object::address_to_object<token::Token>(*player1_token_id0);
p_vector_u8(b"player1_token_object0");
p(&player1_token_object0);
let player1_token_id1 = vector::borrow(&player1_rapper_ids, 1);
p_vector_u8(b"player1_token_id1");
p(player1_token_id1);
let player1_token_object1 =
object::address_to_object<token::Token>(*player1_token_id1);
p_vector_u8(b"player1_token_object1");
p(&player1_token_object1);
let now_secs = timestamp::now_seconds();
p_vector_u8(b"before staking now_secs");
p(&now_secs);
// player1 stakes their first Rapper token
streets::stake(player1, player1_token_object0);
// simulate that more than 4 days has passed
timestamp::fast_forward_seconds(4*SECS_PER_DAY);
let now_secs = timestamp::now_seconds();
p_vector_u8(b"after staking now_secs");
p(&now_secs);
// player1 unstakes their first Rapper token
streets::unstake(player1, &module_owner, player1_token_object0);
// player1 stakes their second Rapper token
streets::stake(player1, player1_token_object1);
// simulate that more than 4 days has passed
timestamp::fast_forward_seconds(4*SECS_PER_DAY);
// player1 unstakes their second Rapper token
streets::unstake(player1, &module_owner, player1_token_object1);
let player1_token_id0_skill = one_shot::public_skill_of(*player1_token_id0);
p_vector_u8(b"player1_token_id0_skill");
p(&player1_token_id0_skill);
assert!(player1_token_id0_skill == 75, 2);
let player1_token_id1_skill = one_shot::public_skill_of(*player1_token_id1);
p_vector_u8(b"player1_token_id1_skill");
p(&player1_token_id1_skill);
assert!(player1_token_id1_skill == 75, 2);
let player1_cred = coin::balance<cred_token::CRED>(player1_address);
p_vector_u8(b"player1_cred");
p(&player1_cred);
assert!(player1_cred == 8, 3);
// player1 goes on stage as the defender, using half of their CRED coins.
rap_battle::go_on_stage_or_battle(
player1,
player1_token_object0,
player1_cred/2
);
/* Player1 goes on stage as the challenger, using half of their CRED coins.
* This effectively allows them to exit the battle.
*/
rap_battle::go_on_stage_or_battle(
player1,
player1_token_object1,
player1_cred/2
);
p_vector_u8(b"");
p_vector_u8(b"After battle");
// player1 gets back all their CRED coins.
player1_cred = coin::balance<cred_token::CRED>(player1_address);
p_vector_u8(b"player1_cred");
p(&player1_cred);
assert!(player1_cred == 8, 3);
// player1 gets back all their Rapper tokens.
balance1 = one_shot::balance_of(player1_address);
p_vector_u8(b"balance1");
p(&balance1);
assert!(balance1 == 2, 1);
coin::destroy_burn_cap(burn_cap);
coin::destroy_mint_cap(mint_cap);
}
}

Please see the comments above for the exploit explanation.

Run with:

aptos move test --filter rap_battle_bug_exit_battle

Output:

Running Move unit tests
[debug] "init_module()"
[debug] "balance1"
[debug] 2
[debug] "player1_rapper_ids"
[debug] [ @0xe46a3c36283330c97668b5d4693766b8626420a5701c18eb64026075c3ec8a0a, @0xfab16b00983f01e5c2b7682472a4f4c3e5929fbba987958570b6290c02817df2 ]
[debug] "player1_token_id0"
[debug] @0xe46a3c36283330c97668b5d4693766b8626420a5701c18eb64026075c3ec8a0a
[debug] "player1_token_object0"
[debug] 0x1::object::Object<0x4::token::Token> {
inner: @0xe46a3c36283330c97668b5d4693766b8626420a5701c18eb64026075c3ec8a0a
}
[debug] "player1_token_id1"
[debug] @0xfab16b00983f01e5c2b7682472a4f4c3e5929fbba987958570b6290c02817df2
[debug] "player1_token_object1"
[debug] 0x1::object::Object<0x4::token::Token> {
inner: @0xfab16b00983f01e5c2b7682472a4f4c3e5929fbba987958570b6290c02817df2
}
[debug] "before staking now_secs"
[debug] 0
[debug] "after staking now_secs"
[debug] 345600
[debug] "player1_token_id0_skill"
[debug] 75
[debug] "player1_token_id1_skill"
[debug] 75
[debug] "player1_cred"
[debug] 8
[debug] ""
[debug] "After battle"
[debug] "player1_cred"
[debug] 8
[debug] "balance1"
[debug] 2
[ PASS ] 0x42::rap_battle_bug_exit_battle::rap_battle_bug_exit_battle
Test result: OK. Total tests: 1; passed: 1; failed: 0
{
"Result": "Success"
}

Recommended Mitigation

Check whether the challenger's address is the same as the defender's address.

const E_CHALLENGER_CANNOT_BE_DEFENDER: u64 = 3;
public entry fun go_on_stage_or_battle (
player: &signer,
rapper_token: Object<Token>,
bet_amount: u64
) acquires BattleArena {
let player_addr = signer::address_of(player);
let arena = borrow_global_mut<BattleArena>(@battle_addr);
if (arena.defender == @0x0) {
assert!(arena.defender_bet == 0, E_BATTLE_ARENA_OCCUPIED);
arena.defender = player_addr;
arena.defender_bet = bet_amount;
let token_id = object::object_address(&rapper_token);
arena.defender_token_id = token_id;
let first_bet = coin::withdraw<cred_token::CRED>(player, bet_amount);
coin::merge(&mut arena.prize_pool, first_bet);
one_shot::transfer_record_only(token_id, player_addr, @battle_addr);
object::transfer(player, rapper_token, @battle_addr);
event::emit(OnStage {
defender: player_addr,
token_id,
cred_bet: bet_amount,
});
} else {
+ assert!(arena.defender != player_addr, E_CHALLENGER_CANNOT_BE_DEFENDER);
assert!(arena.defender_bet == bet_amount, E_BETS_DO_NOT_MATCH);
let defender_addr = arena.defender;
let chall_addr = player_addr;
let chall_token_id = object::object_address(&rapper_token);
one_shot::transfer_record_only(chall_token_id, chall_addr, @battle_addr);
object::transfer(player, rapper_token, @battle_addr);
let chall_coins = coin::withdraw<cred_token::CRED>(player, bet_amount);
coin::merge(&mut arena.prize_pool, chall_coins);
let defender_skill = one_shot::skill_of(arena.defender_token_id);
let challenger_skill = one_shot::skill_of(chall_token_id);
let total_skill = defender_skill + challenger_skill;
let rnd = timestamp::now_seconds() % (if (total_skill == 0) { 1 } else { total_skill });
let winner = if (rnd < defender_skill) { defender_addr } else { chall_addr };
event::emit(Battle {
challenger: chall_addr,
challenger_token_id: chall_token_id,
winner,
});
let pool = coin::extract_all(&mut arena.prize_pool);
if (winner == defender_addr) {
coin::deposit(defender_addr, pool);
one_shot::increment_wins(arena.defender_token_id);
one_shot::transfer_record_only(arena.defender_token_id, @battle_addr, defender_addr);
one_shot::transfer_record_only(chall_token_id, @battle_addr, defender_addr);
} else {
coin::deposit(chall_addr, pool);
one_shot::increment_wins(chall_token_id);
one_shot::transfer_record_only(arena.defender_token_id, @battle_addr, chall_addr);
one_shot::transfer_record_only(chall_token_id, @battle_addr, chall_addr);
};
arena.defender = @0x0;
arena.defender_bet = 0;
arena.defender_token_id = @0x0;
}
}
Updates

Lead Judging Commences

bube Lead Judge 20 days ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
Assigned finding tags:

The defender and challenger can be the same person

Even if the defender and challenger are not the same address. The player can use two different addresses and still be both defender and challenger at the same time. The intended behavior here is the same Rapper tokens to be not able to battle themself.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.