Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
86 changes: 86 additions & 0 deletions POC_EXPLOIT.md
Original file line number Diff line number Diff line change
@@ -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
169 changes: 169 additions & 0 deletions SECURITY_REPORT.md
Original file line number Diff line number Diff line change
@@ -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.
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,10 @@ pub fn change_reward_handler(ctx: Context<ChangeReward>, 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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down