Submission Details
Impact:
Likelihood:
Wrong ownership transfer after battle
Author Revealed upon completion
Wrong ownership transfer after battle
Once the battle is finished, the token ownership should be updated and each user takes back their rapper. However, due to incorrect logic, the ownership of both tokes in transferred to winner.
Risk
Likelihood: High
Affects all completed battles
Impact: High
Those who lose the battle also lose rapper ownership.
Proof of Concept
[test]
public fun test_prove_winner_takes_both_rappers () {
let module_owner = account::create_account_for_test();
let framework = account::create_account_for_test(@0x1);
let user1 = account::create_account_for_test(@0xA);
let user2 = account::create_account_for_test(@0xB);
rap_battle::initialize_for_test(&module_owner);
timestamp::set_time_has_started_for_testing(&framework);
cred_token::initialize_for_test(&module_owner, &framework);
cred_token::register(&user1);
cred_token::register(&user2);
cred_token::mint(&module_owner, signer::address_of(&user1), 1000);
cred_token::mint(&module_owner, signer::address_of(&user2), 1000);
one_shot::mint_rapper(&module_owner, signer::address_of(&user1));
one_shot::mint_rapper(&module_owner, signer::address_of(&user2));
let rappers = one_shot::get_tokens();
assert!(vector::length(&rappers) == 2);
let rapper_1_address = vector::borrow(&rappers, 0);
let rapper1 = object::address_to_object<token::Token>(*rapper_1_address);
let rapper_2_address = vector::borrow(&rappers, 1);
let rapper2 = object::address_to_object<token::Token>(*rapper_2_address);
rap_battle::go_on_stage_or_battle(&user1, rapper1, 500);
let (defender, defender_bet, defender_token_id, prize_pool) = rap_battle::get_arena();
assert!(defender == signer::address_of(&user1));
assert!(defender_bet == 500);
assert!(defender_token_id == *rapper_1_address);
assert!(prize_pool == 500);
rap_battle::go_on_stage_or_battle(&user2, rapper2, 500);
let (defender, defender_bet, defender_token_id, prize_pool) = rap_battle::get_arena();
assert!(defender == @0x0);
assert!(defender_bet == 0);
assert!(defender_token_id == @0x0);
assert!(prize_pool == 0);
let (_, _, _, _, battles_won) = one_shot::read_stats(*rapper_1_address);
assert!(battles_won == 1);
assert!(cred_token::balance_of(signer::address_of(&user1)) == 1500);
let (_, _, _, _, battles_won) = one_shot::read_stats(*rapper_2_address);
assert!(battles_won == 0);
assert!(cred_token::balance_of(signer::address_of(&user2)) == 500);
assert!(object::owner(rapper1) == signer::address_of(&module_owner));
assert!(object::owner(rapper2) == signer::address_of(&module_owner));
assert!(one_shot::get_token_owner(*rapper_1_address) == signer::address_of(&user1));
assert!(one_shot::get_token_owner(*rapper_2_address) == signer::address_of(&user1));
}
The test shows that after the battle, both tokens are owned by the winner (user1). It is known limitation that the actual token is not transferred after battle.
Recommended Mitigation
Fix the ownership transfer logic
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.