security: add missing authorization checks in close-account and transfer-sol

[NikkiAung]

Summary

Audited every native/pinocchio (non-Anchor — no macro-generated account-validation safety net) instruction handler under basics/ for the classic Solana vulnerability classes: missing signer checks, missing owner checks before deserialization, missing PDA re-derivation, arbitrary CPI, duplicate-account aliasing. Found two real, currently-shipping vulnerabilities. Both are invisible to the existing test suites — every existing test only exercises the honest path (the same signer creates and later acts on their own account), so green CI gives false confidence.

This matters more here than in most repos: program-examples is code thousands of learners copy-paste directly into real projects, so a vulnerability in a teaching example teaches the vulnerability to everyone who copies it. Both are the textbook "missing signer authorization" pattern — the most common real-world Solana exploit class.

1. basics/close-account (native + pinocchio): anyone can close and drain anyone else's account

close_user / process_close took target_account and payer, and unconditionally moved nearly all of target_account's lamports into payer, then reassigned it to the System Program. It never checked payer.is_signer, and never re-derived the ["USER", payer.key] PDA to confirm target_account actually belongs to payer.

Exploit: submit CloseUser with [victim's User PDA, attacker's own pubkey, System Program]. The attacker doesn't need the victim's key at all — they sign with their own key (satisfying "some signer exists"), pass the victim's PDA as the account to close, and drain it. PDAs are discoverable via getProgramAccounts.

The Anchor version of this same example was already safe — user: Signer<'info> enforces is_signer, and seeds = [b"USER", user.key().as_ref()] on user_account makes Anchor auto-verify the PDA. Only the native/pinocchio ports dropped both checks when translating away from the framework.

Fix: require payer.is_signer, and re-derive + compare the expected PDA before touching target_account.

2. basics/transfer-sol (anchor + native + pinocchio — all three): anyone can drain any program-owned account

transfer_sol_with_program directly manipulates lamports (**payer.try_borrow_mut_lamports()? -= amount) with no CPI and no is_signer check anywhere. Since the Solana runtime only requires that the owning program initiate a lamport debit (no signature required by the runtime itself), any account owned by this program can be drained by anyone who knows its pubkey.

This is the one case where the bug is not an Anchor-vs-native gap — the Anchor version has it too:

/// CHECK: Use owner constraint to check account is owned by our program
#[account(mut, owner = id())]
payer: UncheckedAccount<'info>,

owner = id() confirms the account is program-owned — which the runtime requires anyway for the debit to succeed at all, so it doesn't add real protection — but never checks is_signer. The top-level basics/transfer-sol/README.md presents this as a working pattern with zero caveat about needing an authorization check, which is exactly the shape of bug that has caused real fund-loss incidents in production "vault" programs holding user funds in program-owned accounts.

Fix: require payer.is_signer in all three implementations. For Anchor, payer changes from UncheckedAccount<'info> to Signer<'info> (constraints like mut/owner compose with either account type, so this is a minimal, idiomatic change — not adding a redundant owner check to native/pinocchio since the runtime already refuses the debit unless the executing program owns the account).

Test plan

Every fix ships with a new adversarial test proving the exploit is blocked. Each one was verified to fail against the unpatched code first (confirming it actually exercises the bug, not a false negative), then verified to pass after the fix, with every pre-existing legitimate-flow test still passing unchanged:

• close-account/native: attacker (own signer, victim's PDA as target) — fails before fix, blocked after. 3/3 tests pass.
• close-account/pinocchio: same attack in Rust via cargo test — fails before fix, blocked after. 1/1 tests pass.
• transfer-sol/anchor (litesvm): attacker marks victim's program-owned account isSigner: false, only signs with their own unrelated fee-payer key — fails before fix, blocked after. 3/3 tests pass.
• transfer-sol/native: same attack via raw @solana/kit account-role manipulation — fails before fix, blocked after. 4/4 tests pass.
• transfer-sol/pinocchio: same attack via raw AccountMeta — fails before fix, blocked after. 1/1 tests pass. (Caught and fixed a flaw in my first draft of this test: the initial victim account had only ever received lamports rather than being created via create_account, so it was actually System-Program-owned and the runtime blocked the debit for an unrelated reason. Recreated the victim properly via create_account with this program as owner so the test exercises the real vulnerability.)
• transfer-sol/anchor/tests/test.ts (the validator-based suite, not litesvm): updated the existing "Transfer SOL with Program" test to add .signers([payerAccount]), since payer is now Signer<'info> and must actually sign — this is a necessary, expected update to the legitimate flow, not a new issue. Could not execute this specific file locally (solana-test-validator/surfpool hangs at startup in this sandbox — unrelated to the change); the identical on-chain program logic is already fully exercised and verified via the litesvm.test.ts suite above, which hits the same compiled program through a full transaction-processing path.

All programs build clean (cargo check / anchor build / cargo build-sbf) with no new warnings beyond pre-existing ones.

[greptile-apps]

Greptile Summary

The PR adds signer and PDA-binding authorization checks to the close-account and transfer-sol examples, with adversarial regression tests across native, Pinocchio, and Anchor implementations.

• Validates close-account targets against the signing payer’s derived User PDA.
• Requires authorization before directly debiting program-owned SOL accounts.
• Adds tests that assert the specific authorization errors returned for attempted attacks.
• Adds test-only Solana error dependencies needed for typed error assertions.

Confidence Score: 4/5

The code changes appear sound, but the PR must not be merged until its three unsigned commits are replaced with verified commits.

No blocking code failure remains, but every commit in the PR reports “No signature,” which violates the repository’s verified-commit requirement.

Important Files Changed

Filename	Overview
basics/close-account/native/program/src/instructions/close_user.rs	Adds payer signature validation and binds the target account to the payer-derived User PDA before closing it.
basics/close-account/pinocchio/program/src/lib.rs	Mirrors the native close-account authorization checks in the Pinocchio implementation.
basics/transfer-sol/anchor/programs/transfer-sol/src/lib.rs	Replaces the unchecked program-transfer payer with a signer while retaining the program-owner constraint.
basics/transfer-sol/native/program/src/instruction.rs	Rejects direct lamport debits unless the payer account authorized the transaction.
basics/transfer-sol/pinocchio/program/src/lib.rs	Adds the corresponding payer signature requirement to the Pinocchio direct-transfer path.
Cargo.toml	Adds workspace test dependencies for matching concrete instruction and transaction errors.
Cargo.lock	Records new dependency edges for existing Solana error packages.

_{Reviews (3): Last reviewed commit: "test(#667): assert on the specific error..." | Re-trigger Greptile}

[NikkiAung]

Hey @dev-jodee — bumping this for review when you get a chance. CI is green and Greptile came back clean (5/5, no blocking findings). Thanks!

[dev-jodee]

thx ! all the same comments, assert on real error instead of just "an error happened"

[NikkiAung]

Hey @dev-jodee — done fixing, all 5 spots now assert on the specific error instead of a generalized failure check:

• close-account (native + pinocchio): IncorrectProgramId
• transfer-sol native + pinocchio: MissingRequiredSignature
• transfer-sol anchor: Anchor's AccountNotSigner (code 3010)

CI is green. Let me know if anything else needs a look!