[Griefing] Defender’s NFT can be locked indefinitely if no challenger joins
Description
-
Normal behavior:
-
When a player (Defender) goes on stage, they risk their Rapper and their stake.
-
A Challenger should arrive to complete the battle, after which NFTs and rewards are distributed.
-
Actual behavior:
-
If no Challenger arrives, the Defender’s Rapper remains in custody of @battle_addr.
-
There is no cancel/withdraw function, so the Defender cannot recover their NFT or stake.
-
Assets remain locked until the protocol owner redeploys or manually intervenes.
if (arena.defender == @0x0) {
arena.defender = player_addr;
arena.defender_token_id = token_id;
...
one_shot::transfer_record_only(token_id, player_addr, @battle_addr);
object::transfer(player, rapper_token, @battle_addr);
}
Risk
Likelihood:
Impact:
-
Defender’s NFT and staked tokens are indefinitely locked.
-
Requires manual fix or redeployment by protocol owner.
-
No direct permanent loss, but players may perceive it as loss of assets.
Proof of Concept
one_shot.move
#[test_only]
public fun test_mint_to_player_and_return_object(
module_owner: &signer,
player: &signer
): (object::Object<aptos_token_v2::token::Token>, address) acquires RapperStats {
use std::string;
use std::option;
use aptos_framework::object::{Self as object};
use aptos_token_v2::token::{Self as token};
use aptos_token_v2::collection;
use aptos_std::table;
if (!exists<Collection>(@battle_addr)) {
init_module(module_owner);
};
if (!exists<RapperStats>(@battle_addr)) {
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 });
};
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 player_addr = signer::address_of(player);
let stats_res = borrow_global_mut<RapperStats>(@battle_addr);
table::add(&mut stats_res.stats, token_id, StatsData {
owner: player_addr,
weak_knees: true,
heavy_arms: true,
spaghetti_sweater: true,
calm_and_ready: false,
battles_won: 0,
});
if (table::contains(&stats_res.owner_counts, player_addr)) {
let cnt = table::borrow_mut(&mut stats_res.owner_counts, player_addr);
*cnt = *cnt + 1;
} else {
table::add(&mut stats_res.owner_counts, player_addr, 1);
};
object::transfer(module_owner, token_obj, player_addr);
let player_obj: object::Object<token::Token> = object::address_to_object<token::Token>(token_id);
(player_obj, token_id)
}
tests/griefing_poc.move
#[test(battle_addr = @0x42, alice = @0xa11ce)]
public fun test_defender_locked_without_challenger(battle_addr: &signer, alice: &signer) {
rap_battle::test_init(battle_addr);
let (alice_obj, alice_token_id) =
one_shot::test_mint_to_player_and_return_object(battle_addr, alice);
rap_battle::go_on_stage_or_battle(alice, alice_obj, 0);
}
Test confirms: Defender’s NFT is locked if no challenger arrives.
Recommended Mitigation
Introduce a cancel/timeout function for defenders, e.g.:
+ public entry fun cancel_defense(
+ player: &signer,
+ module_owner: &signer
+ ) acquires BattleArena {
+ let arena = borrow_global_mut<BattleArena>(@battle_addr);
+ let player_addr = signer::address_of(player);
+
+ // Only current defender can cancel
+ assert!(arena.defender == player_addr, E_NOT_OWNER);
+
+ // Return defender's Rapper: first fix logical registry, then physical object
+ let def_tid = arena.defender_token_id;
+ let def_obj: Object<Token> = object::address_to_object<Token>(def_tid);
+ one_shot::transfer_record_only(def_tid, @battle_addr, player_addr);
+ object::transfer(module_owner, def_obj, player_addr);
+
+ // Refund only the defender's stake (challenger hasn't joined yet)
+ if (arena.defender_bet > 0) {
+ let refund = coin::extract_all(&mut arena.prize_pool);
+ coin::deposit(player_addr, refund);
+ } else {
+ // keep state clean even if zero; extract_all would be zero
+ let _ = coin::extract_all(&mut arena.prize_pool);
+ };
+
+ // Reset arena state
+ arena.defender = @0x0;
+ arena.defender_bet = 0;
+ arena.defender_token_id = @0x0;
+ }
-
Custody signer required: The Rapper object is physically held by @battle_addr, so returning it requires the module owner’s signer (module_owner: &signer) for object::transfer.
-
Registry first, then object: Call one_shot::transfer_record_only before the physical object::transfer to keep the internal ownership table consistent with the real object owner.
-
Refund only defender’s stake: Before any challenger joins, the entire prize_pool consists solely of the defender’s bet. Refund that amount and reset the pool.
-
Zero-bet safety: Even when the bet is zero, extract/clear the pool to avoid state drift and keep the arena clean.
-
Strict authorization: Ensure only the current defender (arena.defender) can execute cancel_defense; abort otherwise.
-
State reset: After canceling, clear defender, defender_bet, and defender_token_id so the arena becomes available again.
-
(Optional) cooldown/timeout: You may enforce a minimum waiting period (e.g., timestamp::now_seconds() - placed_at >= N) before allowing cancelation to reduce spam/griefing.
