security: fix inverted fundraiser deadline logic in token-fundraiser (#674)

* security: fix inverted fundraiser deadline logic in token-fundraiser

contribute.rs and refund.rs both gated their time check on the wrong
side of the comparison. contribute() required duration <= elapsed_days
to pass - contributions only succeeded *after* the fundraiser's
duration had already elapsed, and stayed open forever after that.
refund() required duration >= elapsed_days - refunds only succeeded
*before* the deadline.

Net effect: with any realistic nonzero duration, contribute() and
refund() are live in mutually-exclusive, backwards time windows.
Nobody can contribute while the fundraiser claims to be running; once
contributions start working (only possible post-deadline), refund is
permanently blocked. If the target isn't met, funds sit in the vault
with no instruction able to move them - check_contributions requires
the target met, refund now requires "not yet past deadline" which is
false by construction. A fund-lock, not a theft, but a complete break
of the contract's core guarantee.

Confirmed against the example's own README, whose prose states the
correct intent while the code implemented the opposite - and whose
refund code snippet already had the correct form, meaning refund.rs
itself had drifted from the documented behavior, not the reverse.

Fixed with a two-character flip per file: contribute.rs now requires
duration > elapsed (window still open); refund.rs now requires
duration <= elapsed (window has closed), matching the README exactly.
Also fixed the one inverted README snippet (contribute section; the
refund section was already correct) and the misleading comments above
both checks.

Both test suites previously called initialize(..., 0) - duration=0
made elapsed_days >= 0 trivially satisfy both inverted checks at once,
completely masking the bug. Updated both to a realistic duration=1 and
added adversarial coverage: tests/litesvm.test.ts (which can warp its
own clock) now proves contribute() correctly rejects at the exact
deadline boundary and refund() correctly succeeds once past it, with
balance/account-closure assertions on the happy path.
tests/fundraiser.ts (real validator, no way to fast-forward its clock)
proves refund() is correctly rejected while still active. Verified the
new tests actually fail against the pre-fix code (multiple independent
failure signals, including a direct "succeeded when it should have
rejected" repro at the boundary) and pass after.

Out of scope, called out for follow-up: a possibly-negative elapsed
value getting cast to u16 (wraps on a backwards clock - the fix widens
nothing, kept as a 2-line diff instead of adding saturating_sub
hardening); refund.rs/checker.rs gating on the vault's live token
balance rather than the tracked current_amount, letting anyone grief
the target check by transferring tokens into the vault directly; and
check_contributions closing the Fundraiser PDA while Contributor PDAs
may still be open, stranding their rent. All separate findings from
this time-gate inversion.

* fix(#674): address dev-jodee review - shared test util, trim comments

1. Extracted the duplicated expectAnchorError helper from both test
   files into a shared tests/utils.ts.
2. Trimmed the explanatory comments across both test files - several
   ran 5-13 lines to explain a single design decision. Kept the one
   load-bearing sentence per comment, cut the rest.

Author: NikkiAung

tokens/token-fundraiser/anchor/programs/fundraiser/src/instructions/contribute.rs

@@ -68,10 +68,10 @@ impl<'info> Contribute<'info> {
             FundraiserError::ContributionTooBig
         );
 
-        // Check if the fundraising duration has been reached
+        // Check that the fundraising duration has not elapsed yet
         let current_time = Clock::get()?.unix_timestamp;
         require!(
-            self.fundraiser.duration <= ((current_time - self.fundraiser.time_started) / SECONDS_TO_DAYS) as u16,
+            self.fundraiser.duration > ((current_time - self.fundraiser.time_started) / SECONDS_TO_DAYS) as u16,
             crate::FundraiserError::FundraiserEnded
         );
 

tokens/token-fundraiser/anchor/programs/fundraiser/src/instructions/refund.rs

@@ -54,11 +54,11 @@ pub struct Refund<'info> {
 impl<'info> Refund<'info> {
     pub fn refund(&mut self) -> Result<()> {
 
-        // Check if the fundraising duration has been reached
+        // Check that the fundraising duration has elapsed
         let current_time = Clock::get()?.unix_timestamp;
- 
+
         require!(
-            self.fundraiser.duration >= ((current_time - self.fundraiser.time_started) / SECONDS_TO_DAYS) as u16,
+            self.fundraiser.duration <= ((current_time - self.fundraiser.time_started) / SECONDS_TO_DAYS) as u16,
             crate::FundraiserError::FundraiserNotEnded
         );
 

tokens/token-fundraiser/anchor/readme.MD

@@ -212,10 +212,10 @@ impl<'info> Contribute<'info> {
             FundraiserError::MaximumContributionsReached
         );
 
-        // Check if the fundraising duration has been reached
+        // Check that the fundraising duration has not elapsed yet
         let current_time = Clock::get()?.unix_timestamp;
         require!(
-            self.fundraiser.duration <= ((current_time - self.fundraiser.time_started) / SECONDS_TO_DAYS) as u8,
+            self.fundraiser.duration > ((current_time - self.fundraiser.time_started) / SECONDS_TO_DAYS) as u8,
             crate::FundraiserError::FundraisingEnded
         );
 

tokens/token-fundraiser/anchor/tests/fundraiser.ts

@@ -10,7 +10,9 @@ import {
     TOKEN_PROGRAM_ID,
 } from '@solana/spl-token';
 import BN from 'bn.js';
+import { assert } from 'chai';
 import type { Fundraiser } from '../target/types/fundraiser';
+import { expectAnchorError } from './utils';
 
 describe('fundraiser', () => {
     // Configure the client to use the local cluster.
@@ -78,8 +80,10 @@ describe('fundraiser', () => {
     it('Initialize Fundaraiser', async () => {
         const vault = getAssociatedTokenAddressSync(mint, fundraiser, true);
 
+        // duration=1 day. No clock-warping here (real validator) - the
+        // post-deadline happy path is covered in litesvm.test.ts instead.
         const tx = await program.methods
-            .initialize(new BN(30000000), 0)
+            .initialize(new BN(30000000), 1)
             .accountsPartial({
                 maker: maker.publicKey,
                 fundraiser,
@@ -145,10 +149,12 @@ describe('fundraiser', () => {
     });
 
     it('Contribute to Fundraiser - Robustness Test', async () => {
-        try {
-            const vault = getAssociatedTokenAddressSync(mint, fundraiser, true);
+        // Per-contributor cap is 10% of target (3_000_000); contributor
+        // already holds 2_000_000, so this pushes past the cap.
+        const vault = getAssociatedTokenAddressSync(mint, fundraiser, true);
 
-            const tx = await program.methods
+        await expectAnchorError(
+            program.methods
                 .contribute(new BN(2000000))
                 .accountsPartial({
                     contributor: provider.publicKey,
@@ -159,22 +165,17 @@ describe('fundraiser', () => {
                     tokenProgram: TOKEN_PROGRAM_ID,
                 })
                 .rpc()
-                .then(confirm);
-
-            console.log('\nContributed to fundraiser', tx);
-            console.log('Your transaction signature', tx);
-            console.log('Vault balance', (await provider.connection.getTokenAccountBalance(vault)).value.amount);
-        } catch (error) {
-            console.log('\nError contributing to fundraiser');
-            console.log(error.msg);
-        }
+                .then(confirm),
+            'MaximumContributionsReached',
+        );
     });
 
     it('Check contributions - Robustness Test', async () => {
-        try {
-            const vault = getAssociatedTokenAddressSync(mint, fundraiser, true);
+        // Only 2_000_000 has been contributed against a 30_000_000 target.
+        const vault = getAssociatedTokenAddressSync(mint, fundraiser, true);
 
-            const tx = await program.methods
+        await expectAnchorError(
+            program.methods
                 .checkContributions()
                 .accountsPartial({
                     maker: maker.publicKey,
@@ -186,41 +187,38 @@ describe('fundraiser', () => {
                 })
                 .signers([maker])
                 .rpc()
-                .then(confirm);
-
-            console.log('\nChecked contributions');
-            console.log('Your transaction signature', tx);
-            console.log('Vault balance', (await provider.connection.getTokenAccountBalance(vault)).value.amount);
-        } catch (error) {
-            console.log('\nError checking contributions');
-            console.log(error.msg);
-        }
+                .then(confirm),
+            'TargetNotMet',
+        );
     });
 
-    it('Refund Contributions', async () => {
+    // Can't clock-warp on a real validator, so this only proves refund is
+    // rejected while active - see litesvm.test.ts for the post-deadline
+    // happy path.
+    it('Refund is rejected while the fundraiser is still active', async () => {
         const vault = getAssociatedTokenAddressSync(mint, fundraiser, true);
+        const vaultBalanceBefore = (await provider.connection.getTokenAccountBalance(vault)).value.amount;
 
-        const contributorAccount = await program.account.contributor.fetch(contributor);
-        console.log('\nContributor balance', contributorAccount.amount.toString());
-
-        const tx = await program.methods
-            .refund()
-            .accountsPartial({
-                contributor: provider.publicKey,
-                maker: maker.publicKey,
-                mintToRaise: mint,
-                fundraiser,
-                contributorAccount: contributor,
-                contributorAta: contributorATA,
-                vault,
-                tokenProgram: TOKEN_PROGRAM_ID,
-                systemProgram: anchor.web3.SystemProgram.programId,
-            })
-            .rpc()
-            .then(confirm);
+        await expectAnchorError(
+            program.methods
+                .refund()
+                .accountsPartial({
+                    contributor: provider.publicKey,
+                    maker: maker.publicKey,
+                    mintToRaise: mint,
+                    fundraiser,
+                    contributorAccount: contributor,
+                    contributorAta: contributorATA,
+                    vault,
+                    tokenProgram: TOKEN_PROGRAM_ID,
+                    systemProgram: anchor.web3.SystemProgram.programId,
+                })
+                .rpc()
+                .then(confirm),
+            'FundraiserNotEnded',
+        );
 
-        console.log('\nRefunded contributions', tx);
-        console.log('Your transaction signature', tx);
-        console.log('Vault balance', (await provider.connection.getTokenAccountBalance(vault)).value.amount);
+        const vaultBalanceAfter = (await provider.connection.getTokenAccountBalance(vault)).value.amount;
+        assert.strictEqual(vaultBalanceAfter, vaultBalanceBefore, 'rejected refund must not move any funds');
     });
 });

tokens/token-fundraiser/anchor/tests/litesvm.test.ts

@@ -12,11 +12,14 @@ import {
 import { PublicKey } from '@solana/web3.js';
 import { LiteSVMProvider } from 'anchor-litesvm';
 import BN from 'bn.js';
+import { assert } from 'chai';
 import { LiteSVM } from 'litesvm';
 import IDL from '../target/idl/fundraiser.json';
 import type { Fundraiser } from '../target/types/fundraiser';
+import { expectAnchorError } from './utils';
 
 const PROGRAM_ID = new PublicKey(IDL.address);
+const SECONDS_PER_DAY = 86400n;
 
 describe('fundraiser litesvm', () => {
     const client = new LiteSVM();
@@ -78,8 +81,10 @@ describe('fundraiser litesvm', () => {
     it('Initialize Fundaraiser', async () => {
         const vault = getAssociatedTokenAddressSync(mint, fundraiser, true);
 
+        // duration=1 day. This suite can warp its own clock, so it covers
+        // the full lifecycle (see the tests below).
         const tx = await program.methods
-            .initialize(new BN(30000000), 0)
+            .initialize(new BN(30000000), 1)
             .accountsPartial({
                 maker: maker.publicKey,
                 fundraiser,
@@ -143,10 +148,12 @@ describe('fundraiser litesvm', () => {
     });
 
     it('Contribute to Fundraiser - Robustness Test', async () => {
-        try {
-            const vault = getAssociatedTokenAddressSync(mint, fundraiser, true);
+        // Per-contributor cap is 3_000_000 (10% of target); contributor
+        // holds 2_000_000 already. Must run before the deadline warp below.
+        const vault = getAssociatedTokenAddressSync(mint, fundraiser, true);
 
-            const tx = await program.methods
+        await expectAnchorError(
+            program.methods
                 .contribute(new BN(2000000))
                 .accountsPartial({
                     contributor: provider.publicKey,
@@ -156,22 +163,46 @@ describe('fundraiser litesvm', () => {
                     vault,
                     tokenProgram: TOKEN_PROGRAM_ID,
                 })
-                .rpc();
-
-            console.log('\nContributed to fundraiser', tx);
-            console.log('Your transaction signature', tx);
-            console.log('Vault balance', tokenBalance(vault).toString());
-        } catch (error) {
-            console.log('\nError contributing to fundraiser');
-            console.log(error.msg);
-        }
+                .rpc(),
+            'MaximumContributionsReached',
+        );
+    });
+
+    // Pre-fix this fails with AccountNotInitialized, not a wrongly-
+    // succeeding refund - contribute() was broken from its first call, so
+    // no Contributor account ever formed. Same bug, different symptom.
+    it('Refund is rejected while the fundraiser is still active', async () => {
+        const vault = getAssociatedTokenAddressSync(mint, fundraiser, true);
+        const vaultBalanceBefore = tokenBalance(vault);
+
+        await expectAnchorError(
+            program.methods
+                .refund()
+                .accountsPartial({
+                    contributor: provider.publicKey,
+                    maker: maker.publicKey,
+                    mintToRaise: mint,
+                    fundraiser,
+                    contributorAccount: contributor,
+                    contributorAta: contributorATA,
+                    vault,
+                    tokenProgram: TOKEN_PROGRAM_ID,
+                    systemProgram: anchor.web3.SystemProgram.programId,
+                })
+                .rpc(),
+            'FundraiserNotEnded',
+        );
+
+        assert.strictEqual(tokenBalance(vault), vaultBalanceBefore, 'rejected refund must not move any funds');
     });
 
     it('Check contributions - Robustness Test', async () => {
-        try {
-            const vault = getAssociatedTokenAddressSync(mint, fundraiser, true);
+        // Only 2_000_000 has been contributed against a 30_000_000 target.
+        // Time-independent - checker.rs has no duration check.
+        const vault = getAssociatedTokenAddressSync(mint, fundraiser, true);
 
-            const tx = await program.methods
+        await expectAnchorError(
+            program.methods
                 .checkContributions()
                 .accountsPartial({
                     maker: maker.publicKey,
@@ -182,23 +213,54 @@ describe('fundraiser litesvm', () => {
                     tokenProgram: TOKEN_PROGRAM_ID,
                 })
                 .signers([maker])
-                .rpc();
-
-            console.log('\nChecked contributions');
-            console.log('Your transaction signature', tx);
-            console.log('Vault balance', tokenBalance(vault).toString());
-        } catch (error) {
-            console.log('\nError checking contributions');
-            console.log(error.msg);
-        }
+                .rpc(),
+            'TargetNotMet',
+        );
+    });
+
+    // Warps to the exact deadline boundary (not past it) to pin the
+    // off-by-one directly: contribute must reject exactly here.
+    it('Fundraiser closes to contributions once the duration has elapsed', async () => {
+        const fundraiserAccount = await program.account.fundraiser.fetch(fundraiser);
+        const deadline =
+            BigInt(fundraiserAccount.timeStarted.toString()) + BigInt(fundraiserAccount.duration) * SECONDS_PER_DAY;
+
+        const clock = client.getClock();
+        clock.unixTimestamp = deadline;
+        client.setClock(clock);
+        // Avoids a duplicate-transaction rejection from an earlier identical call.
+        client.expireBlockhash();
+
+        const vault = getAssociatedTokenAddressSync(mint, fundraiser, true);
+
+        // 1_000_000 keeps the per-contributor cap check passing, so only
+        // the time check can reject this - proving it's the deadline.
+        await expectAnchorError(
+            program.methods
+                .contribute(new BN(1000000))
+                .accountsPartial({
+                    contributor: provider.publicKey,
+                    fundraiser,
+                    contributorAccount: contributor,
+                    contributorAta: contributorATA,
+                    vault,
+                    tokenProgram: TOKEN_PROGRAM_ID,
+                })
+                .rpc(),
+            'FundraiserEnded',
+        );
     });
 
     it('Refund Contributions', async () => {
+        // Runs after the deadline warp above, so refund's time check now passes.
         const vault = getAssociatedTokenAddressSync(mint, fundraiser, true);
 
         const contributorAccount = await program.account.contributor.fetch(contributor);
         console.log('\nContributor balance', contributorAccount.amount.toString());
 
+        // Same duplicate-transaction hazard as above.
+        client.expireBlockhash();
+
         const tx = await program.methods
             .refund()
             .accountsPartial({
@@ -216,6 +278,13 @@ describe('fundraiser litesvm', () => {
 
         console.log('\nRefunded contributions', tx);
         console.log('Your transaction signature', tx);
-        console.log('Vault balance', tokenBalance(vault).toString());
+
+        assert.strictEqual(tokenBalance(vault), 0n, 'vault should be fully drained back to the contributor');
+        assert.strictEqual(
+            tokenBalance(contributorATA),
+            10_000_000n,
+            "contributor's full original balance should be restored",
+        );
+        assert.isNull(client.getAccount(contributor), 'the Contributor account should be closed');
     });
 });

tokens/token-fundraiser/anchor/tests/utils.ts

@@ -0,0 +1,14 @@
+import { assert } from 'chai';
+
+// Asserts `promise` rejects with the given Anchor custom error code, not
+// just "something failed".
+export const expectAnchorError = async (promise: Promise<unknown>, code: string) => {
+    let caught: any;
+    try {
+        await promise;
+    } catch (error) {
+        caught = error;
+    }
+    assert.isDefined(caught, `expected the transaction to fail with ${code}`);
+    assert.strictEqual(caught?.error?.errorCode?.code, code, `expected ${code}, got: ${caught}`);
+};