Signer authorization
If your instruction
takes in an authority
account, make sure the account has signed
the transaction.
Why? Because only the owner
of the authority
account can sign
for it—but anyone can pass in the account as a non-signer.
Don't:
pub struct Example<'info'> {
authority: AccountInfo<'info'>
}
DO:
pub struct Example<'info'> {
authority: Signer<'info'>
}
Account data matching
Make sure that passed-in accounts contain valid data.
For example, if your instruction expects a token account, the token account should contain an owner, mint, amount, etc.
Otherwise, you may be operating with the wrong type of account!
the token account can contain arbitrary, invalid data.
pub struct Example<'info'> {
token: AccountInfo<'info'>,
authority: Signer<'info'>
}
Anchor
checks that the token account contains valid data, and that its owner is the signer of the transaction.
pub struct Example<'info'> {
#[account(constraint = authorit.key == & token.owner)]
token: Account<'info', TokenAccount>,
authority: Signer<'info'>
}
Checking account ownership
Make sure the passed-in accounts are owned by the correct program.
For example, if your instruction expects a token account, it should be owned by the token program.
this code doesn't check to make sure the token account is owned
by the SPL token program, so it could be invalid.
let token = SplTokenAccount: : unpack (&ctx.accounts.token.data.borrow ( ) )?;
if ctx.accounts.authority.key != &token.owner {
return Err (ProgramError::InvalidAccountData);
}
msg! ("Your account balance is: {}", token. amount);
Anchor
will verify account ownership for you!
pub struct Example<'info> {
#[account (constraint = authority.key == &token.owner)] token: Account<'info, TokenAccount>,
authority: Signer<'info>,
}
Type cosplay
Make sure one account type (e.g. User) can't be confused for another account type (e.g. Metadata).
This one is a bit confusing, the examples should make it clearer.
you can't tell a deserialized User account from a deserialized Metadata account
#[derive (BorshSerialize, BorshDeserialize)]
pub struct User {
authority: Pubkey,
}
#[derive (BorshSerialize, BorshDeserialize)]
pub struct Metadata {
account: Pubkey,
}
The manual way to fix this is by adding a "discriminant" to both accounts, i.e. something that allows you to distinguish between them.
#[derive (BorshSerialize, BorshDeserialize)]
pub struct User { discriminant: AccountDiscriminant,
authority: Pubkey,
}
#[derive (BorshSerialize, BorshDeserialize)]
pub struct Metadata { discriminant: AccountDiscriminant,
account: Pubkey,
}
#[derive (BorshSerialize, BorshDeserialize, PartialEq) ]
pub enum AccountDiscriminant {
User,
Metadata,
}
The recommended way to fix this is by just using Anchor's #[account] macro, which will automatically add an 8-byte discriminator to the start of the account. Much easier!
#[account]
pub struct User {
authority: Pubkey,
}
#[account]
pub struct Metadata {
account: Pubkey,
}
Account initialization
This is similar to the previous vulnerability—when initializing accounts, make sure you account for the discriminator.
E.g., you don't want to initialize the wrong type of account. And you may not want to re-initialize an already initialized account.
The user account could be another account type (since there is no discriminator). And this also lets people re-initialize previously initialized accounts.
#[derive (Accounts) 1
pub struct Initialize<'info> { user: AccountInfo<' info>,
authority: Signer<'info>,
}
#[derive (BorshSerialize, BorshDeserialize)]
pub struct User {
authority: Pubkey,
}
pub fn initialize (ct: Context<Initialize>) -> ProgramResult {
let mut user = User::try_from slice(&ct.accounts.user.data.borrow () ) . unwrap ( ) ;
user.authority = ct.accounts.authority.key ( );
let mut storage = ct.accounts.user.try borrow mut data)?;
user.serialize(storage.deref mut ( )) .unwrap ( ) ;
Ok(())
}
using #[account(init)] will create a new account and set its account discriminator.
#[derive Accounts) 1
pub struct Init‹'info> ‹
#[account (init, payer = authority, space = 8+32)] user: Account<'info, User>,
authority: Signer<'info>,
system program: Program<'info, System>,
}
Arbitrary CPI
When performing CPIs, make sure you're invoking the correct program.
The token program account gets passed in by the user, and could actually be some other program
pub fn cpi(ct: Context<Cpi>, amount: u64) -> ProgramResult {
solana_program::program::invoke(
&sp1_token::instruction::transfer(
ctx.accounts.token_program.key,
ctx.accounts.source.key,
ct.accounts.destination.key,
ctx.accounts.authority.key,
&[],
amount,
)?,
& [
ctx.accounts.source.clone(),
ctx.accounts.destination.clone(),
ctx.accounts.authority.clone(),
],
)
}
The manual way to fix this is checking to make sure the token program account has the right address.
pub fn cpi_secure (ct: Context<Cpi>, amount: u64) -> ProgramResult {
if &sp1_token::ID == ctx.accounts.token_program.key {
return Err (ProgramError:: IncorrectProgramId);
}
solana_program::program::invoke(
&spl_token::instruction::transfer(
ctx.accounts.token_program.key,
ctx.accounts.source.key,
ctx.accounts.destination.key,
ctx.accounts.authority.key,
&[],
amount,
)?,
& [
ctx.accounts.source.clone(),
ctx.accounts.destination.clone(),
ct.accounts.authority.clone
],
)
}
The recommended way to fix this is by using Anchor's wrapper of the SPL token program.
use anchor_spl::token:: {self, Token, TokenAccount};
#[program]
pub mod arbitrary pi recommended {
use super::*;
pub fn cpi (ctx: Context<Cpi>, amount: u64) -> ProgramResult {
token:: transfer (ctx.accounts.transfer_ctx(), amount)
}
}
#[derive (Accounts) ]
pub struct Cpi<'info> {
source: Account<'info, TokenAccount>,
destination: Account<'info, TokenAccount>,
authority: Signer<' info>,
token program: Program<' info, Token>,
}
impl<'info> Cpi<'info> {
pub fn transfer ct(&self) -> CpiContext<'_, '_, '_, 'info, token::Transfer<'info>> {
let program = self.token_program.to_account_info();
let accounts = token:: Transfer {
from: self.source.to_account_info(),
to: self.destination.to_account_info(),
authority: self.authority.to_account_info(),
};
CpiContext:: new (program, accounts)
}
}
Duplicate mutable accounts
If your program takes in two mutable accounts of the same type, make sure people don't pass in the same account twice.
user_a and user_b may be the same account.
pub fn update (ct: Context<Update>, a: u64, b: u64) -> ProgramResult {
let user_a = &mut ctx.accounts.user_a;
let user_b = &mut ctx.accounts.user_b;
user_a.data = a;
user_b.data = b;
Ok(())
}
#[derive (Accounts)]
pub struct Update‹'info> {
user_a: Account<'info, User>,
user_b: Account<'info, User>,
}
use Anchor to verify that the two accounts are different.
#[derive (Accounts) ]
pub struct Update<'info> {
#[account (constraint = user_a.key() != user_b.key())]
user_a: Account<'info, User>,
user_b: Account<'info, User>,
}
Bump seed canonicalization
Often, you want to have a single PDA associated with a program ID + a set of seeds.
For example, associated token accounts (ATAs).
Thus, when verifying PDAs
, you should use find_program_address
instead of create_program_address
.
Since the bump is passed in by the user (and not verified), set_value could operate on multiple PDAs associated with the program ID + the set of seeds.
pub fn set_value (ct: Context<BumpSeed>, key: u64, new_value: u64, bump: u8) -> ProgramResult {
let address = Pubkey::create_program_address (&[key.to_le_bytes().as_ref (), &[bump]], ctx.program_id)?;
if address != ct.accounts.data.key() {
return Err (ProgramError: :InvalidArgument);
}
ctx.accounts.data.value = new value;
Ok(())
}
Both the PDA address and bump are verified, which means set_value
will only operate on one canonical
PDA.
pub fn set value secure( ct: Context<BumpSeed>, key: u64, new_value: u64, bump: u8,) -> ProgramResult {
let (address, expected bump) =
Pubkey::find_program_address (&[key.to_le_bytes().as_ref (), &[bump]], ctx.program_id);
if address != ctx.accounts.data.key () {
return Err (ProgramError::InvalidArgument);
}
if expected bump != bump {
return Err (ProgramError:: InvalidArgument);
}
ctx.accounts.data.value = new value;
Ok(())
}