One Shot: Reloaded
First Flight #47
Beginner FriendlyNFT
100 EXP
View all submissions
Previous Next
Submission Details
Impact: high
Likelihood: high

Battle winners receive internal ownership records but NFTs remain permanently locked at protocol address

Author Revealed upon completion

Root + Impact

Description

  • Normal behavior: When players battle, both NFTs are transferred to the protocol for custody. After battle resolution, the winner should receive both NFTs back along with the prize pool, allowing them to use these NFTs for future staking or battles.

  • The specific issue: Battle resolution only updates internal ownership records via transfer_record_only() but never executes the actual object::transfer() calls to return NFTs to winners. This occurs because the function lacks the module owner's signer required to transfer NFTs out of the protocol address.

// Root cause in the codebase with @> marks to highlight the relevant section
// Root cause in battle resolution - missing module owner signer
public entry fun go_on_stage_or_battle(
player: &signer, // @> Only player signer available, no module_owner signer
rapper_token: Object<Token>,
bet_amount: u64
) acquires BattleArena {
// ... battle logic ...
if (winner == defender_addr) {
coin::deposit(defender_addr, pool);
one_shot::increment_wins(arena.defender_token_id);
// @> Only updates internal records, no actual NFT transfer possible
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);
// @> MISSING: object::transfer() calls require module_owner signer which is unavailable
} else {
coin::deposit(chall_addr, pool);
one_shot::increment_wins(chall_token_id);
// @> Same issue for challenger winner path
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);
// @> MISSING: Cannot execute object::transfer() without module_owner signer
}
}

Risk

Likelihood:

  • Every battle will trigger this bug since the resolution path always executes and lacks the necessary module owner signer

  • Players will immediately notice they cannot use their "won" NFTs for subsequent protocol interactions

  • The protocol becomes completely unusable after the first battle occurs

Impact:

  • All battled NFTs become permanently inaccessible to players while internal records incorrectly show winner ownership

  • Winners cannot stake, transfer, or battle with their NFTs again due to object ownership mismatch

  • Complete protocol functionality breakdown after first battle with no recovery mechanism

Proof of Concept

#[test_only]
module battle_addr::custody_bug_test {
use std::signer;
use aptos_framework::account;
use battle_addr::one_shot;
​
// Test demonstrating the NFT custody bug
#[test]
public fun test_custody_bug_concept() {
let module_owner = account::create_account_for_test(@battle_addr);
let winner = account::create_account_for_test(@0x111);
let winner_addr = signer::address_of(&winner);
// Mint NFT (auto-initializes module)
one_shot::mint_rapper(&module_owner, winner_addr);
// Verify initial state
assert!(one_shot::balance_of(winner_addr) == 1, 1);
assert!(one_shot::balance_of(@battle_addr) == 0, 2);
// The bug in rap_battle.move:
// After battle, only transfer_record_only() is called
// Missing object::transfer() leaves NFTs stuck at protocol
// Winner shows ownership but cannot access NFT objects
}
}

Recommended Mitigation

- remove this code
+ add this code
public entry fun go_on_stage_or_battle(
player: &signer,
+ module_owner: &signer,
rapper_token: Object<Token>,
bet_amount: u64
) acquires BattleArena {
// ... existing battle logic ...
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);
+ // Return actual NFT objects to winner
+ let defender_obj = object::address_to_object<Token>(arena.defender_token_id);
+ let challenger_obj = object::address_to_object<Token>(chall_token_id);
+ object::transfer(module_owner, defender_obj, defender_addr);
+ object::transfer(module_owner, challenger_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);
+ // Return actual NFT objects to winner
+ let defender_obj = object::address_to_object<Token>(arena.defender_token_id);
+ let challenger_obj = object::address_to_object<Token>(chall_token_id);
+ object::transfer(module_owner, defender_obj, chall_addr);
+ object::transfer(module_owner, challenger_obj, chall_addr);
}
}

Support

FAQs

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