One Shot: Reloaded
First Flight #48
Beginner FriendlyNFT
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

NFTs Permanently Stuck in Protocol Custody After Battles

ambitiousdydx

Root + Impact

Description

  • After a battle, the protocol should return both Rapper NFTs to their owners by transferring the objects back from @battle_addr.

  • The specific issue is that while internal records are updated, no object::transfer calls are made post-battle to return the NFTs, leaving them custodied at @battle_addr indefinitely without a user-callable retrieval function.

// In rap_battle.move
if (winner == defender_addr) {
// ... prize and wins ...
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(module_owner, defender_token, defender_addr);
//@> // Missing: object::transfer(module_owner, challenger_token, defender_addr); // But should be to chall_addr if not stealing
} else {
// Similar missing transfers
};

Risk

Likelihood:

High

  • A battle completes successfully.

  • No post-battle transfer of NFT objects occurs.

Impact:

High

  • Users cannot access or use their NFTs after battles.

  • Protocol custody leads to permanent asset lockup without owner intervention.

Proof of Concept

  • Go on stage with defender, then challenge with another.

  • After battle, attempt to use or transfer the NFTs from original addresses: fails as objects are at @battle_addr.

#[test(module_owner = @battle_addr, player1 = @0x123, player2 = @0x456)]
fun test_nfts_stuck_after_battle(module_owner: &signer, player1: &signer, player2: &signer) acquires battle_addr::rap_battle::BattleArena, battle_addr::one_shot::Collection, battle_addr::one_shot::RapperStats {
// Setup: Initialize modules
battle_addr::cred_token::init_module(module_owner);
battle_addr::one_shot::init_module(module_owner);
battle_addr::rap_battle::init_module(module_owner);
​
// Mint two rappers
battle_addr::one_shot::mint_rapper(module_owner, signer::address_of(player1));
battle_addr::one_shot::mint_rapper(module_owner, signer::address_of(player2));
​
// Player1 goes on stage
let rapper1 = /* assume fetched Object<Token> for player1 */;
battle_addr::rap_battle::go_on_stage_or_battle(player1, rapper1, 1);
​
// Player2 challenges
let rapper2 = /* assume fetched Object<Token> for player2 */;
battle_addr::rap_battle::go_on_stage_or_battle(player2, rapper2, 1);
​
// After battle, NFTs are at @battle_addr
let token_id1 = /* assume token_id of rapper1 */;
let token_id2 = /* assume token_id of rapper2 */;
assert!(object::owner(rapper1) == @battle_addr, 0);
assert!(object::owner(rapper2) == @battle_addr, 0);
​
// No user-callable function to transfer back; demonstrate stuck by absence of transfer
// (In real test, attempt to use or transfer would fail without module_owner)
}

Recommended Mitigation

  • Add after record updates, assuming module_owner signer is passed or accessible:

+ object::transfer(module_owner, object::address_to_object<Token>(arena.defender_token_id), original_defender_addr);
+ object::transfer(module_owner, object::address_to_object<Token>(chall_token_id), original_chall_addr);
Updates

Lead Judging Commences

bube Lead Judge about 1 month ago
Submission Judgement Published
Validated
Assigned finding tags:

The tokens are not returned back to the players after the battle

Support

FAQs

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