security: gate verify_collection behind the collection's original creator

[NikkiAung]

Summary

tokens/nft-operations has an NFT-collection-verification forgery vulnerability, present
identically in both the anchor and pinocchio implementations: verify_collection
CPIs into Metaplex's VerifyCollectionV1/Verify instruction, signed by this program's
[b"authority"] PDA — the same PDA create_collection sets as the collection's Metaplex
update_authority. Neither implementation checks who is allowed to call
verify_collection. Anchor requires authority: Signer<'info> but never compares it to
anything; Pinocchio checks only payer.is_signer(). The PDA itself is global (fixed seed, no
per-collection component) and signs unconditionally for whoever invokes the instruction.

The attack

create_collection is fully permissionless, and mint_nft lets anyone mint an NFT with an
unverified collection field pointing at any collection_mint (freely settable — an
unverified claim requires no permission). So:

1. Attacker mints their own NFT via mint_nft, claiming membership in a victim's real,
   already-created collection.
2. Attacker calls verify_collection, supplying the victim's public collection accounts and
   their own throwaway signer.
3. The program derives the same global PDA (genuinely the victim collection's real
   update_authority) and signs — Metaplex's own checks pass, since from its perspective the
   signer is exactly right. It has no way to know an unrelated caller triggered this.
4. The attacker's fake NFT is now permanently verified: true as an authentic member of the
   victim's collection — a real NFT-marketplace forgery primitive, since most marketplaces
   and wallets only trust the verified badge.

Scoping the signing PDA per collection would not fix this — I checked this carefully
before implementing. PDA derivation is deterministic and public: an attacker supplying the
victim's real collection_mint would still cause the program to derive and sign with that
exact collection's genuine authority. The fix has to be an explicit check on the caller's
identity, not a change to what signs.

Why this went unnoticed

• The anchor README (tokens/nft-operations/anchor/readme.MD) describes authority as:
  "signer of the transaction. This can be used to restrict the address that can execute the
  verify collection method, by adding constraints" — a vague hint that constraints could
  be added, but none actually are, and the concrete consequence is never spelled out. Much
  softer than this repo's compression/cnft-vault README, which explicitly and prominently
  discloses its own analogous gap as an intentional proof-of-concept limitation — I checked
  and ruled that one out as already-disclosed; this one reads differently.
• The pinocchio test file has a comment revealing the author's actual mental model:
  "Metaplex Verify performs strict checks: the signer must be the collection's update
  authority (our PDA)... A successful transaction therefore proves the whole flow is
  correct." This shows the gap wasn't a deliberate simplification — Metaplex's own checks
  really are correct, but they say nothing about who invoked this program's wrapping
  instruction.
• Neither test suite (anchor/tests/litesvm.test.ts, pinocchio/tests/test.ts) ever called
  verify_collection with a different signer than the one who created the collection — zero
  coverage of the vulnerable path.

Fix

Adds a collection_authority PDA (seeded by collection_mint) recording the collection's
actual creator at create_collection time, checked in verify_collection.
create_collection stays fully permissionless — the new account only gates verifying
someone else's collection. Per-collection scoping is necessary, not just stylistic: a
global tracking account would just relocate the bug (whoever creates the first collection
would become "the" authorized verifier for every collection).

• Anchor: new state.rs (CollectionAuthority { creator: Pubkey }) and errors.rs
  (MintNftError::Unauthorized), wired in via init/seeds constraints on both contexts.
• Pinocchio: this program has no existing custom-account infrastructure, so the new PDA
  stores just the raw 32-byte creator pubkey (no discriminator — matches the existing
  precedent in basics/favorites/pinocchio, safe since the account only ever exists at its
  derived address if this program's own create_collection created it there). Since
  Pinocchio gives none of Anchor's automatic owner/length checks, verify_collection
  explicitly verifies collection_authority.owner() == program_id and the correct data
  length before trusting its contents — dropping that check is exactly the class of gap a
  prior round in this series (security: add missing authorization checks in close-account and transfer-sol #667) found in other Pinocchio ports.

Test changes

Both test suites gained a negative test: a second, unrelated signer attempts
verify_collection on the first signer's collection.

• Anchor: asserts the Anchor custom error code Unauthorized.
• Pinocchio: asserts the specific InstructionErrorCustom { code: 1 } and the paired
  on-chain log message, not just "the transaction failed" — confirmed empirically what
  LiteSVM's error/log shapes actually look like before writing the assertion, rather than
  assuming.

Verified for both implementations: reverted just the authorization check (kept the new
account plumbing so it still compiles), rebuilt, confirmed an outsider's verify_collection
call succeeds against the unpatched code, reapplied, confirmed it's rejected.

Out of scope

• mint_nft's unrestricted collection_mint parameter — safe by construction, since an
  unverified claim grants no privilege on its own.
• Lack of PDA-seeds constraints on metadata/master_edition/collection_metadata — this
  example consistently leans on Metaplex's own instruction processors to re-derive and
  validate these against the mint; not a gap this fix needs to touch.
• README documentation drift (the embedded code snippets and the vague "can be used to
  restrict..." line will go stale) — worth a follow-up docs-only edit, not done here.

Verification

• anchor build --ignore-keys, full litesvm.test.ts suite: 4/4 passing.
• cargo build-sbf + pinocchio test suite: 4/4 passing.
• cargo clippy -- -D warnings and cargo fmt --check clean for the Pinocchio crate (a root
  workspace member, unlike the Anchor crate which is .workspace-ignored).
• tsc --noEmit and root prettier --check clean for both.

🤖 Generated with Claude Code

[greptile-apps]

Greptile Summary

The PR closes an NFT collection-verification forgery path by recording each collection's creator in a per-collection PDA and requiring that creator to authorize verification.

• Adds equivalent creator-tracking state and authorization checks to the Anchor and Pinocchio implementations.
• Validates the Pinocchio tracking account's address, owner, length, and stored creator before invoking Metaplex.
• Updates instruction account layouts and adds negative tests proving unrelated signers are rejected.

Confidence Score: 5/5

The PR appears safe to merge and correctly gates collection verification in both implementations.

The new per-collection authority state is initialized from the creator, canonically derived during verification, and checked before the program signs the Metaplex CPI; corresponding authorized and unauthorized paths are covered by updated tests.

Important Files Changed

Filename	Overview
tokens/nft-operations/anchor/programs/mint-nft/src/contexts/create_collection.rs	Initializes a mint-scoped authority account and records the collection creator.
tokens/nft-operations/anchor/programs/mint-nft/src/contexts/verify_collection.rs	Requires the canonical authority account and rejects callers other than its recorded creator.
tokens/nft-operations/anchor/programs/mint-nft/src/state.rs	Defines correctly sized Anchor state for the collection creator.
tokens/nft-operations/anchor/tests/litesvm.test.ts	Updates account plumbing and verifies that an unrelated signer receives the expected authorization error.
tokens/nft-operations/pinocchio/program/src/instructions/create_collection.rs	Creates the canonical tracking PDA and stores the creator's raw public key.
tokens/nft-operations/pinocchio/program/src/instructions/verify_collection.rs	Validates the tracking PDA's address, ownership, length, and creator before signing the Metaplex CPI.
tokens/nft-operations/pinocchio/tests/test.ts	Updates positional account layouts and asserts both the custom error and authorization log for an outsider.

_{Reviews (1): Last reviewed commit: "security: gate verify_collection behind ..." | Re-trigger Greptile}

[NikkiAung]

Hey @dev-jodee — this is ready for review whenever you get a chance. CI is green for both the anchor and pinocchio builds, and Greptile's automated pass found no blocking issues.