fix: audit remediations for external signatures

[0xLeo-sqds]

Summary

On-chain fixes for security issues identified during the external signatures audit.

Fixes

1. Payload hash binding in async creation messages

Issue: create_transaction_v2, create_settings_transaction_v2, and add_transaction_to_batch_v2 construct signed messages that don't include the payload. A malicious relayer can take an external signer's signature and attach it to a completely different payload.

Fix: Added payload_hash parameter to create_transaction_message, create_settings_transaction_create_message, and create_batch_add_transaction_message. Callers hash the instruction args before constructing the message. This matches what the sync path already does.

Files: messages.rs, transaction_create.rs, settings_transaction_create.rs, batch_add_transaction.rs

2. SettingsState expiration + precompile sysvar conflict

Issue: In the async path, is_active() expects the Settings account at remaining_accounts[0] for SettingsState expiration, while precompile verification expects the instructions sysvar at remaining_accounts[0]. External signers using precompile verification are blocked from participating in policies with SettingsState expiration.

Fix: Strip the instructions sysvar from remaining_accounts before passing to is_active(), matching the sync flow pattern. verify_signer handles the sysvar internally via split_instructions_sysvar().

Files: transaction_execute.rs, transaction_create.rs, proposal_create.rs, proposal_vote.rs, transaction_create_from_buffer.rs

3. Constraint evaluation ordering relative to pre-hook

Issue: evaluate_instruction_constraints runs before pre-hook execution. Constraints validate account state that exists before the pre-hook runs, but inner instructions execute against post-hook state.

Fix: Moved evaluate_instruction_constraints() after pre-hook execution in both execute_payload_async and execute_payload_sync.

Files: program_interaction.rs

4. num_signer bounds check in synchronous consensus

Issue: num_signer (user-provided u8) is used as a slice bound on remaining_accounts without validation. If num_signer > remaining_accounts.len(), the program panics.

Fix: Added require!((num_signer as usize) <= remaining_accounts.len()) before the slice access.

Files: context_validation.rs

5. Sync session-key signer privilege leak in inner CPIs

Issue: SynchronousTransactionMessage::new_validated stripped is_signer from inner CPI accounts by matching against canonical signer keys (SmartAccountSigner::key()). Session keys have a raw address that differs from the parent's canonical key, so they were never matched — retaining is_signer=true in inner CPI calls.

Fix: Collect raw AccountInfo keys from the outer signer accounts instead of canonical keys from the signer list.

Files: transaction_execute_sync.rs, transaction_execute_sync_legacy.rs, synchronous_transaction_message.rs

6. Policy sync signer stripping (same class as #5)

Issue: execute_payload_sync in the policy execution engine had the same canonical-key stripping bug as #5. Session keys authenticated against a Policy could retain signer privilege in inner CPIs.

Fix: Same pattern — collect raw AccountInfo keys filtered by is_signer instead of canonical SmartAccountSigner::key() values.

Files: program_interaction.rs

7. Session key create/revoke generalized to ConsensusAccount

Issue: create_session_key and revoke_session_key were hardcoded to Account<Settings>. The Consensus trait already supports session keys on Policies (find_signer_by_session_key, verify_signer), but there was no instruction to create or revoke session keys on Policy accounts — leaving policy session keys with no lifecycle management.

Fix: Generalized both instructions to InterfaceAccount<ConsensusAccount> with check_derivation constraint. Added signers_mut() to the Consensus trait for mutable signer access through the interface.

Files: create_session_key.rs, revoke_session_key.rs, consensus_trait.rs, consensus.rs, settings.rs, policy.rs

8. MigrateToV2 on already-V2 accounts silently nukes proposals

Issue: MigrateToV2 calls force_v2() (no-op if already V2) then invalidate_prior_transactions() unconditionally. On an already-V2 account, this does nothing useful but invalidates all in-flight proposals as a side effect.

Fix: Added require!(matches!(self.signers, SmartAccountSignerWrapper::V1(_))) guard — rejects with InvalidInstructionArgs if signers are already V2.

Files: settings.rs

9. add_signer reverse session key collision

Issue: create_session_key prevents forward collisions (session key != any signer key), but add_signer does NOT check the reverse: whether the new signer's canonical key collides with an existing session key. If a collision exists, the session key holder authenticates as the wrong signer (with potentially different permissions) — a governance threshold bypass.

Fix: Added has_session_key_assigned(&signer.key()) check in add_signer, mirroring the forward check in create_session_key.

Files: wrapper.rs

10. resolve_signer_key does not check session key expiration

Issue: resolve_signer_key matched session keys via get_session_key_data_if_matches without checking expiration. An expired session key could still resolve to the parent signer's canonical key, affecting buffer close authorization and event logging accuracy.

Fix: Added Clock::get() expiration check in the session key match path — expired session keys now fail with NotASigner.

Files: consensus_trait.rs

11. Cancel realloc fails for external signers

Issue: cancel_proposal_inner uses signer.to_account_info() as rent payer for realloc_if_needed. External signers' AccountInfo is not a native tx signer and may lack lamports, causing the system transfer CPI to fail.

Fix: If the signer is not a native tx signer (not is_signer && is_writable), fall back to the last remaining_account (must be is_signer && is_writable).

Files: proposal_vote.rs

12. Buffer close = creator sends lamports to unspendable address

Issue: TransactionBuffer stores creator as the canonical governance key. For P256/secp256k1 signers, this is derived from non-ed25519 curve material — nobody holds the private key. Anchor's close = creator permanently locks lamports at that address.

Fix: Added rent_collector: Pubkey field to TransactionBuffer, set to rent_payer.key() at creation time. Changed close = creator to close = rent_collector with has_one = rent_collector in both CloseTransactionBuffer and CreateTransactionFromBuffer. Creator remains the authorization authority.

Files: transaction_buffer.rs, transaction_buffer_create.rs, transaction_buffer_close.rs, transaction_create_from_buffer.rs

Note: we know this is a breaking changes but there are zero buffer opened on mainnet currently so this is okay to do so.