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

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

s3mvl4d

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,
+ });
+ }
}
​
Updates

Lead Judging Commences

bube Lead Judge 17 days ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

The defender can't cancel the battle if there is no challenger

There is no security impact on the protocol from this issue. The defender should wait until the challenger joins, this is intended behavior.

Support

FAQs

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