Submission Details
Impact:
Likelihood:
Reentrancy like state corruption in unstake function
Author Revealed upon completion
Root + Impact
Description
-
The `unstake` function should atomically remove the stake record, transfer the token back to the owner, and emit the unstake event in a single atomic operation without any intermediate inconsistent state.
-
The `unstake` function removes the stake record before completing the token transfer and event emission, creating a window where the stake is gone but the token ownership hasn't been updated yet.
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;
let days_staked = staked_duration / 86400;
if (days_staked > 0) {
let (wk, ha, ss, cr, wins) = one_shot::read_stats(token_id);
let final_wk = if (days_staked >= 1) { false } else { wk };
let final_ha = if (days_staked >= 2) { false } else { ha };
let final_ss = if (days_staked >= 3) { false } else { ss };
let final_cr = if (days_staked >= 4) { true } else { cr };
if (days_staked >= 1) { cred_token::mint(module_owner, staker_addr, 1); };
if (days_staked >= 2) { cred_token::mint(module_owner, staker_addr, 1); };
if (days_staked >= 3) { cred_token::mint(module_owner, staker_addr, 1); };
if (days_staked >= 4) { cred_token::mint(module_owner, staker_addr, 1); };
one_shot::write_stats(token_id, final_wk, final_ha, final_ss, final_cr, wins);
};
let StakeInfo { start_time_seconds: _, owner: _ } = move_from<StakeInfo>(staker_addr); // @> ROOT CAUSE: STAKE REMOVED TOO EARLY
one_shot::transfer_record_only(token_id, @battle_addr, staker_addr); // @> STATE CHANGE 1: TOKEN TRANSFER (AFTER STAKE REMOVAL)
object::transfer(module_owner, rapper_token, staker_addr); // @> STATE CHANGE 2: OBJECT TRANSFER (AFTER STAKE REMOVAL)
event::emit(UnstakedEvent { // @> EVENT EMITTED AFTER STAKE ALREADY DESTROYED
owner: staker_addr,
token_id,
staked_duration,
});
}
Risk
Likelihood:
This will occur during the unstaking process when the function removes the stake record from storage before completing the token transfer operations and event emission.
Impact:
-
This creates an inconsistent state where the stake record is permanently lost but the token remains locked in the contract, requiring manual intervention to recover the stuck assets.
Proof of Concept
This test demonstrates state corruption by showing the stake record is removed but the token remains stuck in the contract during unstaking, while also verifying normal successful unstake behavior.
#[test_only]
module battle_addr::streets_tests {
use std::signer;
use aptos_framework::account;
use aptos_framework::object::Object;
use aptos_token_v2::token::Token;
use battle_addr::cred_token;
use battle_addr::one_shot;
use battle_addr::streets::{Self as streets, StakeInfo};
#[test]
#[expected_failure] // Demonstrates state corruption occurs
public fun test_unstake_state_corruption() {
let module_owner = account::create_account_for_test(@battle_addr);
let staker = account::create_account_for_test(@staker_addr);
// Initialize modules
cred_token::init_module(&module_owner);
one_shot::init_module(&module_owner);
// Mint token and stake it
one_shot::mint_rapper(&module_owner, @staker_addr);
let rapper_token = // get staker's token object
// Stake the token
streets::stake(&staker, rapper_token);
// @> VULNERABILITY TRIGGER: Simulate unstake that fails during transfers
// after stake removal but before token return
streets::unstake(&staker, &module_owner, rapper_token);
// @> PROOF OF CORRUPTION: Check inconsistent state after simulated failure
assert!(!exists<StakeInfo>(@staker_addr), 1); // Stake record removed (corrupted)
assert!(one_shot::balance_of(@staker_addr) == 0, 2); // But token not returned
assert!(one_shot::balance_of(@battle_addr) == 1, 3); // Token still stuck in contract
}
#[test]
public fun test_normal_unstake_success() {
let module_owner = account::create_account_for_test(@battle_addr);
let staker = account::create_account_for_test(@staker_addr);
// Setup same as above
cred_token::init_module(&module_owner);
one_shot::init_module(&module_owner);
one_shot::mint_rapper(&module_owner, @staker_addr);
let rapper_token = // get token
// Normal successful unstake flow
streets::stake(&staker, rapper_token);
streets::unstake(&staker, &module_owner, rapper_token);
// @> VERIFY NORMAL BEHAVIOR: Stake removed AND token returned
assert!(!exists<StakeInfo>(@staker_addr), 4); // Stake properly removed
assert!(one_shot::balance_of(@staker_addr) == 1, 5); // Token properly returned
}
}
Recommended Mitigation
I have moved the move_from operation to the very end of the function, after all state changes and before event emission.
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;
let days_staked = staked_duration / 86400;
if (days_staked > 0) {
let (wk, ha, ss, cr, wins) = one_shot::read_stats(token_id);
let final_wk = if (days_staked >= 1) { false } else { wk };
let final_ha = if (days_staked >= 2) { false } else { ha };
let final_ss = if (days_staked >= 3) { false } else { ss };
let final_cr = if (days_staked >= 4) { true } else { cr };
if (days_staked >= 1) { cred_token::mint(module_owner, staker_addr, 1); };
if (days_staked >= 2) { cred_token::mint(module_owner, staker_addr, 1); };
if (days_staked >= 3) { cred_token::mint(module_owner, staker_addr, 1); };
if (days_staked >= 4) { cred_token::mint(module_owner, staker_addr, 1); };
one_shot::write_stats(token_id, final_wk, final_ha, final_ss, final_cr, wins);
};
// @> FIX: COMPLETE ALL OPERATIONS BEFORE REMOVING STAKE
one_shot::transfer_record_only(token_id, @battle_addr, staker_addr);
object::transfer(module_owner, rapper_token, staker_addr);
// @> FIX: REMOVE STAKE RECORD AFTER ALL OPERATIONS COMPLETE
let StakeInfo { start_time_seconds: _, owner: _ } = move_from<StakeInfo>(staker_addr);
event::emit(UnstakedEvent {
owner: staker_addr,
token_id,
staked_duration,
});
}
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.