token-fundraiser: derive the contributor PDA bump on-chain

contribute took the contributor bump from instruction data and fed it to
create_program_address. Several bumps can yield a valid address for the same
seeds, so a contributor could open extra, non-canonical records for
themselves and be metered against the per-contributor cap separately on each.
Those records were also unrefundable, since refund only ever derives the
canonical address.

The bump is now derived on-chain with find_program_address and the supplied
byte is gone from the instruction data entirely, so there is no longer a
caller-controlled input to the derivation. This matches the Anchor version,
whose `seeds = [...], bump` constraint likewise derives canonically rather
than accepting a bump from the client.

Adds a test that a contributor_account which is not the canonical PDA is
refused before any record is created.

Author: MarkFeder

tokens/token-fundraiser/pinocchio/program/src/instructions/contribute.rs

@@ -27,7 +27,7 @@ use crate::{
 ///   6. `[]`                 token program
 ///   7. `[]`                 system program
 ///
-/// Instruction data: `[amount: u64 (LE), contributor_bump: u8]`
+/// Instruction data: `[amount: u64 (LE)]`
 pub fn contribute(program_id: &Address, accounts: &mut [AccountView], data: &[u8]) -> ProgramResult {
     let [contributor, mint_to_raise, fundraiser, contributor_account, contributor_ata, vault, _token_program, _system_program] =
         accounts
@@ -40,7 +40,6 @@ pub fn contribute(program_id: &Address, accounts: &mut [AccountView], data: &[u8
     }
 
     let amount = read_u64(data, 0)?;
-    let contributor_bump = *data.get(8).ok_or(ProgramError::InvalidInstructionData)?;
 
     let mut state = Fundraiser::deserialize(&fundraiser.try_borrow()?)?;
 
@@ -76,18 +75,23 @@ pub fn contribute(program_id: &Address, accounts: &mut [AccountView], data: &[u8
 
     // Bind the contributor record to this signer before trusting it, whether it
     // already exists or is about to be created.
+    //
+    // The bump is derived here rather than taken from the caller: several bumps
+    // can yield a valid address for the same seeds, so accepting one would let a
+    // contributor open a second, non-canonical record and be metered against the
+    // cap separately on each. `refund` only ever derives the canonical address,
+    // so those extra records would also be unrefundable.
+    let (contributor_pda, contributor_bump) = Address::find_program_address(
+        &[Contributor::SEED_PREFIX, fundraiser.address().as_ref(), contributor.address().as_ref()],
+        program_id,
+    );
     let bump_bytes = [contributor_bump];
     let seeds = [
         Seed::from(Contributor::SEED_PREFIX),
         Seed::from(fundraiser.address().as_ref()),
         Seed::from(contributor.address().as_ref()),
         Seed::from(&bump_bytes),
     ];
-    let contributor_pda = Address::create_program_address(
-        &[Contributor::SEED_PREFIX, fundraiser.address().as_ref(), contributor.address().as_ref(), &bump_bytes],
-        program_id,
-    )
-    .map_err(|_| ProgramError::InvalidSeeds)?;
     if contributor_account.address() != &contributor_pda {
         return Err(ProgramError::InvalidSeeds);
     }

tokens/token-fundraiser/pinocchio/tests/test.ts

@@ -176,7 +176,6 @@ describe('Token Fundraiser (Pinocchio)', () => {
         contributorAccount: Address,
         contributorAta: Address,
         vault: Address,
-        bump: number,
         amount: bigint,
     ) {
         return {
@@ -191,7 +190,8 @@ describe('Token Fundraiser (Pinocchio)', () => {
                 { address: TOKEN_PROGRAM_ADDRESS, role: AccountRole.READONLY },
                 { address: SYSTEM_PROGRAM_ADDRESS, role: AccountRole.READONLY },
             ],
-            data: concatBytes(Uint8Array.of(1), u64(amount), Uint8Array.of(bump)),
+            // The contributor PDA bump is derived on-chain, not supplied here.
+            data: concatBytes(Uint8Array.of(1), u64(amount)),
         };
     }
 
@@ -261,7 +261,7 @@ describe('Token Fundraiser (Pinocchio)', () => {
         const contributor = await generateKeyPairSigner();
         svm.airdrop(contributor.address, lamports(1_000_000_000n));
         const contributorAta = await fundAta(maker, maker, contributor.address, mint.address, 10n);
-        const [contributorAccount, cbump] = await contributorPda(fundraiser, contributor.address);
+        const [contributorAccount] = await contributorPda(fundraiser, contributor.address);
 
         send(
             await tx(contributor, [
@@ -272,7 +272,6 @@ describe('Token Fundraiser (Pinocchio)', () => {
                     contributorAccount,
                     contributorAta,
                     vault,
-                    cbump,
                     10n,
                 ),
             ]),
@@ -323,7 +322,7 @@ describe('Token Fundraiser (Pinocchio)', () => {
             const contributor = await generateKeyPairSigner();
             svm.airdrop(contributor.address, lamports(1_000_000_000n));
             const contributorAta = await fundAta(maker, maker, contributor.address, mint.address, 10n);
-            const [contributorAccount, cbump] = await contributorPda(fundraiser, contributor.address);
+            const [contributorAccount] = await contributorPda(fundraiser, contributor.address);
             send(
                 await tx(contributor, [
                     contributeIx(
@@ -333,7 +332,6 @@ describe('Token Fundraiser (Pinocchio)', () => {
                         contributorAccount,
                         contributorAta,
                         vault,
-                        cbump,
                         10n,
                     ),
                 ]),
@@ -373,10 +371,10 @@ describe('Token Fundraiser (Pinocchio)', () => {
         const victim = await generateKeyPairSigner();
         svm.airdrop(victim.address, lamports(1_000_000_000n));
         const victimAta = await fundAta(maker, maker, victim.address, mint.address, 1n);
-        const [victimAccount, victimBump] = await contributorPda(fundraiser, victim.address);
+        const [victimAccount] = await contributorPda(fundraiser, victim.address);
         send(
             await tx(victim, [
-                contributeIx(victim, mint.address, fundraiser, victimAccount, victimAta, vault, victimBump, 1n),
+                contributeIx(victim, mint.address, fundraiser, victimAccount, victimAta, vault, 1n),
             ]),
             'victim contribute',
         );
@@ -387,7 +385,7 @@ describe('Token Fundraiser (Pinocchio)', () => {
         const attackerAta = await fundAta(maker, maker, attacker.address, mint.address, 1n);
         sendExpectingFailure(
             await tx(attacker, [
-                contributeIx(attacker, mint.address, fundraiser, victimAccount, attackerAta, vault, victimBump, 1n),
+                contributeIx(attacker, mint.address, fundraiser, victimAccount, attackerAta, vault, 1n),
             ]),
             'contribution into a substituted record',
         );
@@ -396,6 +394,40 @@ describe('Token Fundraiser (Pinocchio)', () => {
         assert.equal(tokenAmount(vault), 1n, 'vault only holds the recorded contribution');
     });
 
+    it('Rejects a contributor record that is not the canonical PDA', async () => {
+        const maker = await generateKeyPairSigner();
+        svm.airdrop(maker.address, lamports(1_000_000_000n));
+        const mint = await createMint(maker, 0, maker.address);
+
+        const [fundraiser, bump] = await fundraiserPda(maker.address);
+        const [vault] = await findAssociatedTokenPda({
+            owner: fundraiser,
+            mint: mint.address,
+            tokenProgram: TOKEN_PROGRAM_ADDRESS,
+        });
+        send(await tx(maker, [initializeIx(maker, mint.address, fundraiser, vault, bump, 100n, 30)]), 'initialize');
+
+        // The record address is derived on-chain, so a contributor cannot open a
+        // second record for themselves at any other address — including one
+        // derived from a non-canonical bump. A record that is not the canonical
+        // PDA is refused before it can be created, which also keeps every record
+        // reachable by `refund` (which only ever derives the canonical address).
+        const contributor = await generateKeyPairSigner();
+        svm.airdrop(contributor.address, lamports(1_000_000_000n));
+        const contributorAta = await fundAta(maker, maker, contributor.address, mint.address, 5n);
+        const notTheCanonicalPda = (await generateKeyPairSigner()).address;
+
+        sendExpectingFailure(
+            await tx(contributor, [
+                contributeIx(contributor, mint.address, fundraiser, notTheCanonicalPda, contributorAta, vault, 5n),
+            ]),
+            'contribution into a non-canonical record',
+        );
+
+        assert.equal(tokenAmount(contributorAta), 5n, 'contributor kept their tokens');
+        assert.isNotOk(svm.getAccount(notTheCanonicalPda)?.exists, 'no second record was created');
+    });
+
     it('Ignores unrecorded direct transfers into the vault', async () => {
         const maker = await generateKeyPairSigner();
         svm.airdrop(maker.address, lamports(1_000_000_000n));
@@ -414,7 +446,7 @@ describe('Token Fundraiser (Pinocchio)', () => {
         const contributor = await generateKeyPairSigner();
         svm.airdrop(contributor.address, lamports(1_000_000_000n));
         const contributorAta = await fundAta(maker, maker, contributor.address, mint.address, 10n);
-        const [contributorAccount, cbump] = await contributorPda(fundraiser, contributor.address);
+        const [contributorAccount] = await contributorPda(fundraiser, contributor.address);
         send(
             await tx(contributor, [
                 contributeIx(
@@ -424,7 +456,6 @@ describe('Token Fundraiser (Pinocchio)', () => {
                     contributorAccount,
                     contributorAta,
                     vault,
-                    cbump,
                     10n,
                 ),
             ]),