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

L02. staked token ID mismatch / stats manipulation risk

Author Revealed upon completion

Root + Impact

Description

  • Normal behavior: When a staker unstakes their rapper NFT, the contract should update the token’s stats according to the staking period and return the NFT and rewards to the original staker. Ownership metadata in RapperStats should remain accurate.

  • Issue: The unstake function does not verify that the rapper_token parameter matches the token actually staked by the staker. This allows a staker to pass a different token ID. The call to transfer_record_only then updates stats and ownership metadata for a token the staker never staked, potentially minting rewards incorrectly.

// Root cause in the codebase with @> marks to highlight the relevant section
let token_id = object::object_address(&rapper_token);
assert!(exists<StakeInfo>(staker_addr), E_TOKEN_NOT_STAKED);
let stake_info = borrow_global<StakeInfo>(staker_addr);
assert!(stake_info.owner == staker_addr, E_NOT_OWNER);
// @> No check that stake_info.token_id == token_id
one_shot::transfer_record_only(token_id, @battle_addr, staker_addr);
object::transfer(module_owner, rapper_token, staker_addr);
​

  • Mitigating factor: The function requires both the staker’s and module_owner’s signatures, which reduces the risk because a staker cannot unilaterally call unstake for an arbitrary token without module_owner approval.

Risk

Likelihood:

  • Without module_owner, any staker could pass an arbitrary token ID to unstake.

  • With module_owner required, risk is reduced but still exists if module_owner logic is misused or bypassed.

Impact:

  • Stats and owner_counts tables become inaccurate, misrepresenting NFT ownership.

  • Stakers could receive rewards (CRED tokens) for NFTs they never staked if the module_owner approves incorrect calls.

Proof of Concept

// Scenario with mitigation note:
// Alice staked NFT #1
// Module owner signs the transaction
rap_stake::unstake(&alice_signer, &module_owner_signer, nft_2);
​
// NFT #2 stats are updated as if Alice staked it
// NFT #2 ownership is set to Alice
// Alice may receive CRED rewards incorrectly
​
// This only works if module_owner signs the transaction, reducing risk compared to a staker-only call
​

Recommended Mitigation

+ struct StakeInfo has key {
+ owner: address,
+ token_id: address, // store the staked token ID
+ start_time_seconds: u64,
+ }
+
+ // During staking
+ move_to(staker, StakeInfo {
+ owner: staker_addr,
+ token_id,
+ start_time_seconds: timestamp::now_seconds(),
+ });
+
+ // During unstake
+ assert!(stake_info.token_id == token_id, E_TOKEN_MISMATCH);
- one_shot::transfer_record_only(token_id, @battle_addr, staker_addr);
+ one_shot::transfer_record_only(stake_info.token_id, @battle_addr, staker_addr);
​

Summary: Storing and checking the token ID ensures the staker can only unstake the exact token they staked, making transfer_record_only safe and rewards accurate. The module_owner signature reduces the risk but does not fully eliminate it.

Support

FAQs

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