One Shot: Reloaded

First Flight #47
Beginner FriendlyNFT
100 EXP
Submission Details
Impact: medium
Likelihood: medium

No Cancel Path: Defender’s NFT and Bet Locked Indefinitely

Author Revealed upon completion

Root + Impact

Description

  • Under normal operation, a defender is expected to “take the stage” by transferring a Rapper NFT and placing a CRED bet; then a challenger with an equal bet is expected to arrive, after which a winner is determined and the prize pool is paid out. The arena state is cleared only after a completed battle.

  • It was observed that no path exists to cancel or exit the “on stage” state before a challenger arrives. When go_on_stage_or_battle is called on the empty arena, the defender’s NFT is transferred into custody of @battle_addr and the defender’s bet is withdrawn and merged into prize_pool. No entry function is exposed to return the NFT or refund the bet without a challenger, creating a liveness risk and indefinite lock of user assets. The README also states that NFTs are custodied at @battle_addr during battles.

// Root cause in the codebase with @> marks to highlight the relevant section
module battle_addr::rap_battle {
use aptos_framework::object::{Self as object, Object};
use aptos_framework::coin::{Self as coin, Coin};
use aptos_token_v2::token::Token;
use battle_addr::one_shot;
struct BattleArena has key {
defender: address,
defender_bet: u64,
defender_token_id: address,
prize_pool: Coin<cred_token::CRED>,
} // No timeout / cancel path stored in state. @>
public entry fun go_on_stage_or_battle(
player: &signer,
rapper_token: Object<Token>,
bet_amount: u64
) acquires BattleArena {
let arena = borrow_global_mut<BattleArena>(@battle_addr);
if (arena.defender == @0x0) {
// Defender path:
arena.defender = signer::address_of(player); // @>
arena.defender_bet = bet_amount; // @>
let token_id = object::object_address(&rapper_token);
arena.defender_token_id = token_id; // @>
let first_bet = coin::withdraw<cred_token::CRED>(player, bet_amount);
coin::merge(&mut arena.prize_pool, first_bet); // Bet locked in prize_pool. @>
one_shot::transfer_record_only(token_id, signer::address_of(player), @battle_addr);
object::transfer(player, rapper_token, @battle_addr); // NFT transferred to @battle_addr custody. @>
// No way to unwind this branch without a challenger. @>
} else {
// Challenger path, after which arena is reset to empty.
// ...
}
}
// No `cancel`, `withdraw`, `unwind`, or timeout-based exit function is defined. @>
}

Risk

Likelihood:

  • Protocol usage patterns where challengers do not arrive (low activity periods, UI downtime, or fragmented liquidity) will leave defenders “on stage” indefinitely because no alternative transition exists.

  • Any mismatch of common bet sizes in the ecosystem (e.g., most users bet different amounts) will effectively prevent matching, causing the same indefinite on-stage state.

Impact:

  • Liveness failure / asset lock: The defender’s NFT remains in custody of @battle_addr, and the defender’s CRED bet remains in prize_pool, with no protocol-native mechanism to recover them without a challenger.

  • Operational burden / centralization pressure: Because actual transfers out from @battle_addr require its signer, recovery becomes a manual/centralized operation, increasing trust and support overhead.

Proof of Concept

Minimal PoC sequence:

1) Defender prepares wallet with a Rapper NFT and some CRED.

2) Defender calls rap_battle::go_on_stage_or_battle(player, rapper_token, 100)

// Effects from code (defender branch):
// - arena.defender = defender_addr
// - arena.defender_bet = 100
// - arena.defender_token_id = <token_id>
// - coin::withdraw<cred_token::CRED>(player, 100) merged into arena.prize_pool
// - object::transfer(player, rapper_token, @battle_addr) // NFT custody -> @battle_addr
// (No subsequent entry function exists to cancel, refund, or return the NFT.)
//
// 3) Time passes; no challenger arrives.
// 4) Defender scans module interface: only entry point is go_on_stage_or_battle.
// No cancel/withdraw entry function is available -> assets remain locked.

Recommended Mitigation

A cancel/exit mechanism should be introduced to guarantee progress. Two complementary improvements are recommended:

  1. Add a cancel_stage entry function that can be called only by the current defender (and optionally only after a cooldown). This function should:

    • Refund the entire prize_pool to the defender.

    • Return the defender’s NFT from @battle_addr to the defender (both the internal record and the physical object transfer).

    • Clear the arena state.

    • Emit a StageCancelled event.
      Because @battle_addr is the custodian, the function must perform the physical transfer using an authorized signer (e.g., module_owner: &signer whose address equals @battle_addr), consistent with how streets::unstake handles transfers out of custody.

  2. (Optional) Add a timeout / expiry so that, after N seconds on stage without a match, cancel_stage becomes callable (or can be executed permissionlessly to auto-unwind in favor of the defender).

module battle_addr::rap_battle {
+ use aptos_framework::event;
+ use std::signer;
+ use aptos_framework::timestamp;
+ // ...existing uses...
- const E_BATTLE_ARENA_OCCUPIED: u64 = 1;
- const E_BETS_DO_NOT_MATCH: u64 = 2;
+ const E_BATTLE_ARENA_OCCUPIED: u64 = 1;
+ const E_BETS_DO_NOT_MATCH: u64 = 2;
+ const E_NO_DEFENDER_ON_STAGE: u64 = 3;
+ const E_NOT_DEFENDER: u64 = 4;
+ const E_BAD_MODULE_OWNER: u64 = 5;
+ // Optional: grace period before cancel is allowed
+ const CANCEL_COOLDOWN_SECS: u64 = 300;
struct BattleArena has key {
defender: address,
defender_bet: u64,
defender_token_id: address,
prize_pool: Coin<cred_token::CRED>,
}
+ #[event]
+ struct StageCancelled has drop, store {
+ defender: address,
+ token_id: address,
+ refunded_cred: u64,
+ }
public entry fun go_on_stage_or_battle(
player: &signer,
rapper_token: Object<Token>,
bet_amount: u64
) acquires BattleArena {
// ...existing logic...
}
+ /// Allows the current defender to exit if no challenger has matched the bet.
+ /// Requires module owner signer to physically transfer NFT out of @battle_addr.
+ public entry fun cancel_stage(
+ defender: &signer,
+ module_owner: &signer,
+ // Optional: pass block time check externally if desired
+ ) acquires BattleArena {
+ let arena = borrow_global_mut<BattleArena>(@battle_addr);
+ assert!(arena.defender != @0x0, E_NO_DEFENDER_ON_STAGE);
+ let defender_addr = signer::address_of(defender);
+ assert!(arena.defender == defender_addr, E_NOT_DEFENDER);
+ assert!(signer::address_of(module_owner) == @battle_addr, E_BAD_MODULE_OWNER);
+
+ // Optional cooldown to reduce spam / griefing
+ // assert!(timestamp::now_seconds() - arena.started_at >= CANCEL_COOLDOWN_SECS, E_COOLDOWN);
+
+ // 1) Refund all CRED that was escrowed as the defender's initial bet
+ let refunded = coin::extract_all(&mut arena.prize_pool);
+ let refunded_amt = coin::value<&cred_token::CRED>(&refunded);
+ coin::deposit(defender_addr, refunded);
+
+ // 2) Return NFT (update registry + physical transfer from @battle_addr)
+ one_shot::transfer_record_only(arena.defender_token_id, @battle_addr, defender_addr);
+ // NOTE: a helper may be needed to obtain the Object<Token> from its id.
+ // If available, perform: object::transfer(module_owner, <rapper_token>, defender_addr);
+
+ // 3) Reset arena
+ let token_id = arena.defender_token_id;
+ arena.defender = @0x0;
+ arena.defender_bet = 0;
+ arena.defender_token_id = @0x0;
+
+ event::emit(StageCancelled {
+ defender: defender_addr,
+ token_id,
+ refunded_cred: refunded_amt,
+ });
+ }
}

Support

FAQs

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