Submission Details
Severity:
Users will loose their NFTs after a battle ends
Root + Impact
Description
Users can place their rapper NFTs in the arena for battle. After the battle, both NFTs should be sent back to their owner, but the code only transfers the record and fails to transfer the actual object.
For every one_shot::transfer_record_only, there should be an object::transfer with the corresponding parameters, but this is not the case when transferring NFTs to owners after battle.
// in rap_battle::go_on_stage_or_battle
[...]
if (arena.defender == @0x0) {
[...]
one_shot::transfer_record_only(token_id, player_addr, @battle_addr);
@> object::transfer(player, rapper_token, @battle_addr);
[...]
} else {
[...]
one_shot::transfer_record_only(chall_token_id, chall_addr, @battle_addr);
@> object::transfer(player, rapper_token, @battle_addr);
[...]
if (winner == defender_addr) {
coin::deposit(defender_addr, pool);
one_shot::increment_wins(arena.defender_token_id);
// @audit-reported The winner steals the loser's rapper
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);
// @audit-issue The rapper_token ownership is not transferred, only the record
} 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:
This happens after every battles.
Impact:
Users won't actually own their NFTs anymore and won't be able to use them anymore.
Proof of Concept
You will first need to add test_only functions in the code:
// In cred_token:
#[test_only]
public fun mint_cred(to: address,amount: u64) acquires CredCapabilities {
let caps = borrow_global<CredCapabilities>(@battle_addr);
let coins = coin::mint(amount, &caps.mint_cap);
coin::deposit(to, coins);
}
#[test_only]
public entry fun init(account: &signer) {
init_module(account);
}
// In one_shot:
#[test_only]
public fun get_mint_rapper_event_minter(event: &MintRapperEvent): address {
event.minter
}
#[test_only]
public fun get_mint_rapper_event_token_id(event: &MintRapperEvent): address {
event.token_id
}
// In rap_battle:
#[test_only]
public entry fun init(sender: &signer) {
init_module(sender);
}
Add this code in the test file:
#[test(
module_owner = @battle_addr, alice = @0x999, bob = @998, aptos_framework = @aptos_framework
)]
public fun test_rapper_not_transferred(
module_owner: signer, alice: signer, bob: signer, aptos_framework: signer
) {
// Initialize the test framework
timestamp::set_time_has_started_for_testing(&aptos_framework);
aptos_coin::ensure_initialized_with_apt_fa_metadata_for_test();
let alice_address = signer::address_of(&alice);
let bob_address = signer::address_of(&bob);
// Initialize modules
cred_token::init(&module_owner);
rap_battle::init(&module_owner);
// Both users get one rapper
one_shot::mint_rapper(&module_owner, alice_address);
let events = event::emitted_events<one_shot::MintRapperEvent>();
let event = events.borrow(0);
let alice_rapper = one_shot::get_mint_rapper_event_token_id(event);
one_shot::mint_rapper(&module_owner, bob_address);
let events = event::emitted_events<one_shot::MintRapperEvent>();
let event = events.borrow(1);
let bob_rapper = one_shot::get_mint_rapper_event_token_id(event);
rap_battle::go_on_stage_or_battle(&alice, object::address_to_object<token::Token>(alice_rapper), 0);
rap_battle::go_on_stage_or_battle(&bob, object::address_to_object<token::Token>(bob_rapper), 0);
// Alice can't go on stage anymore (this call aborts)
rap_battle::go_on_stage_or_battle(&alice, object::address_to_object<token::Token>(alice_rapper), 0);
}
Recommended Mitigation
Add object::transfer to owners after a battle ends.
Updates
Lead Judging Commences
bube 20 days ago
Submission Judgement Published Assigned finding tags:
The tokens are not returned back to the players after the battle
Appeal created
0xrektified
20 days ago
bube
19 days ago
bube 18 days ago
Submission Judgement Published Assigned finding tags:
The tokens are not returned back to the players after the battle
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.