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

Losing Rapper NFT is permanently stuck in protocol custody.

Author Revealed upon completion

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

Support

FAQs

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