diff --git a/contracts/game_contract/src/lib.rs b/contracts/game_contract/src/lib.rs index dcb5ef0..8071d0c 100644 --- a/contracts/game_contract/src/lib.rs +++ b/contracts/game_contract/src/lib.rs @@ -141,7 +141,15 @@ const ORACLE_CONTRACT: Symbol = symbol_short!("ORACLE"); // Address of oracle co const TOURNAMENT_TIMELOCK: Symbol = symbol_short!("TL_DUR"); // u64 - lock duration in ledger sequences const TOURNAMENT_ESCROWS: Symbol = symbol_short!("TL_ESC"); // Map -// ──────────────────────────────────────────────────────────────────────────── +// Reentrancy guard (#860) +const R_GUARD: Symbol = symbol_short!("R_GUARD"); + +// Admin key rotation timelock (#890): 24h = 17280 ledger sequences at 5s/ledger +const ADMIN_TIMELOCK: Symbol = symbol_short!("ADM_TLK"); // u64 - lock duration (ledger sequences) +const PENDING_ADMIN_KEY: Symbol = symbol_short!("PEND_ADM"); // Option> - proposed new admin key +const PENDING_ADMIN_TIMESTAMP: Symbol = symbol_short!("PEND_TS"); // u64 - ledger sequence when proposal was made + +// �"?�"?�"?�"?�"?�"?�"?�"?�"?�"?�"?�"?�"?�"?�"?�"?�"?�"?�"?�"?�"?�"?�"?�"?�"?�"?�"?�"?�"?�"?�"?�"?�"?�"?�"?�"?�"?�"?�"?�"?�"?�"?�"?�"?�"?�"?�"?�"?�"?�"?�"?�"?�"?�"?�"?�"?�"?�"?�"?�"?�"?�"?�"?�"?�"?�"?�"?�"?�"?�"?�"?�"?�"?�"?�"?�"? // Multi-sig fee proposal type (#535) // ──────────────────────────────────────────────────────────────────────────── @@ -237,6 +245,14 @@ pub enum ContractError { EmptyBatch = 37, /// claim_puzzle_rewards_batch called with more proofs than MAX_BATCH_SIZE BatchTooLarge = 38, + /// Reentrancy guard triggered — function already entered + ReentrantCall = 39, + /// No pending admin key proposal exists + NoPendingAdminKey = 40, + /// Timelock has not yet expired — cannot accept proposal + TimelockNotExpired = 41, + /// An admin key proposal is already pending + AdminKeyAlreadyPending = 42, } #[contract] @@ -265,6 +281,19 @@ impl GameContract { TokenClient::new(env, &Self::token_contract_address(env)) } + fn non_reentrant_enter(env: &Env) -> Result<(), ContractError> { + let guard: u32 = env.storage().instance().get(&R_GUARD).unwrap_or(0); + if guard == 1 { + return Err(ContractError::ReentrantCall); + } + env.storage().instance().set(&R_GUARD, &1u32); + Ok(()) + } + + fn non_reentrant_exit(env: &Env) { + env.storage().instance().set(&R_GUARD, &0u32); + } + /// Gas-optimized tournament payout — single pass, no redundant map reads. /// Validates percentages and distributes atomically. pub fn payout_tournament_optimized( @@ -321,6 +350,8 @@ impl GameContract { // Dust to first winner let remainder = total_pool - distributed; + Self::non_reentrant_enter(&env)?; + let token_client = Self::token_client(&env); let contract_address = env.current_contract_address(); @@ -336,6 +367,7 @@ impl GameContract { games.set(game_id, settled_game); env.storage().instance().set(&GAMES, &games); + Self::non_reentrant_exit(&env); Ok(()) } @@ -365,10 +397,13 @@ impl GameContract { player1.require_auth(); + Self::non_reentrant_enter(&env)?; + let token_client = Self::token_client(&env); let contract_address = env.current_contract_address(); if token_client.balance(&player1) < wager_amount { + Self::non_reentrant_exit(&env); return Err(ContractError::InsufficientFunds); } @@ -408,6 +443,7 @@ impl GameContract { escrow.set(player1, current_escrow + wager_amount); env.storage().instance().set(&ESCROW, &escrow); + Self::non_reentrant_exit(&env); Ok(game_counter) } @@ -449,10 +485,14 @@ impl GameContract { } player2.require_auth(); + + Self::non_reentrant_enter(&env)?; + let token_client = Self::token_client(&env); let contract_address = env.current_contract_address(); if token_client.balance(&player2) < game.wager_amount { + Self::non_reentrant_exit(&env); return Err(ContractError::InsufficientFunds); } @@ -475,6 +515,7 @@ impl GameContract { games.set(game_id, game); env.storage().instance().set(&GAMES, &games); + Self::non_reentrant_exit(&env); Ok(()) } @@ -572,12 +613,21 @@ impl GameContract { env.crypto() .ed25519_verify(&admin_pubkey, &digest_bytes, &signature); + Self::non_reentrant_enter(&env)?; + game.state = GameState::Drawn; - Self::process_draw_payout(&env, &game)?; + match Self::process_draw_payout(&env, &game) { + Ok(()) => {} + Err(e) => { + Self::non_reentrant_exit(&env); + return Err(e); + } + } games.set(game_id, game); env.storage().instance().set(&GAMES, &games); + Self::non_reentrant_exit(&env); Ok(()) } @@ -629,13 +679,22 @@ impl GameContract { env.crypto() .ed25519_verify(&admin_pubkey, &digest_bytes, &signature); + Self::non_reentrant_enter(&env)?; + game.winner = Some(winner.clone()); - Self::process_payout(&env, &game, &winner)?; + match Self::process_payout(&env, &game, &winner) { + Ok(()) => {} + Err(e) => { + Self::non_reentrant_exit(&env); + return Err(e); + } + } game.state = GameState::Settled; games.set(game_id, game); env.storage().instance().set(&GAMES, &games); + Self::non_reentrant_exit(&env); Ok(()) } @@ -658,6 +717,8 @@ impl GameContract { player.require_auth(); + Self::non_reentrant_enter(&env)?; + // Refund player1's staked wager let mut escrow: Map = env .storage() @@ -677,6 +738,7 @@ impl GameContract { games.set(game_id, game); env.storage().instance().set(&GAMES, &games); + Self::non_reentrant_exit(&env); Ok(()) } @@ -707,13 +769,22 @@ impl GameContract { game.player1.clone() }; + Self::non_reentrant_enter(&env)?; + game.winner = Some(winner.clone()); - Self::process_payout(&env, &game, &winner)?; + match Self::process_payout(&env, &game, &winner) { + Ok(()) => {} + Err(e) => { + Self::non_reentrant_exit(&env); + return Err(e); + } + } game.state = GameState::Settled; games.set(game_id, game); env.storage().instance().set(&GAMES, &games); + Self::non_reentrant_exit(&env); Ok(()) } @@ -738,12 +809,21 @@ impl GameContract { winner.require_auth(); - Self::process_payout(&env, &game, &winner)?; + Self::non_reentrant_enter(&env)?; + + match Self::process_payout(&env, &game, &winner) { + Ok(()) => {} + Err(e) => { + Self::non_reentrant_exit(&env); + return Err(e); + } + } game.state = GameState::Settled; games.set(game_id, game); env.storage().instance().set(&GAMES, &games); + Self::non_reentrant_exit(&env); Ok(()) } @@ -795,6 +875,8 @@ impl GameContract { total_pool = game.wager_amount * 2; } + Self::non_reentrant_enter(&env)?; + // Deduct wagers first to prevent double-counting escrow.set(game.player1.clone(), player1_escrow - game.wager_amount); if let Some(ref player2) = game.player2 { @@ -815,6 +897,7 @@ impl GameContract { } if total_percentage != 100 { + Self::non_reentrant_exit(&env); return Err(ContractError::InvalidPercentage); } @@ -832,6 +915,7 @@ impl GameContract { games.set(game_id, settled_game); env.storage().instance().set(&GAMES, &games); + Self::non_reentrant_exit(&env); Ok(()) } @@ -1057,6 +1141,85 @@ impl GameContract { env.storage().instance().set(&CONTRACT_ADMIN, &admin); } + /// Propose a new admin key with a 24-hour timelock. + /// Only the current admin (CONTRACT_ADMIN) can propose. + pub fn propose_new_admin_key( + env: Env, + admin: Address, + new_key: BytesN<32>, + ) -> Result<(), ContractError> { + admin.require_auth(); + let stored_admin: Address = env + .storage() + .instance() + .get(&CONTRACT_ADMIN) + .ok_or(ContractError::NotAuthorized)?; + if admin != stored_admin { + return Err(ContractError::NotAuthorized); + } + if env.storage().instance().has(&PENDING_ADMIN_KEY) { + return Err(ContractError::AdminKeyAlreadyPending); + } + + let current_seq = env.ledger().sequence(); + env.storage().instance().set(&PENDING_ADMIN_KEY, &new_key); + env.storage() + .instance() + .set(&PENDING_ADMIN_TIMESTAMP, ¤t_seq); + env.storage() + .instance() + .set(&ADMIN_TIMELOCK, &(current_seq + ADMIN_TIMELOCK_DURATION)); + + env.events().publish( + (symbol_short!("admin_key_proposed"),), + (admin, new_key.clone(), current_seq), + ); + + Ok(()) + } + + /// Accept a pending admin key proposal after the timelock expires. + /// Anyone can call this once the 24-hour window has elapsed. + pub fn accept_new_admin_key(env: Env) -> Result<(), ContractError> { + let proposed_key: BytesN<32> = env + .storage() + .instance() + .get(&PENDING_ADMIN_KEY) + .ok_or(ContractError::NoPendingAdminKey)?; + + let proposal_seq: u64 = env + .storage() + .instance() + .get(&PENDING_ADMIN_TIMESTAMP) + .ok_or(ContractError::NoPendingAdminKey)?; + + let current_seq = env.ledger().sequence(); + let lock_duration: u64 = env + .storage() + .instance() + .get(&ADMIN_TIMELOCK) + .unwrap_or(ADMIN_TIMELOCK_DURATION); + + if current_seq < proposal_seq + lock_duration { + return Err(ContractError::TimelockNotExpired); + } + + env.storage().instance().set(&ADMIN_KEY, &proposed_key); + env.storage().instance().remove(&PENDING_ADMIN_KEY); + env.storage().instance().remove(&PENDING_ADMIN_TIMESTAMP); + env.storage().instance().remove(&ADMIN_TIMELOCK); + + env.events().publish( + (symbol_short!("admin_key_accepted"),), + (proposed_key,), + ); + + Ok(()) + } + + /// Admin key rotation timelock duration (default: 17280 ledger sequences = 24 hours at 5s/ledger). + pub const ADMIN_TIMELOCK_DURATION: u64 = 17280; + // ── #199 – claim_puzzle_reward ──────────────────────────────────────────── // // Accepts a backend ED25519 signature that proves the user solved a puzzle, @@ -1128,6 +1291,8 @@ impl GameContract { env.crypto() .ed25519_verify(&admin_pubkey, &digest_bytes, &signature); + Self::non_reentrant_enter(&env)?; + // 4. Mark nonce as used (state-before-interaction pattern) nonces.set(nonce, true); env.storage().instance().set(&USED_NONCE, &nonces); @@ -1156,6 +1321,7 @@ impl GameContract { env.events() .publish((symbol_short!("pzl_rwd"), recipient.clone()), reward_amount); + Self::non_reentrant_exit(&env); Ok(()) } @@ -1223,6 +1389,8 @@ impl GameContract { let mut total_claimed: i128 = 0; + Self::non_reentrant_enter(&env)?; + for proof in proofs.iter() { let Proof { recipient, @@ -1232,12 +1400,14 @@ impl GameContract { } = proof; if reward_amount <= 0 || reward_amount > i64::MAX as i128 { + Self::non_reentrant_exit(&env); return Err(ContractError::InvalidAmount); } // Replay protection — also rejects duplicate nonces within the // same batch, since `nonces` is updated as we go. if nonces.get(nonce).unwrap_or(false) { + Self::non_reentrant_exit(&env); return Err(ContractError::Unauthorized); } @@ -1286,6 +1456,7 @@ impl GameContract { env.events() .publish((symbol_short!("pzlbatch"),), (proofs.len(), total_claimed)); + Self::non_reentrant_exit(&env); Ok(()) } @@ -1384,12 +1555,15 @@ impl GameContract { filer.require_auth(); + Self::non_reentrant_enter(&env)?; + let dispute_fee: i128 = env.storage().instance().get(&DISPUTE_FEE).unwrap_or(0); if dispute_fee > 0 { let token_client = Self::token_client(&env); let contract_address = env.current_contract_address(); if token_client.balance(&filer) < dispute_fee { + Self::non_reentrant_exit(&env); return Err(ContractError::InsufficientDisputeFee); } @@ -1426,6 +1600,7 @@ impl GameContract { (dispute_counter, filer), ); + Self::non_reentrant_exit(&env); Ok(dispute_counter) } @@ -1480,8 +1655,16 @@ impl GameContract { return Err(ContractError::TimeoutNotReached); } + Self::non_reentrant_enter(&env)?; + game.winner = Some(claimant.clone()); - Self::process_payout(&env, &game, &claimant)?; + match Self::process_payout(&env, &game, &claimant) { + Ok(()) => {} + Err(e) => { + Self::non_reentrant_exit(&env); + return Err(e); + } + } game.state = GameState::Settled; games.set(game_id, game); @@ -1492,6 +1675,7 @@ impl GameContract { (claimant, timeout_duration), ); + Self::non_reentrant_exit(&env); Ok(()) } @@ -1560,20 +1744,35 @@ impl GameContract { return Err(ContractError::GameAlreadyCompleted); } + Self::non_reentrant_enter(&env)?; + match winner { Some(ref winner_addr) => { if *winner_addr != game.player1 && Some(winner_addr.clone()) != game.player2 { + Self::non_reentrant_exit(&env); return Err(ContractError::NotPlayer); } game.state = GameState::Completed; game.winner = Some(winner_addr.clone()); - Self::process_payout(&env, &game, winner_addr)?; + match Self::process_payout(&env, &game, winner_addr) { + Ok(()) => {} + Err(e) => { + Self::non_reentrant_exit(&env); + return Err(e); + } + } game.state = GameState::Settled; } None => { game.state = GameState::Drawn; game.winner = None; - Self::process_draw_payout(&env, &game)?; + match Self::process_draw_payout(&env, &game) { + Ok(()) => {} + Err(e) => { + Self::non_reentrant_exit(&env); + return Err(e); + } + } } } @@ -1590,6 +1789,7 @@ impl GameContract { (dispute_id, winner), ); + Self::non_reentrant_exit(&env); Ok(()) } @@ -1629,6 +1829,8 @@ impl GameContract { return Err(ContractError::GameAlreadyCompleted); } + Self::non_reentrant_enter(&env)?; + // Update dispute status dispute.status = DisputeStatus::Rejected; dispute.resolution = Some(reason); @@ -1650,6 +1852,7 @@ impl GameContract { (dispute_id, filer), ); + Self::non_reentrant_exit(&env); Ok(()) } @@ -2226,6 +2429,8 @@ impl GameContract { return Err(ContractError::InvalidPercentage); } + Self::non_reentrant_enter(&env)?; + let token_client = Self::token_client(&env); let contract_address = env.current_contract_address(); let total = escrow.total_amount; @@ -2256,6 +2461,7 @@ impl GameContract { escrow_id, ); + Self::non_reentrant_exit(&env); Ok(()) } @@ -3544,4 +3750,95 @@ mod tests { let escrow = client.get_tournament_escrow(&escrow_id); assert!(escrow.released); } + + // ── Reentrancy Guard Tests (#860) ────────────────────────────────────────── + + #[test] + fn test_reentrancy_guard_normal_call_succeeds() { + let env = Env::default(); + env.mock_all_auths(); + let issuer = Address::generate(&env); + let stellar_token = env.register_stellar_asset_contract_v2(issuer.clone()); + let token_address = stellar_token.address(); + let stellar_asset_client = StellarAssetClient::new(&env, &token_address); + let admin = Address::generate(&env); + let player1 = Address::generate(&env); + let treasury_addr = Address::generate(&env); + stellar_asset_client.mint(&player1, &1_000i128); + let contract_id = env.register_contract(None, GameContract); + let client = GameContractClient::new(&env, &contract_id); + client.initialize_token(&admin, &token_address); + client.initialize_puzzle_rewards( + &admin, + &Bytes::from_slice(&env, &[0u8; 32]), + &0i128, + &0u32, + &treasury_addr, + ); + let game_id = client.create_game(&player1, &100); + assert!(game_id > 0); + } + + #[test] + fn test_reentrancy_guard_rejects_nested_entry() { + let env = Env::default(); + env.mock_all_auths(); + let issuer = Address::generate(&env); + let stellar_token = env.register_stellar_asset_contract_v2(issuer.clone()); + let token_address = stellar_token.address(); + let stellar_asset_client = StellarAssetClient::new(&env, &token_address); + let admin = Address::generate(&env); + let player1 = Address::generate(&env); + let treasury_addr = Address::generate(&env); + stellar_asset_client.mint(&player1, &1_000i128); + let contract_id = env.register_contract(None, GameContract); + let client = GameContractClient::new(&env, &contract_id); + client.initialize_token(&admin, &token_address); + client.initialize_puzzle_rewards( + &admin, + &Bytes::from_slice(&env, &[0u8; 32]), + &0i128, + &0u32, + &treasury_addr, + ); + env.as_contract(&contract_id, || { + env.storage().instance().set(&R_GUARD, &1u32); + }); + let result = client.try_create_game(&player1, &100); + assert_eq!(result, Err(Ok(ContractError::ReentrantCall))); + } + + #[test] + fn test_reentrancy_guard_released_after_call() { + let env = Env::default(); + env.mock_all_auths(); + let issuer = Address::generate(&env); + let stellar_token = env.register_stellar_asset_contract_v2(issuer.clone()); + let token_address = stellar_token.address(); + let stellar_asset_client = StellarAssetClient::new(&env, &token_address); + let admin = Address::generate(&env); + let player1 = Address::generate(&env); + let player2 = Address::generate(&env); + let treasury_addr = Address::generate(&env); + stellar_asset_client.mint(&player1, &1_000i128); + stellar_asset_client.mint(&player2, &1_000i128); + let contract_id = env.register_contract(None, GameContract); + let client = GameContractClient::new(&env, &contract_id); + client.initialize_token(&admin, &token_address); + client.initialize_puzzle_rewards( + &admin, + &Bytes::from_slice(&env, &[0u8; 32]), + &0i128, + &0u32, + &treasury_addr, + ); + + // First call: create_game enters and exits guard + let game_id = client.create_game(&player1, &100); + + // Guard should be released, allowing join_game to proceed + client.join_game(&game_id, &player2); + let game = client.get_game(&game_id); + assert_eq!(game.state, GameState::InProgress); + } } diff --git a/contracts/game_contract/src/test.rs b/contracts/game_contract/src/test.rs index 3e84972..45fe85c 100644 --- a/contracts/game_contract/src/test.rs +++ b/contracts/game_contract/src/test.rs @@ -1248,4 +1248,117 @@ fn fuzz_payout_even_split() { assert_eq!(v1 + v2, pool, "[iter {iter}] 50/50 total"); }); } + #[test] + fn test_reentrancy_guard_payout_tournament() { + let env = Env::default(); + let contract_id = env.register_contract(None, GameContract); + let client = GameContractClient::new(&env, &contract_id); + let player1 = Address::generate(&env); + let player2 = Address::generate(&env); + let wager: i128 = 1000; + let game_id = seed_completed_game(&env, &contract_id, &player1, &player2, wager); + let winner1 = Address::generate(&env); + let mut winners = Vec::new(&env); + winners.push_back(winner1.clone()); + let mut percentages = Vec::new(&env); + percentages.push_back(100); + client.mock_all_auths().payout_tournament(&game_id, &winners, &percentages); + let games: Map = env.as_contract(&contract_id, || { + env.storage().instance().get(&GAMES).unwrap() + }); + let game = games.get(game_id).unwrap(); + assert_eq!(game.state, GameState::Settled); + } + + #[test] + fn test_reentrancy_guard_claim_win() { + let env = Env::default(); + let contract_id = env.register_contract(None, GameContract); + let client = GameContractClient::new(&env, &contract_id); + let player1 = Address::generate(&env); + let player2 = Address::generate(&env); + let wager: i128 = 1000; + let game_id = seed_completed_game(&env, &contract_id, &player1, &player2, wager); + let winner = Address::generate(&env); + client.mock_all_auths().claim_win(&game_id, &winner); + let games: Map = env.as_contract(&contract_id, || { + env.storage().instance().get(&GAMES).unwrap() + }); + let game = games.get(game_id).unwrap(); + assert_eq!(game.state, GameState::Settled); + } + + // ADMIN_KEY Rotation Timelock Tests (#890) + + #[test] + fn test_propose_new_admin_key_admin_only() { + let env = Env::default(); + let contract_id = env.register_contract(None, GameContract); + let client = GameContractClient::new(&env, &contract_id); + let admin = Address::generate(&env); + let admin_key = Bytes::from_slice(&env, &[0u8; 32]); + env.mock_all_auths(); + client.initialize_puzzle_rewards(&admin, &admin_key, &0i128, &0u32, &Address::generate(&env)); + let stranger = Address::generate(&env); + let new_key = BytesN::from_array(&env, &[2u8; 32]); + let res = client.try_propose_new_admin_key(&stranger, &new_key); + assert!(res.is_err()); + } + + #[test] + fn test_propose_new_admin_key_sets_pending() { + let env = Env::default(); + let contract_id = env.register_contract(None, GameContract); + let client = GameContractClient::new(&env, &contract_id); + let admin = Address::generate(&env); + let admin_key = Bytes::from_slice(&env, &[0u8; 32]); + env.mock_all_auths(); + client.initialize_puzzle_rewards(&admin, &admin_key, &0i128, &0u32, &Address::generate(&env)); + let new_key = BytesN::from_array(&env, &[2u8; 32]); + client.propose_new_admin_key(&admin, &new_key); + env.as_contract(&contract_id, || { + let pending: BytesN<32> = env.storage().instance().get(&PENDING_ADMIN_KEY).unwrap(); + assert_eq!(pending, new_key); + }); + } + + #[test] + fn test_accept_new_admin_key_no_pending() { + let env = Env::default(); + let contract_id = env.register_contract(None, GameContract); + let client = GameContractClient::new(&env, &contract_id); + let res = client.try_accept_new_admin_key(); + assert!(res.is_err()); + } + + #[test] + fn test_accept_new_admin_key_timelock_not_expired() { + let env = Env::default(); + let contract_id = env.register_contract(None, GameContract); + let client = GameContractClient::new(&env, &contract_id); + let admin = Address::generate(&env); + let admin_key = Bytes::from_slice(&env, &[0u8; 32]); + env.mock_all_auths(); + client.initialize_puzzle_rewards(&admin, &admin_key, &0i128, &0u32, &Address::generate(&env)); + let new_key = BytesN::from_array(&env, &[2u8; 32]); + client.propose_new_admin_key(&admin, &new_key); + let res = client.try_accept_new_admin_key(); + assert!(res.is_err()); + } + + #[test] + fn test_propose_new_admin_key_already_pending() { + let env = Env::default(); + let contract_id = env.register_contract(None, GameContract); + let client = GameContractClient::new(&env, &contract_id); + let admin = Address::generate(&env); + let admin_key = Bytes::from_slice(&env, &[0u8; 32]); + env.mock_all_auths(); + client.initialize_puzzle_rewards(&admin, &admin_key, &0i128, &0u32, &Address::generate(&env)); + let key1 = BytesN::from_array(&env, &[2u8; 32]); + let key2 = BytesN::from_array(&env, &[3u8; 32]); + client.propose_new_admin_key(&admin, &key1); + let res = client.try_propose_new_admin_key(&admin, &key2); + assert!(res.is_err()); + } }