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

[H-02] NFT Ownership Registry Can Become Desynchronized

agilegypsy

Root + Impact

Description

The protocol maintains an internal ownership registry (RapperStats.stats table) that tracks NFT ownership separately from actual token objects. This creates a critical vulnerability where the internal registry can become desynchronized from actual token ownership, leading to users losing access to their NFTs in staking and battle functions.

When NFTs are transferred outside the protocol's controlled functions, the internal registry becomes outdated while the actual token ownership changes, resulting in users being unable to stake or battle with their legitimate NFTs.

// In one_shot.move
@> struct StatsData has store {
@> owner: address, // This can become outdated
@> weak_knees: bool,
@> heavy_arms: bool,
@> spaghetti_sweater: bool,
@> calm_and_ready: bool,
@> battles_won: u64,
@> }

Risk

Likelihood:

  • NFTs can be transferred directly via object::transfer, bypassing the protocol's internal registry

  • Users may accidentally transfer NFTs outside the protocol's controlled functions

  • The internal registry has no mechanism to sync with actual token ownership

Impact:

  • Users lose access to staking and battle functionality for their legitimate NFTs

  • Ownership disputes between internal registry and actual token ownership

  • Protocol dysfunction as core features become unreliable

Proof of Concept

This PoC demonstrates how the ownership registry becomes desynchronized:

// Step-by-step demonstration of the desynchronization attack
let user = account::create_account_for_test(@user_addr);
let new_owner = account::create_account_for_test(@new_owner_addr);
let module_owner = account::create_account_for_test(@battle_addr);
​
// 1. Mint NFT to original user
one_shot::mint_rapper(&module_owner, signer::address_of(&user));
let token_id = /* token address */;
​
// 2. Verify internal registry shows user as owner
let (wk, ha, ss, cr, wins) = one_shot::read_stats(token_id);
// Registry correctly shows @user_addr as owner
​
// 3. Transfer NFT directly (bypassing protocol sync)
let token_object = /* get token object */;
object::transfer(&user, token_object, signer::address_of(&new_owner));
​
// 4. Check ownership status
// Actual token owner: @new_owner_addr (correct)
// Internal registry owner: @user_addr (outdated!)
​
// 5. Attempt to use NFT - fails for new owner
// new_owner tries to stake the NFT:
// streets::stake(&new_owner, token_object)
// This fails because internal registry still shows @user_addr as owner
​
// Result: New legitimate owner cannot use their NFT

Recommended Mitigation

The mitigation implements a synchronization mechanism to keep the internal registry aligned with actual token ownership:

- public(friend) fun transfer_record_only(token_id: address, from: address, to: address)
+ public(friend) fun sync_ownership(token_id: address, new_owner: address) acquires RapperStats {
+ let stats_res = borrow_global_mut<RapperStats>(@battle_addr);
+ assert!(table::contains(&stats_res.stats, token_id), E_INVALID_TOKEN);
+
+ let stats = table::borrow_mut(&mut stats_res.stats, token_id);
+ let old_owner = stats.owner;
+
+ // Update ownership
+ stats.owner = new_owner;
+
+ // Update owner counts
+ let c_old = table::borrow_mut(&mut stats_res.owner_counts, old_owner);
+ *c_old = *c_old - 1;
+ if (*c_old == 0) {
+ table::remove(&mut stats_res.owner_counts, old_owner);
+ };
+
+ if (table::contains(&stats_res.owner_counts, new_owner)) {
+ let c_new = table::borrow_mut(&mut stats_res.owner_counts, new_owner);
+ *c_new = *c_new + 1;
+ } else {
+ table::add(&mut stats_res.owner_counts, new_owner, 1);
+ };
+ }

This mitigation adds a synchronization mechanism that updates the internal ownership registry whenever token ownership changes, ensuring the registry always reflects the actual token owner.

Updates

Lead Judging Commences

bube Lead Judge 16 days ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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