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

Losing Rapper NFT is permanently stuck in protocol custody.

folijr01

Root + Impact

Description

  • The rap_battle module's normal behavior is to handle custody of both Rapper NFTs during a battle and return them to their respective owners once the battle is complete. The specific issue is that while the code updates the internal ownership records using one_shot::transfer_record_only, it does not physically transfer the losing Rapper NFT back to its owner using object::transfer. This leaves the NFT permanently stuck in the protocol's custody at the @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); @>
}

Risk

Likelihood:

  • This will occur whenever a battle ends with a winner and a loser.

  • The object::transfer function is not called for the losing NFT, so it will remain at the @battle_addr.

Impact:

  • A player who loses a battle suffers a permanent loss of their Rapper NFT.

  • This results in a severe resource leak and asset loss for the user, which can lead to a complete loss of trust in the protocol.

Proof of Concept

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); @>
}

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

Lead Judging Commences

bube Lead Judge 16 days ago
Submission Judgement Published
Validated
Assigned finding tags:

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

Appeal created

blackgrease Auditor
16 days ago
bube Lead Judge
15 days ago
bube Lead Judge 14 days 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.