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

L04. Rapper NFT Ownership Not Validated Before Metadata Update

accessdenied

Root + Impact

Description

  • Normal behavior: Players stake a rapper NFT and CRED to go on stage. The contract updates the RapperStats metadata to mark the NFT as on stage and transfers the NFT to the arena.

  • Issue: The function calls one_shot::transfer_record_only(token_id, player_addr, @battle_addr) before transferring the NFT:

let token_id = object::object_address(&rapper_token);
arena.defender_token_id = token_id;
​
one_shot::transfer_record_only(token_id, player_addr, @battle_addr);
object::transfer(player, rapper_token, @battle_addr);
​

  • If the player does not actually own rapper_token, transfer_record_only still executes.

  • Mitigating factor: object::transfer reverts if the signer does not own the NFT, rolling back the transaction and preventing persistent corruption.

Risk

Likelihood:

  • Low, because normal users can only pass NFTs they control.

  • Could occur if a malicious module attempts to call the function with an NFT the signer does not own.

Impact:

  • Minimal in production, as the transaction reverts and no permanent corruption occurs.

  • Metadata could be temporarily inconsistent during transaction execution, but the abort restores state.

Proof of Concept

// Pseudocode
rap_battle::go_on_stage_or_battle(&player_signer, unowned_rapper_token, 10);
​
// transfer_record_only executes and updates stats
// object::transfer reverts because player_signer is not the owner
// Transaction aborts
// Stats metadata is rolled back
​

Recommended Mitigation

- one_shot::transfer_record_only(token_id, player_addr, @battle_addr);
+ assert!(object::owner(&rapper_token) == signer::address_of(player), E_NOT_OWNER);
+ object::transfer(player, rapper_token, @battle_addr);
+ one_shot::transfer_record_only(token_id, player_addr, @battle_addr);
​

  • Explanation:

    • Check ownership explicitly before modifying metadata.

    • Transfer NFT first, then update RapperStats only if the transfer succeeds.

    • Reduces reliance on transaction rollback for safety and improves clarity.

Summary:

  • Low-risk issue in practice, but adding an ownership check makes the contract safer and prevents temporary metadata inconsistencies during transaction execution.

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.