Skip to content

fix(pda-rent-payer): make native/pinocchio create_new_account require a signer and create the account via CPI - #7

Open
SwineCoder101 wants to merge 2 commits into
mainfrom
fix/pda-rent-payer-port-parity
Open

fix(pda-rent-payer): make native/pinocchio create_new_account require a signer and create the account via CPI#7
SwineCoder101 wants to merge 2 commits into
mainfrom
fix/pda-rent-payer-port-parity

Conversation

@SwineCoder101

Copy link
Copy Markdown
Owner

Bug: native and pinocchio create_new_account hand rent-vault lamports to any account instead of creating one

The anchor reference requires new_account to sign and creates it with a system_program::create_account CPI funded by the rent-vault PDA. The native and pinocchio ports skip both: they never check new_account.is_signer and simply move minimum_balance(0) lamports from the vault into whatever key is passed. Any caller can therefore direct vault lamports into an existing wallet of their choice, and the "new account" the README describes is never actually created. The pinocchio port additionally trusts a client-supplied bump validated only with create_program_address, so a non-canonical bump selects an alternate vault PDA. This is a correctness/security bug in the ports, not a style difference.

Affected

  • native/program/src/instructions/create_new_account.rs
  • native/program/src/instructions/init_rent_vault.rs (vault owner, see Fix)
  • pinocchio/program/src/instructions/create_new_account.rs
  • pinocchio/program/src/instructions/init_rent_vault.rs (vault owner, see Fix)
  • anchor/ is the reference and is unchanged.

Functionality

init_rent_vault funds a ["rent_vault"] PDA. create_new_account uses that PDA as the payer to create a fresh, empty, system-owned account for a signing keypair, transferring exactly Rent::minimum_balance(0) from the vault (anchor: CreateNewAccount { new_account: Signer, rent_vault: SystemAccount, system_program } + create_account CPI signed with the vault seeds).

The bug

  1. Missing signer check and no account creation.
    • native/program/src/instructions/create_new_account.rs:11-28 (before the fix): new_account is read with next_account_info and never checked for is_signer; the instruction ends with **rent_vault.lamports.borrow_mut() -= ...; **new_account.lamports.borrow_mut() += ... (lines 25-26).
    • pinocchio/program/src/instructions/create_new_account.rs:9-32 (before the fix): same shape, rent_vault.set_lamports(...) / new_account.set_lamports(...) at lines 29-30, no is_signer() check.
    • Because the vault was created with owner = program_id (init_rent_vault.rs), the program may debit it directly, so the runtime does not stop the transfer. Scenario: an attacker passes their own funded wallet as new_account (writable, not a signer) and receives minimum_balance(0) from the vault; nothing is created and nothing requires the wallet's signature. Passing an existing wallet that does sign also succeeds, which the anchor version rejects with AccountAlreadyInUse.
  2. Non-canonical bump (pinocchio only).
    • pinocchio/program/src/instructions/create_new_account.rs:18-23 (before the fix): bump = instruction_data[0] is fed to Address::create_program_address, which accepts any off-curve bump. A vault initialised at a non-canonical bump is then accepted as the rent payer. The native port derives with find_program_address and is not affected.

Reproduce

cd basics/pda-rent-payer/native && pnpm install --frozen-lockfile && pnpm build-and-test
cd basics/pda-rent-payer/pinocchio && pnpm install --frozen-lockfile && pnpm build-and-test

Tests added in native/tests/test.ts and pinocchio/tests/test.ts:

  • Rejects paying vault lamports into an existing wallet that does not sign
  • Rejects paying vault lamports into an existing wallet even when it signs
  • Rejects a non-canonical rent vault bump (pinocchio only)
  • Create a new account using the Rent Vault now asserts the created account exists, holds exactly minimum_balance(0), is system-owned with zero data, and that the vault decreased by exactly that amount.

Against the unmodified programs:

  PDA Rent-Payer
    ✔ Initialize the Rent Vault
    ✔ Create a new account using the Rent Vault
    1) Rejects paying vault lamports into an existing wallet that does not sign
    2) Rejects paying vault lamports into an existing wallet even when it signs
    3) Rejects a non-canonical rent vault bump          (pinocchio)

  1) AssertionError: unsigned new_account must be rejected
  2) AssertionError: an already existing new_account must be rejected
  3) AssertionError: non-canonical bump must be rejected

(native: 2 passing, 2 failing; pinocchio: 2 passing, 3 failing.)

Fix

  • create_new_account (both ports) now returns MissingRequiredSignature unless new_account is a signer, and creates the account with a system_program::create_account CPI (lamports = minimum_balance(0), space = 0, owner = system program) signed with the vault's seeds and canonical bump, exactly as the anchor reference does. The system program enforces the rest: new_account must sign and must not already exist (AccountAlreadyInUse), so lamports can no longer be pushed into an existing wallet.
  • pinocchio create_new_account derives the vault with Address::find_program_address and rejects the instruction with InvalidSeeds unless the supplied bump equals the canonical one and the passed account matches the derived address. The instruction data layout ([1, bump]) is unchanged.
  • init_rent_vault (both ports) now creates the vault owned by the system program instead of program_id. A create_account CPI can only debit a system-owned from, and this matches anchor's rent_vault: SystemAccount. The vault's balance (minimum_balance(0) + fund_lamports) is unchanged.
  • Test adjustments: Initialize the Rent Vault additionally asserts the vault is system-owned. No test relied on the old behaviour beyond the happy path, which still passes. The Rust litesvm tests in program/tests/test.rs are unchanged and still pass.

Known limitation (out of scope, unchanged)

In all three variants create_new_account has no authority: anyone who can send a transaction can drain the vault minimum_balance(0) at a time by creating fresh keypairs. This is consistent across anchor, native and pinocchio, and the README does not claim otherwise, so it is left as-is here for the maintainers to decide whether the example should gate the instruction behind the vault's funder.

pinocchio init_rent_vault still accepts a client-supplied bump via create_program_address. After this fix a vault initialised at a non-canonical bump is simply unusable (it can never be the payer), so this is harmless, and it is left unchanged to keep the diff focused.

Verification

cd basics/pda-rent-payer/native && pnpm build-and-test
  PDA Rent-Payer
    ✔ Initialize the Rent Vault
    ✔ Create a new account using the Rent Vault
    ✔ Rejects paying vault lamports into an existing wallet that does not sign
    ✔ Rejects paying vault lamports into an existing wallet even when it signs
  4 passing

cd basics/pda-rent-payer/pinocchio && pnpm build-and-test
  PDA Rent-Payer
    ✔ Initialize the Rent Vault
    ✔ Create a new account using the Rent Vault
    ✔ Rejects paying vault lamports into an existing wallet that does not sign
    ✔ Rejects paying vault lamports into an existing wallet even when it signs
    ✔ Rejects a non-canonical rent vault bump
  5 passing

cargo test --manifest-path=./program/Cargo.toml     (native and pinocchio)
  test test_pda_rent_payer ... ok

cargo clippy -p pda-rent-payer-program -- -D warnings
cargo clippy -p pda-rent-payer-pinocchio-program -- -D warnings
cargo fmt --check

…to unsigned/existing wallets and non-canonical vaults

native and pinocchio: an existing funded wallet passed as new_account,
with or without its signature, receives rent-vault lamports instead of
being rejected. pinocchio additionally accepts a non-canonical rent_vault
bump, letting an alternate PDA act as the rent payer. The happy-path test
now asserts the created account's lamports, owner, data length and the
exact vault delta.
… a signer and create the account via CPI

Match the anchor reference: new_account must sign and is created with a
system_program create_account CPI (space 0, system-owned) funded by the
rent-vault PDA, so vault lamports can no longer be paid into an existing
wallet. The vault is now created as a system-owned account so it can fund
the CPI. pinocchio derives the vault with find_program_address and rejects
non-canonical bumps. Tests assert the vault owner. The unauthenticated-drain design
limitation shared by all variants is out of scope and left as-is.
@SwineCoder101
SwineCoder101 force-pushed the fix/pda-rent-payer-port-parity branch from 6b33763 to d97641b Compare August 27, 2026 12:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant