From 2bee7a46edda975b23887eac6527d2a43644c32a Mon Sep 17 00:00:00 2001 From: unknown Date: Tue, 10 Feb 2026 11:35:39 +0100 Subject: [PATCH 1/2] fix: prevent division by zero in update_staked_weight CRITICAL SECURITY FIX: Division-by-zero vulnerability in NFT staking The change_reward instruction allowed calls when current_time == staking_ends_at due to using require_gte! instead of require_gt!. This would set last_reward_change_time equal to staking_ends_at, causing base=0 in update_staked_weight and permanently locking all staked NFTs. Changes: - change_reward.rs: require_gte! -> require_gt! for timing check - stake_details.rs: Added defense-in-depth guard (base > 0) before division Impact: Prevents permanent locking of user NFTs by malicious/careless creators --- .../nft-stake-vault/src/instructions/change_reward.rs | 5 ++++- .../programs/nft-stake-vault/src/state/stake_details.rs | 4 ++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/nft-stake-vault/programs/nft-stake-vault/src/instructions/change_reward.rs b/nft-stake-vault/programs/nft-stake-vault/src/instructions/change_reward.rs index c130f6f..3ca4462 100644 --- a/nft-stake-vault/programs/nft-stake-vault/src/instructions/change_reward.rs +++ b/nft-stake-vault/programs/nft-stake-vault/src/instructions/change_reward.rs @@ -42,7 +42,10 @@ pub fn change_reward_handler(ctx: Context, new_reward: u64) -> Res let current_reward = *stake_details.reward.last().unwrap(); let last_reward_change_time = *stake_details.reward_change_time.last().unwrap(); - require_gte!(staking_ends_at, current_time, StakeError::StakingIsOver); + // FIX: Use strict greater-than to prevent division by zero in update_staked_weight + // When current_time == staking_ends_at, the reward change would set + // last_reward_change_time = staking_ends_at, causing base = 0 in weight calculation + require_gt!(staking_ends_at, current_time, StakeError::StakingIsOver); require_eq!(staking_status, true, StakeError::StakingInactive); let (current_actual_balance, new_staked_weight) = calc_actual_balance( diff --git a/nft-stake-vault/programs/nft-stake-vault/src/state/stake_details.rs b/nft-stake-vault/programs/nft-stake-vault/src/state/stake_details.rs index 0ebd0d1..b2d2ef1 100644 --- a/nft-stake-vault/programs/nft-stake-vault/src/state/stake_details.rs +++ b/nft-stake-vault/programs/nft-stake-vault/src/state/stake_details.rs @@ -95,6 +95,10 @@ impl Details { .checked_sub(last_reward_time) .ok_or(StakeError::ProgramSubError)? as u128; // directly converting to u128 since it can't be negative + // FIX: Defense-in-depth guard against division by zero + // This catches edge cases where staking_ends_at == last_reward_change_time + require!(base > 0, StakeError::ProgramDivError); + let weight_time = stake_time.max(last_reward_time); let mut num = self.staking_ends_at From 14665ef2223ca709e150bef5e40e51aa21113569 Mon Sep 17 00:00:00 2001 From: unknown Date: Tue, 10 Feb 2026 11:41:43 +0100 Subject: [PATCH 2/2] docs: add security report and PoC documentation --- POC_EXPLOIT.md | 86 +++++++++++++++++++++++ SECURITY_REPORT.md | 169 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 255 insertions(+) create mode 100644 POC_EXPLOIT.md create mode 100644 SECURITY_REPORT.md diff --git a/POC_EXPLOIT.md b/POC_EXPLOIT.md new file mode 100644 index 0000000..bc53c80 --- /dev/null +++ b/POC_EXPLOIT.md @@ -0,0 +1,86 @@ +# Proof of Concept: Division by Zero Vulnerability + +## Vulnerability Summary +A malicious or careless staking pool creator can permanently lock all staked NFTs by calling `change_reward` at the exact moment when `current_time == staking_ends_at`. + +## Attack Steps + +### Prerequisites +- Staking pool exists with active stakers +- Creator has authority to call `change_reward` + +### Exploitation +``` +1. Wait until clock.unix_timestamp == stake_details.staking_ends_at +2. Call change_reward(new_amount) +3. This sets reward_change_time.last() = staking_ends_at +4. Any subsequent unstake/claim calls update_staked_weight() +5. update_staked_weight calculates: base = staking_ends_at - last_reward_time = 0 +6. Division by zero causes transaction failure +7. All NFTs permanently locked in contract +``` + +### Simulated Test Case +```rust +#[test] +fn test_division_by_zero_exploit() { + // Setup: Create staking pool ending at time T + let staking_ends_at = 1000; + let mut stake_details = create_stake_details(staking_ends_at); + + // User stakes NFT + stake_details.update_staked_weight(500, true).unwrap(); + + // EXPLOIT: Creator calls change_reward exactly at end time + let exploit_time = staking_ends_at; // current_time == staking_ends_at + stake_details.change_reward(200, exploit_time); + // Now: reward_change_time.last() == staking_ends_at + + // IMPACT: User tries to unstake - FAILS with division by zero + // base = staking_ends_at - last_reward_time = 1000 - 1000 = 0 + let result = stake_details.update_staked_weight(exploit_time, false); + assert!(result.is_err()); // Division by zero! + + // User's NFT is now PERMANENTLY LOCKED +} +``` + +### Vulnerable Code Path +``` +change_reward (current_time == staking_ends_at) + └── require_gte!(staking_ends_at, current_time) ✓ passes (1000 >= 1000) + └── stake_details.change_reward(new_reward, current_time) + └── reward_change_time.push(current_time) // Now equals staking_ends_at + +unstake/claim + └── update_staked_weight(...) + └── base = staking_ends_at - last_reward_time = 0 + └── weight = num / base // DIVISION BY ZERO + └── TRANSACTION FAILS + +NFTs cannot be withdrawn -> PERMANENT LOCK +``` + +## Fix Applied + +### Primary Fix (change_reward.rs) +```diff +- require_gte!(staking_ends_at, current_time, StakeError::StakingIsOver); ++ // FIX: Use strict greater-than to prevent division by zero ++ require_gt!(staking_ends_at, current_time, StakeError::StakingIsOver); +``` + +### Defense-in-Depth (stake_details.rs) +```diff + let base = self.staking_ends_at + .checked_sub(last_reward_time) + .ok_or(StakeError::ProgramSubError)? as u128; ++ ++ // FIX: Defense-in-depth guard against division by zero ++ require!(base > 0, StakeError::ProgramDivError); +``` + +## Severity: CRITICAL +- **Impact**: Total loss of staked assets (NFTs permanently locked) +- **Likelihood**: Requires specific timing but trivially exploitable +- **Exploitability**: Single transaction, no special tools needed diff --git a/SECURITY_REPORT.md b/SECURITY_REPORT.md new file mode 100644 index 0000000..ebd871b --- /dev/null +++ b/SECURITY_REPORT.md @@ -0,0 +1,169 @@ +# Security Audit Report: NFT-Staking-Program + +## Summary +A critical division-by-zero vulnerability was identified in the NFT Staking Program that can result in permanent locking of user assets. + +**Repository:** https://github.com/0xShuk/NFT-Staking-Program +**Audited by:** AI Security Agent (agentpay-monocle) +**Date:** February 10, 2026 +**Severity:** CRITICAL + +--- + +## Vulnerability Details + +### Title +Division by Zero in `update_staked_weight` Causes Permanent Asset Lock + +### Location +- **File:** `nft-stake-vault/programs/nft-stake-vault/src/state/stake_details.rs` +- **Function:** `update_staked_weight` +- **Line:** 88 + +### Vulnerable Code +```rust +pub fn update_staked_weight(&mut self, stake_time: i64, increase_weight: bool) -> Result<()> { + let last_reward_time = *self.reward_change_time.last().unwrap(); + + let base = self.staking_ends_at + .checked_sub(last_reward_time) + .ok_or(StakeError::ProgramSubError)? as u128; // <-- CAN BE ZERO + + // ... + + let weight = num.checked_div(base).ok_or(StakeError::ProgramDivError)?; // <-- DIVISION BY ZERO +``` + +### Root Cause +The `base` variable is calculated as `staking_ends_at - last_reward_time`. If these two values are equal, `base = 0`, causing `checked_div(base)` to return `None` and trigger a `ProgramDivError`. + +### Attack Vector +1. Creator initializes staking with an end time +2. When `current_time == staking_ends_at`, creator calls `change_reward()` +3. The `change_reward_handler` has check `require_gte!(staking_ends_at, current_time, StakeError::StakingIsOver)` which **allows** `staking_ends_at == current_time` +4. After the call, `reward_change_time.last() = current_time = staking_ends_at` +5. Any subsequent call to `unstake()` or `withdraw_reward()` will invoke `update_staked_weight()` +6. The function will fail with division by zero +7. **Users cannot unstake their NFTs - assets are permanently locked** + +### Impact +- **Severity:** CRITICAL +- **Asset Loss:** Users' staked NFTs are permanently locked in the program +- **Exploitability:** Medium (requires timing the `change_reward` call precisely) +- **Attack Type:** Griefing / Asset Freezing + +### Affected Functions +1. `unstake_handler` - Users cannot retrieve their NFTs +2. `withdraw_reward_handler` - Users cannot claim rewards + +--- + +## Proof of Concept + +### Scenario +``` +Time: T0 - Staking starts +Time: T1 - User stakes NFT +Time: T2 - Staking ends (staking_ends_at) + +Attack: +1. At exactly T2, creator calls: change_reward(new_reward) +2. This sets: reward_change_time.last() = T2 = staking_ends_at +3. User calls: unstake() +4. update_staked_weight() calculates: base = T2 - T2 = 0 +5. Division by zero error → transaction fails +6. User's NFT is permanently locked +``` + +### Test Code +```typescript +it("Demonstrates division by zero vulnerability", async () => { + // Setup: Create staking pool, user stakes NFT + // ... + + // Wait until staking_ends_at + await sleep(stakingDuration); + + // Creator calls change_reward at exact end time + await program.methods + .changeReward(new BN(newRewardRate)) + .accounts({ + stakeDetails: stakeDetailsPDA, + creator: creator.publicKey, + systemProgram: SystemProgram.programId, + }) + .signers([creator]) + .rpc(); + + // User attempts to unstake - THIS WILL FAIL + try { + await program.methods + .unstake() + .accounts({ + stakeDetails: stakeDetailsPDA, + nftRecord: nftRecordPDA, + // ... other accounts + }) + .signers([user]) + .rpc(); + + assert.fail("Should have thrown error"); + } catch (e) { + // Error: ProgramDivError - Division by zero + console.log("NFT permanently locked due to division by zero!"); + } +}); +``` + +--- + +## Recommended Fix + +### Option 1: Strict Time Check in `change_reward_handler` (Recommended) +Change the condition from `>=` to `>`: + +```rust +// Before (vulnerable) +require_gte!(staking_ends_at, current_time, StakeError::StakingIsOver); + +// After (fixed) +require_gt!(staking_ends_at, current_time, StakeError::StakingIsOver); +``` + +This prevents `change_reward` from being called at the exact moment staking ends. + +### Option 2: Guard in `update_staked_weight` +Add a check for zero base: + +```rust +pub fn update_staked_weight(&mut self, stake_time: i64, increase_weight: bool) -> Result<()> { + let last_reward_time = *self.reward_change_time.last().unwrap(); + + let base = self.staking_ends_at + .checked_sub(last_reward_time) + .ok_or(StakeError::ProgramSubError)? as u128; + + // NEW: Guard against division by zero + if base == 0 { + // Staking period has ended at reward change time + // Weight should remain unchanged or be set to 0 + return Ok(()); + } + + // ... rest of function +``` + +### Option 3: Both Fixes +Apply both fixes for defense in depth. + +--- + +## Additional Findings + +### Low Severity: Missing Active Check in Unstake +The `unstake_handler` does not verify `is_active` status, allowing unstaking even after `close_staking()` is called. This may be intentional but should be documented. + +--- + +## Conclusion +The NFT Staking Program contains a critical vulnerability that can permanently lock user assets. The fix is straightforward and should be applied immediately. I recommend using Option 1 (strict time check) as the primary fix and Option 2 as defense in depth.