One Shot: Reloaded

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

Undocumented Asset Forfeiture: Battle Winner Takes Loser's Rapper NFT

Author Revealed upon completion

Undocumented Asset Forfeiture: Battle Winner Takes Loser's Rapper NFT

Severity: Critical (High likelihood + complete asset loss + undocumented behavior contradicting user expectations).

#Description
The rap_battle module's core gameplay logic contains a critical flaw that directly contradicts the project's documentation and leads to an unexpected loss of player assets. The project description states, "the winner receives the prize pool, and the winner’s Rapper accrues a win." However, the on-chain implementation of a battle's resolution (go_on_stage_or_battle) transfers the internal ownership record of both the winner's and the loser's Rapper NFTs to the winner.

This behavior means that every battle is effectively a high-stakes "pink slips" match where the losing player permanently forfeits their Rapper NFT to the winner. This undocumented mechanic results in irreversible loss of players' primary game assets, which is a severe violation of user expectations and trust.

Root Cause in the codebase with @> marks to highlight the relevant section:

// rap_battle.move
// ... inside go_on_stage_or_battle function, after battle outcome calculation ...
event::emit(Battle {
challenger: chall_addr,
challenger_token_id: chall_token_id,
winner,
});
let pool = coin::extract_all(&mut arena.prize_pool);
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); // Transfers loser's NFT to winner
} else { // Challenger wins
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); // Transfers loser's NFT to winner
@> one_shot::transfer_record_only(chall_token_id, @battle_addr, chall_addr);
};
arena.defender = @0x0;
arena.defender_bet = 0;
arena.defender_token_id = @0x0;
}
}
  • State Mutation: In both the defender-wins and challenger-wins branches, the one_shot::transfer_record_only function is called for both participating NFTs, assigning their internal ownership records to the winner's address.

#Risk

  • Likelihood: High. This is the inherent, default behavior for every completed battle within the current code, not an edge case.

  • Impact: Critical. Players who lose a battle will unexpectedly and irreversibly lose their Rapper NFT. This leads to a direct, unannounced loss of a player's core asset, severely damaging user trust and rendering the game unplayable for those who discover this consequence too late.

#PoC

  1. Player A (0xA) stakes RapperNFT_A as a defender. one_shot records 0xRapper_A owned by 0xA.

  2. Player B (0xB) challenges with RapperNFT_B. one_shot records 0xRapper_B owned by 0xB.

  3. Battle resolves, Player A wins.

  4. The code executes one_shot::transfer_record_only(0xRapper_A, @battle_addr, 0xA) (returning winner's NFT) and one_shot::transfer_record_only(0xRapper_B, @battle_addr, 0xA).

  5. Result: Player A's internal NFT count increases by one (they now own 0xRapper_B), while Player B's internal NFT count decreases by one, and 0xRapper_B is now internally recorded as owned by 0xA. Player B has lost their NFT.

#Recommended Mitigation
Modify the rap_battle::go_on_stage_or_battle function to ensure that only the winner's NFT is returned to them (or remains with them), and the loser's NFT is returned to the loser.

--- a/sources/rap_battle.move
+++ b/sources/rap_battle.move
@@ -87,10 +87,10 @@
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);
+ one_shot::transfer_record_only(chall_token_id, @battle_addr, chall_addr); // Return challenger's NFT to challenger
} else { // Challenger wins
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(arena.defender_token_id, @battle_addr, defender_addr); // Return defender's NFT to defender
one_shot::transfer_record_only(chall_token_id, @battle_addr, chall_addr);
};

Support

FAQs

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