One Shot: Reloaded

First Flight #47
Beginner FriendlyNFT
100 EXP
Submission Details
Impact: high
Likelihood: high

Battle Does Not Return NFTs to Winner

Author Revealed upon completion

Root + Impact

Description

Normal behavior: After a battle, both the defender and challenger NFTs should be returned to the rightful owner (the winner or split as per rules).

Issue: The code only updates the internal stats and ownership tables (transfer_record_only) but never calls object::transfer to actually return the NFTs to the winner’s account. As a result, both NFTs remain locked under @battle_addr.

// After deciding winner
if (winner == defender_addr) {
coin::deposit(defender_addr, pool);
one_shot::increment_wins(arena.defender_token_id);
// @> Only record changes, no object::transfer back to owners
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);
}

Risk

This happens in every battle resolution, since the return transfers are never executed.

Impact:

Both players permanently lose custody of their NFTs. Assets become locked at @battle_addr, causing total loss unless the module owner manually intervenes.

Recommended Mitigation

- 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);
+ 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);
+ object::transfer(module_owner, arena.defender_token, defender_addr);
+ object::transfer(module_owner, challenger_token, defender_addr);

Support

FAQs

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