One Shot: Reloaded

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

Phantom Registry Ownership Hijack

Root + Impact

Description

  • Expected: Any “logical” ownership change (registry/table) must only occur after verifying the NFT is actually in protocol custody and that from == current owner in the registry.

  • Issue: transfer_record_only(token_id, from, to) updates the registry without an explicit check that the object is truly custodied by the protocol (and without an assertion that the registry and custody are in sync). If other flows later rely only on the registry (e.g., during unstake), an attacker can create desync that enables theft when custody later becomes available.

// one_shot.move (conceptual)
public fun transfer_record_only(token_id, from, to) acquires Stats {
let s = table::borrow_mut(&mut stats_res.stats, token_id);
// Missing: assert!(s.owner == from)
// Missing: assert!(is_custodied(token_id, @battle_addr)) // proof of custody
s.owner = to;
decrement(owner_counts[from]);
increment(owner_counts[to]);
}

Risk

Likelihood:

  • Every path that calls transfer_record_only(..., @battle_addr, X) without first proving custody risks registry/custody divergence.

  • Any follow-up function that trusts the registry to decide “who can pull the object out” becomes an escalation vector.

Impact:

  • Theft post-facto when custody becomes available.

  • Long-lived state corruption (phantom ownership) that breaks invariants and can brick withdrawal flows.

Proof of Concept

// Sketch:
// 1) Force a call path that executes transfer_record_only(tokenB, @battle_addr, attacker)
// even though tokenB’s object is NOT actually inside protocol custody (missing custody check).
// 2) Later, when tokenB does get custodied (e.g., owner stakes or enters battle),
// any unstake/withdraw flow that trusts the registry may transfer tokenB to attacker.

Recommended Mitigation

public fun transfer_record_only(token_id: address, from: address, to: address) acquires Stats {
- let s = table::borrow_mut(&mut stats_res.stats, token_id);
- s.owner = to;
+ let s = table::borrow_mut(&mut stats_res.stats, token_id);
+ assert!(s.owner == from, E_NOT_OWNER);
+ assert!(is_custodied(token_id, @battle_addr), E_NOT_IN_CUSTODY); // prove custody
+ s.owner = to;
let c_from = table::borrow_mut(&mut stats_res.owner_counts, from);
*c_from = *c_from - 1;
let c_to = table::borrow_mut(&mut stats_res.owner_counts, to);
*c_to = *c_to + 1;
}
Updates

Lead Judging Commences

bube Lead Judge 17 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.