One Shot: Reloaded

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

NFT cannot enter battle after is transferred to another legit owner

Author Revealed upon completion

NFT cannot enter battle after is transferred to another legit owner

Description

A Rapper NFT owner should be able to enter a battle with their Rapper and have the protocol resolve the match and payout correctly. However, when an NFT is transferred to a legitimate external owner, the battle enter path aborts, effectively preventing a legitimate owned NFT from entering a battle.

Risk

Likelihood: High

Affects all NFTs that have been transferred to other legitimate users.

Impact: High

NFTs that have been transferred to other legitimate users cannot use the game. Once an NFT is transferred, the effect is as it is DoSed.

Proof of Concept

#[test]
#[expected_failure]
public fun test_can_battle_with_transferred_token () {
let module_owner = account::create_account_for_test(@battle_addr);
let framework = account::create_account_for_test(@0x1);
let user1 = account::create_account_for_test(@0xA);
let user2 = account::create_account_for_test(@0xB);
let user3 = account::create_account_for_test(@0xC);
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::register(&user3);
cred_token::mint(&module_owner, signer::address_of(&user1), 1000);
cred_token::mint(&module_owner, signer::address_of(&user2), 1000);
cred_token::mint(&module_owner, signer::address_of(&user3), 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);
object::transfer(&user1, rapper1, signer::address_of(&user3));
assert!(object::owner(rapper1) == signer::address_of(&user3));
rap_battle::go_on_stage_or_battle(&user3, rapper1, 500);
rap_battle::go_on_stage_or_battle(&user2, rapper2, 500);
}

The test demonstrates the scenario in which rapper1 is transferred to user3, and user3 tries to enter battle with the game aborting.

Mitigation

The root cause of the issue is on one_shot contract which keeps in stats the NFT owner and performs some logic around it.
Alternatively the object::owner method can be used to check ownership realtime.

- public(friend) fun transfer_record_only(token_id: address, from: address, to: address)
+ public(friend) fun transfer_record_only(token_id: address, to: address)
acquires RapperStats {
let stats_res = borrow_global_mut<RapperStats>(@battle_addr);
let s = table::borrow_mut(&mut stats_res.stats, token_id);
+ let from: address = object::owner(object::address_to_object<token::Token>(token_id))
// rest of the code is fine

Support

FAQs

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