Winner cannot physically recover their Rapper NFT
Description
one_shot::transfer_record_only(token_id, player_addr, @battle_addr);
@> object::transfer(player, rapper_token, @battle_addr);
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);
}
Thus, the winner sees their address as the "owner" in stats, but does not actually regain custody of their NFT object. The NFT remains locked under @battle_addr.
Risk
Likelihood:
-
Every single battle ends with the Rapper NFTs stuck in custody at @battle_addr.
-
There is no function in the codebase that restores custody to the winner.
-
The bug will occur consistently whenever two players fight.
Impact:
-
Permanent Asset Lock: Rapper NFTs are irretrievably stuck.
-
Broken Gameplay & Incentives: Players have no reason to participate in battles if they cannot reclaim their NFTs.
-
Loss of Funds: If NFTs have market or in-game value, users lose their assets despite "winning."
Proof of Concept
#[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 {
}
#[test_only]
public fun test_init(arena_owner: &signer) {
init_module(arena_owner)
}
module battle_addr::asset_lock_stage_poc {
use std::signer;
use aptos_framework::object::{Self as object, Object};
use aptos_token_v2::token::Token;
use battle_addr::one_shot;
use battle_addr::rap_battle;
#[test(battle_addr = @0x42, alice = @0xa11ce)]
public fun test_asset_lock_after_on_stage(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);
let obj_from_custody: Object<Token> = object::address_to_object<Token>(alice_token_id);
object::transfer(battle_addr, obj_from_custody, signer::address_of(alice));
}
#[test(battle_addr = @0x42, alice = @0xa11ce)]
#[expected_failure]
public fun test_player_cannot_transfer_while_in_custody(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);
let try_take: Object<Token> = object::address_to_object<Token>(alice_token_id);
object::transfer(alice, try_take, signer::address_of(alice));
}
}
Steps
-
Alice and Bob each mint a Rapper NFT
→ one_shot::test_mint_to_player_and_return_object
-
Alice goes on stage with bet = 0
→ rap_battle::go_on_stage_or_battle
-
Bob matches bet = 0, completing a battle
→ rap_battle::go_on_stage_or_battle
-
After battle:
-
Both NFTs remain in custody of @battle_addr.
-
Winner cannot transfer their NFT back.
-
@battle_addr can transfer the NFTs (proving custody).
Recommended Mitigation
Explicitly return NFTs to the actual winner after the battle:
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);
+ let def_obj: Object<Token> = object::address_to_object<Token>(arena.defender_token_id);
+ object::transfer(module_owner, def_obj, defender_addr);
+ let chall_obj: Object<Token> = object::address_to_object<Token>(chall_token_id);
+ object::transfer(module_owner, chall_obj, 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);
+ let def_obj: Object<Token> = object::address_to_object<Token>(arena.defender_token_id);
+ object::transfer(module_owner, def_obj, chall_addr);
+ let chall_obj: Object<Token> = object::address_to_object<Token>(chall_token_id);
+ object::transfer(module_owner, chall_obj, chall_addr);
}
