Skip to content

Security: rikokurnia/nirvana-infinity

Security

SECURITY.md

Nirvana Protocol — Security Review (Week 7)

Program: DygVVPMyA2WnbfJiqta38ySR3YhnKbqbFcNzcsjkti4K (devnet) Scope: programs/nirvana/src/lib.rs — create_stream, withdraw, cancel, trigger_milestone, top_up, release_vault Framework: Anchor 0.31.1 Reviewer: Riko Kurnia Sandi Method: Manual code review against the coral-xyz/sealevel-attacks catalog + integration/edge/adversarial tests (tests/nirvana.ts, tests/nirvana-edge-security.ts).

Verdict: No critical or high-severity issues remaining. All vesting funds are gated by signer + PDA + arithmetic checks that are exercised by the test suite (40/40 passing).

Checklist

1. Signer authority verified on every instruction ✅

Instruction Who must sign Enforced by Test
create_stream authority (funder, also payer) Signer + init payer every create test
withdraw recipient only has_one = recipient + state PDA seeds bind recipient Task 3: Unauthorized withdrawal must fail
cancel authority only has_one = authority + close = authority should fail for unauthorized user to cancel
top_up authority only has_one = authority security: only the authority can top_up
trigger_milestone authority or designated arbiter explicit `require!(is_authority
release_vault authority Signer; PDA re-derived from authority+recipient see Finding F3

An unauthorized signer also changes the derived state-PDA seeds, so Anchor rejects on ConstraintSeeds/ConstraintHasOne before the instruction body runs. Verified by the unauthorized-withdraw, unauthorized-cancel, and unauthorized-top_up tests.

2. PDA seeds are unique & canonical ✅

  • State PDA: ["state", authority, recipient, nonce.to_le_bytes()] — the nonce (added in the Week 6 fix) guarantees the same (authority, recipient) pair can hold unlimited independent streams with no collision.
  • Vault PDA: ["vault", state] — bound to its parent state.
  • Canonical bump is stored (state.bump) and reused via bump = distribution_state.bump on every subsequent instruction, preventing bump-seed substitution.
  • Test: security: two concurrent streams to the SAME recipient (nonce uniqueness) creates two streams for one pair, asserts distinct state+vault PDAs, and confirms cancelling one leaves the other live.

3. No integer overflow / underflow ✅

  • Every arithmetic op uses checked_add / checked_sub / checked_mul / checked_div, returning NirvanaError::MathOverflow on failure.
  • Linear-vesting math casts to u128 before multiplying (base_amount as u128 * elapsed) then divides, so the intermediate product cannot overflow u64.
  • Division-by-zero is guarded by total_duration > 0.
  • Test: edge: create_stream rejects amounts that overflow u64 (MathOverflow) passes u64::MAX + u64::MAX and asserts the deposit total is rejected before any token transfer.

4. Account ownership & type validation ✅

  • Accounts are typed via Anchor's #[account] / Account<TokenAccount> / Account<Mint>, so Anchor verifies the owning program and discriminator (defends against "type cosplay").
  • Token accounts are constrained: recipient_token_account.mint == distribution_state.token_mint, authority_token_account.owner == authority.
  • Test: security: withdraw with a wrong-mint token account is rejected proves the mint constraint blocks settling into a foreign-mint account.

5. Arbitrary CPI / token-program substitution ✅

  • All token movement goes through token::transfer / token::close_account with token_program: Program<Token>, so a fake token program cannot be substituted.
  • The vault's transfer authority is the state PDA, signed via CpiContext::new_with_signer with the canonical seeds + stored bump — only the program can move vault funds.

6. Account closing / revival ✅

  • cancel uses Anchor's close = authority, which zeroes the account, reassigns it to the System Program, and drains lamports — the standard safe-close, immune to the "closed-account revival" attack.
  • The vault SPL account is explicitly token::close_account-d during cancel, returning rent to the authority.
  • Test: Task 4: Cancel ... verify account closure and edge: cancel just before end ... confirm the state account is unreachable after cancel.

7. Reentrancy ✅ (N/A by platform)

  • Solana executes instructions single-threaded with no synchronous callback into the program mid-CPI, so classic reentrancy does not apply.
  • Defensive ordering is still correct: withdraw updates claimed_amount in the same instruction as the transfer, so a repeated call within a transaction cannot double-claim.
  • Test: edge: double withdraw does not double-spend withdraws a fully-vested stream twice; the second call returns NothingToWithdraw and the balance is unchanged.

Findings & Fixes

# Severity Finding Resolution
F1 Medium (tests) After the Week 6 nonce upgrade, tests/nirvana.ts still called create_stream with the old 7-arg signature and derived the state PDA with 3 seeds (no nonce). The entire suite failed to run against the deployed program. Fixed the getPDAs/createStream helpers to pass the nonce arg and seed the PDA with nonce.to_le_bytes(). All 29 existing tests now pass.
F2 Low (tests) The "trigger milestone after expiry" test asserted on the error code StreamExpired, but the client surfaced the human-readable #[msg] "Stream expired.", so the assertion was brittle. Contract behavior was correct. Relaxed the matcher to /StreamExpired|Stream expired/i.
F3 Informational release_vault derives state_signer with legacy seeds ["state", authority, recipient] (no nonce) and only runs when that account is empty (StreamStillActive otherwise). For nonce-era streams this branch is effectively unreachable — it exists solely to sweep orphaned vaults left by streams cancelled before the vault-closing upgrade. Documented as intentional migration tooling. It cannot touch a live (nonce-seeded) stream, and cancel now closes vaults, so no new orphans are produced.
F4 Informational Linear vesting uses integer division and rounds down, leaving sub-token "dust" mid-stream. Not exploitable: the remainder is never lost — it settles to the recipient at full vest, or splits correctly to recipient/authority on cancel. Tested by the full-withdraw and cancel-split tests.
F5 Low (tooling) On Node ≥ 23 the native TypeScript loader runs the .ts tests as ESM, breaking CommonJS named imports from @coral-xyz/anchor. Test scripts in package.json and Anchor.toml now set NODE_OPTIONS=--no-experimental-strip-types so ts-node compiles to CommonJS.

How to reproduce

# Terminal 1
solana-test-validator --reset \
  --bpf-program DygVVPMyA2WnbfJiqta38ySR3YhnKbqbFcNzcsjkti4K target/deploy/nirvana.so

# Terminal 2
ANCHOR_PROVIDER_URL=http://localhost:8899 \
ANCHOR_WALLET=~/.config/solana/id.json \
npm test

Expected: 37 passing. See COVERAGE.md for the per-branch coverage matrix (>80%).

There aren't any published security advisories