Submission Details
Impact:
Likelihood:
The winner takes both NFT records in battle
Author Revealed upon completion
Root + Impact
Description
-
Normal behavior: After the battle is over the winner should receive the
CREDprize and a win increment. Each player should retain the ownership of their own Rapper NFT -
Issue: The winner branch reassigns both the defender’s and the challenger’s Rapper records to the winner which transferz logical ownership of the losers NFT too
// Root cause in the codebase with @> marks to highlight the relevant section
let pool = coin::extract_all(&mut arena.prize_pool);
if (winner == defender_addr) {
coin::deposit(defender_addr, pool);
one_shot::increment_wins(arena.defender_token_id);
one_shot::transfer_record_only(arena.defender_token_id, @battle_addr, defender_addr);
@> one_shot::transfer_record_only(chall_token_id, @battle_addr, defender_addr);
} else {
coin::deposit(chall_addr, pool);
one_shot::increment_wins(chall_token_id);
@> one_shot::transfer_record_only(arena.defender_token_id, @battle_addr, chall_addr);
one_shot::transfer_record_only(chall_token_id, @battle_addr, chall_addr);
};
Risk
Likelihood:
-
Triggers each and every time a battle resolves
-
There is no ownership guard that prevents this
Impact:
-
The winner gets the ownership of the opponents Rapper NFT
-
Impact 2
Proof of Concept
#[test_only]
module battle_addr::nft_ownership_theft_test {
use std::signer;
use aptos_framework::account;
use aptos_framework::timestamp;
use aptos_framework::coin;
use aptos_framework::object;
use aptos_token_v2::token::Token;
use battle_addr::rap_battle;
use battle_addr::one_shot;
use battle_addr::cred_token::{Self, CRED};
#[test(framework = @0x1, module_owner = @battle_addr, defender = @0x100, challenger = @0x200)]
fun test_winner_steals_loser_nft_ownership(
framework: &signer,
module_owner: &signer,
defender: &signer,
challenger: &signer
) {
// Setup
timestamp::set_time_has_started_for_testing(framework);
account::create_account_for_test(signer::address_of(module_owner));
account::create_account_for_test(signer::address_of(defender));
account::create_account_for_test(signer::address_of(challenger));
// Initialize modules
rap_battle::init_module(module_owner);
one_shot::init_module(module_owner);
cred_token::init_module(module_owner);
// Register and fund both players
cred_token::register(defender);
cred_token::register(challenger);
cred_token::mint(module_owner, signer::address_of(defender), 1000);
cred_token::mint(module_owner, signer::address_of(challenger), 1000);
// Mint rapper NFTs
one_shot::mint_rapper(module_owner, signer::address_of(defender));
one_shot::mint_rapper(module_owner, signer::address_of(challenger));
let def_addr = signer::address_of(defender);
let chall_addr = signer::address_of(challenger);
// Verify initial ownership
assert!(one_shot::balance_of(def_addr) == 1, 1);
assert!(one_shot::balance_of(chall_addr) == 1, 2);
// Get token objects
let defender_token = create_test_token(defender);
let challenger_token = create_test_token(challenger);
// Defender goes on stage
rap_battle::go_on_stage_or_battle(defender, defender_token, 100);
// Verify ownership during staging
assert!(one_shot::balance_of(@battle_addr) == 1, 3); // Defenders token held by battle
assert!(one_shot::balance_of(def_addr) == 0, 4); // Defender temporarily has 0
assert!(one_shot::balance_of(chall_addr) == 1, 5); // Challenger still has 1
timestamp::update_global_time_for_test(150 * 1000000); // 150 % 100 = 50, challenger wins
rap_battle::go_on_stage_or_battle(challenger, challenger_token, 100);
assert!(one_shot::balance_of(chall_addr) == 2, 6); // Challenger now owns 2 NFTs
assert!(one_shot::balance_of(def_addr) == 0, 7); // Defender lost their NFT ownership
assert!(one_shot::balance_of(@battle_addr) == 0, 8); // Battle arena empty
}
#[test(framework = @0x1, module_owner = @battle_addr, defender = @0x100, challenger = @0x200)]
fun test_defender_wins_steals_challenger_nft(
framework: &signer,
module_owner: &signer,
defender: &signer,
challenger: &signer
) {
// Setup (same as above)
timestamp::set_time_has_started_for_testing(framework);
account::create_account_for_test(signer::address_of(module_owner));
account::create_account_for_test(signer::address_of(defender));
account::create_account_for_test(signer::address_of(challenger));
rap_battle::init_module(module_owner);
one_shot::init_module(module_owner);
cred_token::init_module(module_owner);
cred_token::register(defender);
cred_token::register(challenger);
cred_token::mint(module_owner, signer::address_of(defender), 1000);
cred_token::mint(module_owner, signer::address_of(challenger), 1000);
one_shot::mint_rapper(module_owner, signer::address_of(defender));
one_shot::mint_rapper(module_owner, signer::address_of(challenger));
let def_addr = signer::address_of(defender);
let chall_addr = signer::address_of(challenger);
// Initial state
assert!(one_shot::balance_of(def_addr) == 1, 1);
assert!(one_shot::balance_of(chall_addr) == 1, 2);
let defender_token = create_test_token(defender);
let challenger_token = create_test_token(challenger);
// Defender stages
rap_battle::go_on_stage_or_battle(defender, defender_token, 100);
// Set timestamp so defender wins
timestamp::update_global_time_for_test(125 * 1000000);
// Challenger battles and loses
rap_battle::go_on_stage_or_battle(challenger, challenger_token, 100);
assert!(one_shot::balance_of(def_addr) == 2, 3); // Defender now owns both
assert!(one_shot::balance_of(chall_addr) == 0, 4); // Challenger lost ownership
assert!(coin::balance<CRED>(def_addr) == 1100, 5); // 1000 initial + 200 prize - 100 bet
}
#[test(framework = @0x1, module_owner = @battle_addr, player1 = @0x100, player2 = @0x200, player3 = @0x300)]
fun test_nft_accumulation_exploit(
framework: &signer,
module_owner: &signer,
player1: &signer,
player2: &signer,
player3: &signer
) {
// Setup
timestamp::set_time_has_started_for_testing(framework);
account::create_account_for_test(signer::address_of(module_owner));
account::create_account_for_test(signer::address_of(player1));
account::create_account_for_test(signer::address_of(player2));
account::create_account_for_test(signer::address_of(player3));
rap_battle::init_module(module_owner);
one_shot::init_module(module_owner);
cred_token::init_module(module_owner);
// Setup all players
let players = vector[player1, player2, player3];
let i = 0;
while (i < 3) {
let player = *vector::borrow(&players, i);
cred_token::register(player);
cred_token::mint(module_owner, signer::address_of(player), 1000);
one_shot::mint_rapper(module_owner, signer::address_of(player));
i = i + 1;
};
let p1_addr = signer::address_of(player1);
// Player1 will accumulate all NFTs by winning battles
// Battle 1: Player1 vs Player2
let p1_token = create_test_token(player1);
let p2_token = create_test_token(player2);
rap_battle::go_on_stage_or_battle(player2, p2_token, 100);
timestamp::update_global_time_for_test(150 * 1000000); // Ensure challenger wins
rap_battle::go_on_stage_or_battle(player1, p1_token, 100);
assert!(one_shot::balance_of(p1_addr) == 2, 1);
// Battle 2: Player1 vs Player3
// Player1 uses their original token again
let p1_token2 = create_test_token(player1);
let p3_token = create_test_token(player3);
rap_battle::go_on_stage_or_battle(player3, p3_token, 100);
timestamp::update_global_time_for_test(250 * 1000000); // Ensure challenger wins
rap_battle::go_on_stage_or_battle(player1, p1_token2, 100);
// Player1 now owns 3 NFTs (accumulating all defeated opponents)
assert!(one_shot::balance_of(p1_addr) == 3, 2);
assert!(one_shot::balance_of(signer::address_of(player2)) == 0, 3);
assert!(one_shot::balance_of(signer::address_of(player3)) == 0, 4);
}
fun create_test_token(owner: &signer): object::Object<Token> {
abort 99
}
}
Recommended Mitigation
@@
- if (winner == defender_addr) {
+ if (winner == defender_addr) {
coin::deposit(defender_addr, pool);
one_shot::increment_wins(arena.defender_token_id);
one_shot::transfer_record_only(arena.defender_token_id, @battle_addr, defender_addr);
- one_shot::transfer_record_only(chall_token_id, @battle_addr, defender_addr);
+ one_shot::transfer_record_only(chall_token_id, @battle_addr, chall_addr);
} else {
coin::deposit(chall_addr, pool);
one_shot::increment_wins(chall_token_id);
- one_shot::transfer_record_only(arena.defender_token_id, @battle_addr, chall_addr);
+ one_shot::transfer_record_only(arena.defender_token_id, @battle_addr, defender_addr);
one_shot::transfer_record_only(chall_token_id, @battle_addr, chall_addr);
};
Rewards breakdown
nSLOC
436This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.
Rewards Distribution
The contest is complete and the rewards are being distributed.