One Shot: Reloaded
First Flight #48
Beginner FriendlyNFT
100 EXP
View results
Previous Next
Submission Details
Severity: low
Valid

Unstake Binding Bug — NFT Theft & Fake Rewards

raminidalf

Root + Impact

Description

  • Expected behavior: When a Rapper NFT is staked, the protocol should lock that specific token and return it on unstake, along with rewards.

  • Issue: The unstake function accepts a rapper_token parameter and does not validate it against the NFT originally staked. This allows returning any Rapper NFT currently custodied at @battle_addr.

// streets.move
public entry fun unstake(staker: &signer, module_owner: &signer, rapper_token: Object<Token>) acquires StakeInfo {
let token_id = object::object_address(&rapper_token);
​
// @> BUG: stake_info does not record token_id
// @> Rewards and stat updates are computed on the token_id provided here
let (wk, ha, ss, cr, wins) = one_shot::read_stats(token_id);
​
one_shot::transfer_record_only(token_id, @battle_addr, staker_addr);
object::transfer(module_owner, rapper_token, staker_addr); // @> Returns any passed token
}
​

Risk

Likelihood:

  • Occurs on every unstake call, because the function trusts the user-supplied token.

  • Becomes exploitable whenever multiple NFTs are custodied at @battle_addr.

Impact:

  • Theft of any Rapper NFT held in custody.

  • Fake stat changes and unearned CRED rewards.

Proof of Concept

// Simplified scenario:
streets::stake(&alice, &battle_addr, rapperA);
streets::stake(&charlie, &battle_addr, rapperB);
​
// Bob calls unstake with rapperB object
streets::unstake(&bob, &battle_addr, rapperB);
​
// Result: Bob receives rapperB + CRED rewards, even though he never staked it.
​

Recommended Mitigation

struct StakeInfo has key, store {
- start_time_seconds: u64,
- owner: address,
+ start_time_seconds: u64,
+ owner: address,
+ token_id: address, // bind token_id
}
​
public entry fun stake(...) {
- move_to(staker, StakeInfo { start_time_seconds: now, owner: staker_addr });
+ move_to(staker, StakeInfo { start_time_seconds: now, owner: staker_addr, token_id });
}
​
public entry fun unstake(...) acquires StakeInfo {
- let token_id = object::object_address(&rapper_token);
- let (wk, ha, ss, cr, wins) = one_shot::read_stats(token_id);
- one_shot::transfer_record_only(token_id, @battle_addr, staker_addr);
- object::transfer(module_owner, rapper_token, staker_addr);
+ let si = borrow_global<StakeInfo>(staker_addr);
+ assert!(si.token_id == object::object_address(&rapper_token), E_TOKEN_NOT_STAKED);
+ let (wk, ha, ss, cr, wins) = one_shot::read_stats(si.token_id);
+ one_shot::transfer_record_only(si.token_id, @battle_addr, staker_addr);
+ // Return the custodied token from protocol storage
+ let stored_obj = internal_pop_custodied_object(si.token_id);
+ object::transfer(module_owner, stored_obj, staker_addr);
}
​
Updates

Lead Judging Commences

bube Lead Judge about 1 month ago
Submission Judgement Published
Validated
Assigned finding tags:

Malicious staker can unstake someone else's token

Support

FAQs

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