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
Open
Conversation
…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
force-pushed
the
fix/pda-rent-payer-port-parity
branch
from
August 27, 2026 12:23
6b33763 to
d97641b
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bug: native and pinocchio
create_new_accounthand rent-vault lamports to any account instead of creating oneThe anchor reference requires
new_accountto sign and creates it with asystem_program::create_accountCPI funded by the rent-vault PDA. The native and pinocchio ports skip both: they never checknew_account.is_signerand simply moveminimum_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 withcreate_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.rsnative/program/src/instructions/init_rent_vault.rs(vault owner, see Fix)pinocchio/program/src/instructions/create_new_account.rspinocchio/program/src/instructions/init_rent_vault.rs(vault owner, see Fix)anchor/is the reference and is unchanged.Functionality
init_rent_vaultfunds a["rent_vault"]PDA.create_new_accountuses that PDA as the payer to create a fresh, empty, system-owned account for a signing keypair, transferring exactlyRent::minimum_balance(0)from the vault (anchor:CreateNewAccount { new_account: Signer, rent_vault: SystemAccount, system_program }+create_accountCPI signed with the vault seeds).The bug
native/program/src/instructions/create_new_account.rs:11-28(before the fix):new_accountis read withnext_account_infoand never checked foris_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, nois_signer()check.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 asnew_account(writable, not a signer) and receivesminimum_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 withAccountAlreadyInUse.pinocchio/program/src/instructions/create_new_account.rs:18-23(before the fix):bump = instruction_data[0]is fed toAddress::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 withfind_program_addressand is not affected.Reproduce
Tests added in
native/tests/test.tsandpinocchio/tests/test.ts:Rejects paying vault lamports into an existing wallet that does not signRejects paying vault lamports into an existing wallet even when it signsRejects a non-canonical rent vault bump(pinocchio only)Create a new account using the Rent Vaultnow asserts the created account exists, holds exactlyminimum_balance(0), is system-owned with zero data, and that the vault decreased by exactly that amount.Against the unmodified programs:
(native: 2 passing, 2 failing; pinocchio: 2 passing, 3 failing.)
Fix
create_new_account(both ports) now returnsMissingRequiredSignatureunlessnew_accountis a signer, and creates the account with asystem_program::create_accountCPI (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_accountmust sign and must not already exist (AccountAlreadyInUse), so lamports can no longer be pushed into an existing wallet.create_new_accountderives the vault withAddress::find_program_addressand rejects the instruction withInvalidSeedsunless 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 ofprogram_id. Acreate_accountCPI can only debit a system-ownedfrom, and this matches anchor'srent_vault: SystemAccount. The vault's balance (minimum_balance(0) + fund_lamports) is unchanged.Initialize the Rent Vaultadditionally asserts the vault is system-owned. No test relied on the old behaviour beyond the happy path, which still passes. The Rust litesvm tests inprogram/tests/test.rsare unchanged and still pass.Known limitation (out of scope, unchanged)
In all three variants
create_new_accounthas no authority: anyone who can send a transaction can drain the vaultminimum_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_vaultstill accepts a client-supplied bump viacreate_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