From c922175fe8ef0df913a52f04a4f8659d03e801ec Mon Sep 17 00:00:00 2001 From: techvoyagerX Date: Mon, 9 Sep 2024 03:29:48 -0400 Subject: [PATCH] Added comments to 5-arbitrary-cpi in secure, recommended, and insecure versions --- programs/5-arbitrary-cpi/insecure/src/lib.rs | 5 ++++- programs/5-arbitrary-cpi/recommended/src/lib.rs | 7 +++++++ programs/5-arbitrary-cpi/secure/src/lib.rs | 6 +++++- 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/programs/5-arbitrary-cpi/insecure/src/lib.rs b/programs/5-arbitrary-cpi/insecure/src/lib.rs index 58fe906..1705589 100644 --- a/programs/5-arbitrary-cpi/insecure/src/lib.rs +++ b/programs/5-arbitrary-cpi/insecure/src/lib.rs @@ -7,10 +7,12 @@ declare_id!("Fg6PaFpoGXkYsidMpWTK6W2BeZ7FEfcYkg476zPFsLnS"); pub mod arbitrary_cpi_insecure { use super::*; + // Insecure CPI invocation without program ID verification pub fn cpi(ctx: Context, amount: u64) -> ProgramResult { + // Directly invokes the `transfer` instruction from the token program without checking the legitimacy of the CPI solana_program::program::invoke( &spl_token::instruction::transfer( - ctx.accounts.token_program.key, + ctx.accounts.token_program.key, // Unsafe: token program isn't verified ctx.accounts.source.key, ctx.accounts.destination.key, ctx.accounts.authority.key, @@ -28,6 +30,7 @@ pub mod arbitrary_cpi_insecure { #[derive(Accounts)] pub struct Cpi<'info> { + // Uses generic AccountInfo, lacking token-specific account validation source: AccountInfo<'info>, destination: AccountInfo<'info>, authority: AccountInfo<'info>, diff --git a/programs/5-arbitrary-cpi/recommended/src/lib.rs b/programs/5-arbitrary-cpi/recommended/src/lib.rs index 28f4a39..1d21e10 100644 --- a/programs/5-arbitrary-cpi/recommended/src/lib.rs +++ b/programs/5-arbitrary-cpi/recommended/src/lib.rs @@ -7,20 +7,26 @@ declare_id!("Fg6PaFpoGXkYsidMpWTK6W2BeZ7FEfcYkg476zPFsLnS"); pub mod arbitrary_cpi_recommended { use super::*; + // Safe CPI invocation using verified token accounts pub fn cpi(ctx: Context, amount: u64) -> ProgramResult { + // Uses anchor_spl's `transfer` helper, which performs more comprehensive checks on the accounts token::transfer(ctx.accounts.transfer_ctx(), amount) } } #[derive(Accounts)] pub struct Cpi<'info> { + // Strong typing for TokenAccount ensures proper token program interactions source: Account<'info, TokenAccount>, destination: Account<'info, TokenAccount>, + // Verifies authority as a signer authority: Signer<'info>, + // Program type enforces the correct program being used token_program: Program<'info, Token>, } impl<'info> Cpi<'info> { + // Creates the context required for invoking the token transfer CPI pub fn transfer_ctx(&self) -> CpiContext<'_, '_, '_, 'info, token::Transfer<'info>> { let program = self.token_program.to_account_info(); let accounts = token::Transfer { @@ -28,6 +34,7 @@ impl<'info> Cpi<'info> { to: self.destination.to_account_info(), authority: self.authority.to_account_info(), }; + // Builds the CPI context for safe execution CpiContext::new(program, accounts) } } diff --git a/programs/5-arbitrary-cpi/secure/src/lib.rs b/programs/5-arbitrary-cpi/secure/src/lib.rs index 503605d..1cb3a3c 100644 --- a/programs/5-arbitrary-cpi/secure/src/lib.rs +++ b/programs/5-arbitrary-cpi/secure/src/lib.rs @@ -7,10 +7,13 @@ declare_id!("Fg6PaFpoGXkYsidMpWTK6W2BeZ7FEfcYkg476zPFsLnS"); pub mod arbitrary_cpi_secure { use super::*; + // Secure CPI invocation with program ID validation pub fn cpi_secure(ctx: Context, amount: u64) -> ProgramResult { + // Verifies the token program is indeed the spl_token program if &spl_token::ID != ctx.accounts.token_program.key { - return Err(ProgramError::IncorrectProgramId); + return Err(ProgramError::IncorrectProgramId); // Fail if not the correct program } + // Safely invoke the transfer instruction solana_program::program::invoke( &spl_token::instruction::transfer( ctx.accounts.token_program.key, @@ -31,6 +34,7 @@ pub mod arbitrary_cpi_secure { #[derive(Accounts)] pub struct Cpi<'info> { + // Same as the insecure version but with added program ID validation source: AccountInfo<'info>, destination: AccountInfo<'info>, authority: AccountInfo<'info>,