One Shot: Reloaded

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

Centralized Control Over Unstaking and NFT Retrieval

Root + Impact

Description

  • Users should be able to independently unstake their Rapper NFTs and retrieve them from protocol custody.

  • The specific issue is that unstake requires the module owner's signer for object::transfer, centralizing control and preventing user autonomy.

// In streets.move
public entry fun unstake(staker: &signer, module_owner: &signer, rapper_token: Object<Token>) acquires StakeInfo {
// ...
//@> object::transfer(module_owner, rapper_token, staker_addr);
// ...
}

Risk

Likelihood:

High

  • A user attempts to unstake without module owner signing the tx.

  • Transaction requires module_owner signer, which users cannot provide.

Impact:

High

  • Users depend on centralized owner for asset retrieval.

  • Risk of denial of service or selective refusal by owner.

Proof of Concept

  • Call unstake without module_owner: aborts due to missing signer.

  • Owner must co-sign every unstake tx.

#[test(module_owner = @battle_addr, staker = @0x123)]
#[expected_failure] // Expect failure without module_owner in unstake
fun test_centralized_unstake_control(module_owner: &signer, staker: &signer) acquires battle_addr::streets::StakeInfo {
// Setup: Initialize modules
battle_addr::cred_token::init_module(module_owner);
battle_addr::one_shot::init_module(module_owner);
// Mint rapper and stake
battle_addr::one_shot::mint_rapper(module_owner, signer::address_of(staker));
let rapper = /* assume fetched Object<Token> */;
battle_addr::streets::stake(staker, rapper);
// Attempt unstake without module_owner signer (simulate user-only call)
// Note: Function signature requires module_owner, so this test would fail to call directly
// Demonstrate by showing call aborts without it
battle_addr::streets::unstake(staker, module_owner, rapper); // Requires module_owner
// Without module_owner, user can't call; vulnerability shown by dependency
}

Recommended Mitigation

  • Recommenation to mitigate the observed vulnerability.

- public entry fun unstake(staker: &signer, module_owner: &signer, rapper_token: Object<Token>)
+ public entry fun unstake(staker: &signer, rapper_token: Object<Token>) // Remove module_owner
- object::transfer(module_owner, rapper_token, staker_addr);
+ object::transfer_raw(@battle_addr, object::object_address(&rapper_token), staker_addr); // If possible, or redesign custody
Updates

Lead Judging Commences

bube Lead Judge 16 days ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

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