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

one_shot module's owner_counts table is not updated when NFTs change hands in rap_battle module.

Author Revealed upon completion

Root + Impact

Description

  • The one_shot module maintains an owner_counts table to track the number of Rapper NFTs an address owns. The normal behavior is that this count is incremented when a new Rapper is minted. The specific issue is that when a Rapper NFT is transferred to a new owner in a battle (in the rap_battle module), the owner_counts table in the one_shot module is never decremented for the old owner or incremented for the new owner.

// Root cause in the codebase with @> marks to highlight the relevant section
// The one_shot module correctly increments the count upon minting
public(friend) fun mint_rapper(module_owner: &signer, recipient: address): Object<Token> acquires RapperStats, Collection {
// ...
let stats_res = borrow_global_mut<RapperStats>(@battle_addr);
let count = table::borrow_mut(&mut stats_res.owner_counts, recipient); @>
*count = *count + 1;
// ...
}
​
// But the rap_battle module's transfer logic does not update it
if (winner == 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); @>
} else {
// ...
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 every time a Rapper NFT is transferred between players.

  • The rap_battle module uses one_shot::transfer_record_only to update ownership, but this function does not affect the owner_counts table.

Impact:

  • The owner_counts table will contain inaccurate data, leading to a state-inconsistency bug

  • Any future function that relies on this count will fail or provide incorrect information, potentially affecting game logic or user-facing displays.

Proof of Concept

// The one_shot module correctly increments the count upon minting
public(friend) fun mint_rapper(module_owner: &signer, recipient: address): Object<Token> acquires RapperStats, Collection {
// ...
let stats_res = borrow_global_mut<RapperStats>(@battle_addr);
let count = table::borrow_mut(&mut stats_res.owner_counts, recipient); @>
*count = *count + 1;
// ...
}
​
// But the rap_battle module's transfer logic does not update it
if (winner == 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); @>
} else {
// ...
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

- public(friend) fun transfer_record_only(token_id: address, old_owner: address, new_owner: address) acquires RapperStats {
- let stats_res = borrow_global_mut<RapperStats>(@battle_addr);
- let s = table::borrow_mut(&mut stats_res.stats, token_id);
- s.owner = new_owner;
- }
+ public(friend) fun transfer_record_and_counts(token_id: address, old_owner: address, new_owner: address) acquires RapperStats {
+ let stats_res = borrow_global_mut<RapperStats>(@battle_addr);
+ let s = table::borrow_mut(&mut stats_res.stats, token_id);
+ s.owner = new_owner;
+
+ let old_count = table::borrow_mut(&mut stats_res.owner_counts, old_owner);
+ *old_count = *old_count - 1;
+
+ let new_count = table::borrow_mut(&mut stats_res.owner_counts, new_owner);
+ *new_count = *new_count + 1;
+ }

Support

FAQs

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