diff --git a/.github/label-actions.yml b/.github/label-actions.yml index 0226a174c..b700da4c5 100644 --- a/.github/label-actions.yml +++ b/.github/label-actions.yml @@ -15,4 +15,5 @@ solved: # Set a close reason close-reason: 'resolved' # Lock the discussion - lock: true \ No newline at end of file + lock: true + diff --git a/content/courses/program-security/account-data-matching.md b/content/courses/program-security/account-data-matching.md index f9963a123..e5d32ed50 100644 --- a/content/courses/program-security/account-data-matching.md +++ b/content/courses/program-security/account-data-matching.md @@ -2,49 +2,59 @@ title: Account Data Matching objectives: - Explain the security risks associated with missing data validation checks - - Implement data validation checks using long-form Rust + - Implement data validation checks using native Rust - Implement data validation checks using Anchor constraints description: - "How to check your program's data accounts in both Anchor and Native Rust." + Learn how to properly validate account data in Solana programs to prevent + security vulnerabilities. --- +# Account Data Matching + ## Summary -- Use **data validation checks** to verify that account data matches an expected - value. Without appropriate data validation checks, unexpected accounts may be - used in an instruction. -- To implement data validation checks in Rust, simply compare the data stored on - an account to an expected value. +- **Data validation checks** are crucial for verifying that account data matches + expected values. Without proper checks, malicious users could exploit your + program using unexpected accounts. +- Implement data validation in native Rust by comparing account data to expected + values: ```rust if ctx.accounts.user.key() != ctx.accounts.user_data.user { return Err(ProgramError::InvalidAccountData.into()); } ``` -- In Anchor, you can use `constraint` to checks whether the given expression - evaluates to true. Alternatively, you can use `has_one` to check that a target - account field stored on the account matches the key of an account in the - `Accounts` struct. +- Use [Anchor constraints](https://www.anchor-lang.com/docs/account-constraints) + to simplify the process: + - Use `constraint` to evaluate custom expressions + - Use `has_one` to check that a field on one account matches the key of + another account ## Lesson -Account data matching refers to data validation checks used to verify the data -stored on an account matches an expected value. Data validation checks provide a -way to include additional constraints to ensure the appropriate accounts are -passed into an instruction. +Account data matching involves implementing data validation checks to ensure +that the +data stored on an account matches expected values. This is essential for +preventing unauthorized access and maintaining the integrity of your program's +state. + +### Why Account Data Matching Matters -This can be useful when accounts required by an instruction have dependencies on -values stored in other accounts or if an instruction is dependent on the data -stored in an account. +Without proper data validation, your program becomes vulnerable to various +attacks: -#### Missing data validation check +1. Unauthorized access: malicious users could pass in unexpected accounts, + gaining access to functionality they shouldn't have +2. State manipulation: attackers might alter the program's state in unintended + ways, compromising its integrity +3. Fund theft: in programs dealing with tokens, inadequate checks could + lead to unauthorized withdrawals -The example below includes an `update_admin` instruction that updates the -`admin` field stored on an `admin_config` account. +Let's look at an example to illustrate the importance of account data matching. -The instruction is missing a data validation check to verify the `admin` account -signing the transaction matches the `admin` stored on the `admin_config` -account. This means any account signing the transaction and passed into the -instruction as the `admin` account can update the `admin_config` account. +### Example: Insecure Admin Update + +Consider a program with an `update_admin` instruction handler that changes the +admin of a configuration account: ```rust use anchor_lang::prelude::*; @@ -52,9 +62,9 @@ use anchor_lang::prelude::*; declare_id!("Fg6PaFpoGXkYsidMpWTK6W2BeZ7FEfcYkg476zPFsLnS"); #[program] -pub mod data_validation { +pub mod insecure_admin { use super::*; - ... + pub fn update_admin(ctx: Context) -> Result<()> { ctx.accounts.admin_config.admin = ctx.accounts.new_admin.key(); Ok(()) @@ -65,9 +75,9 @@ pub mod data_validation { pub struct UpdateAdmin<'info> { #[account(mut)] pub admin_config: Account<'info, AdminConfig>, - #[account(mut)] - pub admin: Signer<'info>, - pub new_admin: SystemAccount<'info>, + pub current_admin: Signer<'info>, + /// CHECK: This account is not read or written in this instruction + pub new_admin: UncheckedAccount<'info>, } #[account] @@ -76,214 +86,134 @@ pub struct AdminConfig { } ``` -#### Add data validation check +This instruction is insecure because it doesn't verify that the `current_admin` +signer matches the `admin` stored in the `admin_config` account. Any signer +could potentially update the admin! -The basic Rust approach to solve this problem is to simply compare the passed in -`admin` key to the `admin` key stored in the `admin_config` account, throwing an -error if they don’t match. +### Implementing Data Validation Checks -```rust -if ctx.accounts.admin.key() != ctx.accounts.admin_config.admin { - return Err(ProgramError::InvalidAccountData.into()); -} -``` +Let's secure this instruction using two methods: native Rust and Anchor +constraints. -By adding a data validation check, the `update_admin` instruction would only -process if the `admin` signer of the transaction matched the `admin` stored on -the `admin_config` account. +#### Native Rust Approach -```rust -use anchor_lang::prelude::*; - -declare_id!("Fg6PaFpoGXkYsidMpWTK6W2BeZ7FEfcYkg476zPFsLnS"); +In native Rust, we can add a simple comparison: -#[program] -pub mod data_validation { - use super::*; - ... - pub fn update_admin(ctx: Context) -> Result<()> { - if ctx.accounts.admin.key() != ctx.accounts.admin_config.admin { - return Err(ProgramError::InvalidAccountData.into()); - } - ctx.accounts.admin_config.admin = ctx.accounts.new_admin.key(); - Ok(()) +```rust +pub fn update_admin(ctx: Context) -> Result<()> { + if ctx.accounts.current_admin.key() != ctx.accounts.admin_config.admin { + return Err(ProgramError::InvalidAccountData.into()); } -} - -#[derive(Accounts)] -pub struct UpdateAdmin<'info> { - #[account(mut)] - pub admin_config: Account<'info, AdminConfig>, - #[account(mut)] - pub admin: Signer<'info>, - pub new_admin: SystemAccount<'info>, -} - -#[account] -pub struct AdminConfig { - admin: Pubkey, + ctx.accounts.admin_config.admin = ctx.accounts.new_admin.key(); + Ok(()) } ``` -#### Use Anchor constraints +This check ensures that only the current admin can update the admin_config +account. -Anchor simplifies this with the `has_one` constraint. You can use the `has_one` -constraint to move the data validation check from the instruction logic to the -`UpdateAdmin` struct. +#### Anchor Constraints -In the example below, `has_one = admin` specifies that the `admin` account -signing the transaction must match the `admin` field stored on the -`admin_config` account. To use the `has_one` constraint, the naming convention -of the data field on the account must be consistent with the naming on the -account validation struct. +Anchor provides a more declarative way to implement these checks using +[account constraints](https://www.anchor-lang.com/docs/account-constraints): ```rust -use anchor_lang::prelude::*; - -declare_id!("Fg6PaFpoGXkYsidMpWTK6W2BeZ7FEfcYkg476zPFsLnS"); - -#[program] -pub mod data_validation { - use super::*; - ... - pub fn update_admin(ctx: Context) -> Result<()> { - ctx.accounts.admin_config.admin = ctx.accounts.new_admin.key(); - Ok(()) - } -} - #[derive(Accounts)] pub struct UpdateAdmin<'info> { #[account( mut, - has_one = admin + has_one = current_admin @ MyError::InvalidAdmin )] pub admin_config: Account<'info, AdminConfig>, - #[account(mut)] - pub admin: Signer<'info>, - pub new_admin: SystemAccount<'info>, -} - -#[account] -pub struct AdminConfig { - admin: Pubkey, + pub current_admin: Signer<'info>, + /// CHECK: This account is not read or written in this instruction + pub new_admin: UncheckedAccount<'info>, } ``` -Alternatively, you can use `constraint` to manually add an expression that must -evaluate to true in order for execution to continue. This is useful when for -some reason naming can’t be consistent or when you need a more complex -expression to fully validate the incoming data. +The `has_one = current_admin` constraint checks that the `current_admin` +account's key matches the `admin` field in the `AdminConfig` account. If it +doesn't match, Anchor will return the custom `InvalidAdmin` error. + +Alternatively, you can use the `constraint` attribute for more complex checks: ```rust #[derive(Accounts)] pub struct UpdateAdmin<'info> { #[account( mut, - constraint = admin_config.admin == admin.key() + constraint = admin_config.admin == current_admin.key() @ MyError::InvalidAdmin )] pub admin_config: Account<'info, AdminConfig>, - #[account(mut)] - pub admin: Signer<'info>, - pub new_admin: SystemAccount<'info>, + pub current_admin: Signer<'info>, + /// CHECK: This account is not read or written in this instruction + pub new_admin: UncheckedAccount<'info>, } ``` -## Lab +This approach allows for more flexibility in your validation logic. -For this lab we’ll create a simple “vault” program similar to the program we -used in the Signer Authorization lesson and the Owner Check lesson. Similar to -those labs, we’ll show in this lab how a missing data validation check could -allow the vault to be drained. + + Remember to define your custom errors in your program's error enum: -#### 1. Starter +```rust +#[error_code] +pub enum MyError { + #[msg("Only the current admin can perform this action")] + InvalidAdmin, +} +``` -To get started, download the starter code from the `starter` branch of -[this repository](https://github.com/Unboxed-Software/solana-account-data-matching). -The starter code includes a program with two instructions and the boilerplate -setup for the test file. + -The `initialize_vault` instruction initializes a new `Vault` account and a new -`TokenAccount`. The `Vault` account will store the address of a token account, -the authority of the vault, and a withdraw destination token account. +By implementing these checks, you ensure that only the rightful admin can update +the 'admin_config' account, significantly improving your program's security. -The authority of the new token account will be set as the `vault`, a PDA of the -program. This allows the `vault` account to sign for the transfer of tokens from -the token account. +## Lab: Securing a Vault Program -The `insecure_withdraw` instruction transfers all the tokens in the `vault` -account’s token account to a `withdraw_destination` token account. +In this lab, we'll create a simple "vault" program to demonstrate the importance +of account data matching. We'll implement both an insecure and a secure version +of a withdraw instruction to highlight the difference. -Notice that this instruction \***\*does\*\*** have a signer check for -`authority` and an owner check for `vault`. However, nowhere in the account -validation or instruction logic is there code that checks that the `authority` -account passed into the instruction matches the `authority` account on the -`vault`. +### 1. Setup -```rust -use anchor_lang::prelude::*; -use anchor_spl::token::{self, Mint, Token, TokenAccount}; +Clone the starter code from the `starter` branch of +[this repository](https://github.com/solana-developers/solana-account-data-matching). -declare_id!("Fg6PaFpoGXkYsidMpWTK6W2BeZ7FEfcYkg476zPFsLnS"); +The starter code includes a program with two instructions and a boilerplate test +file: -#[program] -pub mod account_data_matching { - use super::*; +1. `initialize_vault`: Creates a new `Vault` account and a `tokenAccount`. +2. `insecure_withdraw`: Transfers tokens from the vault's token account to a + destination account. - pub fn initialize_vault(ctx: Context) -> Result<()> { - ctx.accounts.vault.token_account = ctx.accounts.token_account.key(); - ctx.accounts.vault.authority = ctx.accounts.authority.key(); - ctx.accounts.vault.withdraw_destination = ctx.accounts.withdraw_destination.key(); - Ok(()) - } - - pub fn insecure_withdraw(ctx: Context) -> Result<()> { - let amount = ctx.accounts.token_account.amount; - - let seeds = &[b"vault".as_ref(), &[*ctx.bumps.get("vault").unwrap()]]; - let signer = [&seeds[..]]; + + The `insecure_withdraw` instruction has a critical flaw. Can you spot it before we implement the fix? + - let cpi_ctx = CpiContext::new_with_signer( - ctx.accounts.token_program.to_account_info(), - token::Transfer { - from: ctx.accounts.token_account.to_account_info(), - authority: ctx.accounts.vault.to_account_info(), - to: ctx.accounts.withdraw_destination.to_account_info(), - }, - &signer, - ); +### 2. Understand the Insecure Withdraw Instruction - token::transfer(cpi_ctx, amount)?; - Ok(()) - } -} +Let's examine the `insecure_withdraw` instruction: -#[derive(Accounts)] -pub struct InitializeVault<'info> { - #[account( - init, - payer = authority, - space = 8 + 32 + 32 + 32, - seeds = [b"vault"], - bump, - )] - pub vault: Account<'info, Vault>, - #[account( - init, - payer = authority, - token::mint = mint, - token::authority = vault, - seeds = [b"token"], - bump, - )] - pub token_account: Account<'info, TokenAccount>, - pub withdraw_destination: Account<'info, TokenAccount>, - pub mint: Account<'info, Mint>, - #[account(mut)] - pub authority: Signer<'info>, - pub token_program: Program<'info, Token>, - pub system_program: Program<'info, System>, - pub rent: Sysvar<'info, Rent>, +```rust +pub fn insecure_withdraw(ctx: Context) -> Result<()> { + let amount = ctx.accounts.token_account.amount; + + let seeds = &[b"vault".as_ref(), &[ctx.bumps.vault]]; + let signers = [&seeds[..]]; + + let cpi_ctx = CpiContext::new_with_signer( + ctx.accounts.token_program.to_account_info(), + token::Transfer { + from: ctx.accounts.token_account.to_account_info(), + authority: ctx.accounts.vault.to_account_info(), + to: ctx.accounts.withdraw_destination.to_account_info(), + }, + &signers, + ); + + token::transfer(cpi_ctx, amount)?; + Ok(()) } #[derive(Accounts)] @@ -304,103 +234,71 @@ pub struct InsecureWithdraw<'info> { pub token_program: Program<'info, Token>, pub authority: Signer<'info>, } - -#[account] -pub struct Vault { - token_account: Pubkey, - authority: Pubkey, - withdraw_destination: Pubkey, -} ``` -#### 2. Test `insecure_withdraw` instruction - -To prove that this is a problem, let’s write a test where an account other than -the vault’s `authority` tries to withdraw from the vault. - -The test file includes the code to invoke the `initialize_vault` instruction -using the provider wallet as the `authority` and then mints 100 tokens to the -`vault` token account. +The critical flaw here is that while the instruction requires an `authority` +signer, it doesn't verify whether the `authority` account passed to the +instruction matches the `authority` stored in the `Vault` account. This means +that any signer could potentially withdraw funds from the vault, even if they're +not the authorized user. -Add a test to invoke the `insecure_withdraw` instruction. Use -`withdrawDestinationFake` as the `withdrawDestination` account and `walletFake` -as the `authority`. Then send the transaction using `walletFake`. +### 3. Test the Insecure Withdraw Instruction -Since there are no checks the verify the `authority` account passed into the -instruction matches the values stored on the `vault` account initialized in the -first test, the instruction will process successfully and the tokens will be -transferred to the `withdrawDestinationFake` account. +Let's write a test to demonstrate this vulnerability. Add the following test to +your `tests/account-data-matching.ts` file: ```typescript -describe("account-data-matching", () => { - ... - it("Insecure withdraw", async () => { - const tx = await program.methods - .insecureWithdraw() - .accounts({ - vault: vaultPDA, - tokenAccount: tokenPDA, - withdrawDestination: withdrawDestinationFake, - authority: walletFake.publicKey, - }) - .transaction() - - await anchor.web3.sendAndConfirmTransaction(connection, tx, [walletFake]) - - const balance = await connection.getTokenAccountBalance(tokenPDA) - expect(balance.value.uiAmount).to.eq(0) - }) -}) +test("Insecure withdraw allows unauthorized access", async () => { + // Setup: Initialize vault and mint some tokens to it + // ... (initialization code here) + + // Attempt unauthorized withdrawal + const tx = await program.methods + .insecureWithdraw() + .accounts({ + vault: vaultPDA, + tokenAccount: tokenPDA, + withdrawDestination: unauthorizedWalletWithdrawDestination, + authority: unauthorizedWallet.publicKey, + }) + .transaction(); + + await anchor.web3.sendAndConfirmTransaction(connection, tx, [ + unauthorizedWallet, + ]); + + // Check that the withdrawal succeeded (it shouldn't have!) + const balance = await connection.getTokenAccountBalance(tokenPDA); + expect(balance.value.uiAmount).to.eq(0); +}); ``` -Run `anchor test` to see that both transactions will complete successfully. +Run `anchor test` to see that this unauthorized withdrawal succeeds, +demonstrating the security flaw. -```bash -account-data-matching - ✔ Initialize Vault (811ms) - ✔ Insecure withdraw (403ms) -``` +### 4. Implement a Secure Withdraw Instruction -#### 3. Add `secure_withdraw` instruction - -Let’s go implement a secure version of this instruction called -`secure_withdraw`. - -This instruction will be identical to the `insecure_withdraw` instruction, -except we’ll use the `has_one` constraint in the account validation struct -(`SecureWithdraw`) to check that the `authority` account passed into the -instruction matches the `authority` account on the `vault` account. That way -only the correct authority account can withdraw the vault’s tokens. +Now, let's implement a secure version of the withdraw instruction: ```rust -use anchor_lang::prelude::*; -use anchor_spl::token::{self, Mint, Token, TokenAccount}; - -declare_id!("Fg6PaFpoGXkYsidMpWTK6W2BeZ7FEfcYkg476zPFsLnS"); - -#[program] -pub mod account_data_matching { - use super::*; - ... - pub fn secure_withdraw(ctx: Context) -> Result<()> { - let amount = ctx.accounts.token_account.amount; - - let seeds = &[b"vault".as_ref(), &[*ctx.bumps.get("vault").unwrap()]]; - let signer = [&seeds[..]]; - - let cpi_ctx = CpiContext::new_with_signer( - ctx.accounts.token_program.to_account_info(), - token::Transfer { - from: ctx.accounts.token_account.to_account_info(), - authority: ctx.accounts.vault.to_account_info(), - to: ctx.accounts.withdraw_destination.to_account_info(), - }, - &signer, - ); - - token::transfer(cpi_ctx, amount)?; - Ok(()) - } +pub fn secure_withdraw(ctx: Context) -> Result<()> { + let amount = ctx.accounts.token_account.amount; + + let seeds = &[b"vault".as_ref(), &[ctx.bumps.vault]]; + let signers = [&seeds[..]]; + + let cpi_ctx = CpiContext::new_with_signer( + ctx.accounts.token_program.to_account_info(), + token::Transfer { + from: ctx.accounts.token_account.to_account_info(), + authority: ctx.accounts.vault.to_account_info(), + to: ctx.accounts.withdraw_destination.to_account_info(), + }, + &signers, + ); + + token::transfer(cpi_ctx, amount)?; + Ok(()) } #[derive(Accounts)] @@ -408,10 +306,9 @@ pub struct SecureWithdraw<'info> { #[account( seeds = [b"vault"], bump, - has_one = token_account, has_one = authority, + has_one = token_account, has_one = withdraw_destination, - )] pub vault: Account<'info, Vault>, #[account( @@ -427,60 +324,67 @@ pub struct SecureWithdraw<'info> { } ``` -#### 4. Test `secure_withdraw` instruction +The key difference here is the use of a +[`has_one` constraint](https://www.anchor-lang.com/docs/account-constraints) in +the `SecureWithdraw` struct. These ensure that the `authority`, `token_account`, +and `withdraw_destination` passed to the instruction match those stored in the +`Vault` account. + +### 5. Test the Secure Withdraw Instruction -Now let’s test the `secure_withdraw` instruction with two tests: one that uses -`walletFake` as the authority and one that uses `wallet` as the authority. We -expect the first invocation to return an error and the second to succeed. +Now, let's test our secure instruction: ```typescript -describe("account-data-matching", () => { - ... - it("Secure withdraw, expect error", async () => { - try { - const tx = await program.methods - .secureWithdraw() - .accounts({ - vault: vaultPDA, - tokenAccount: tokenPDA, - withdrawDestination: withdrawDestinationFake, - authority: walletFake.publicKey, - }) - .transaction() - - await anchor.web3.sendAndConfirmTransaction(connection, tx, [walletFake]) - } catch (err) { - expect(err) - console.log(err) - } - }) - - it("Secure withdraw", async () => { - await spl.mintTo( - connection, - wallet.payer, - mint, - tokenPDA, - wallet.payer, - 100 - ) +test("Secure withdraw prevents unauthorized access", async () => { + // Setup: Initialize vault and mint some tokens to it + // ... (initialization code here) + // Attempt unauthorized withdrawal + // Note: unauthorizedWallet represents an unauthorized wallet trying to withdraw funds + try { await program.methods .secureWithdraw() .accounts({ vault: vaultPDA, tokenAccount: tokenPDA, - withdrawDestination: withdrawDestination, - authority: wallet.publicKey, + withdrawDestination: unauthorizedWalletWithdrawDestination, + authority: unauthorizedWallet.publicKey, }) - .rpc() + .rpc(); + + // If we reach here, the test failed + assert.fail("Expected an error, but withdrawal succeeded"); + } catch (error) { + // Check that it's the error we expect + expect(error.error.errorCode.code).to.equal("ConstraintHasOne"); + } - const balance = await connection.getTokenAccountBalance(tokenPDA) - expect(balance.value.uiAmount).to.eq(0) - }) -}) + // Check that the balance hasn't changed + const balance = await connection.getTokenAccountBalance(tokenPDA); + expect(balance.value.uiAmount).to.eq(100); // Assuming 100 tokens were initially minted +}); + +test("Secure withdraw allows authorized access", async () => { + // Perform authorized withdrawal + await program.methods + .secureWithdraw() + .accounts({ + vault: vaultPDA, + tokenAccount: tokenPDA, + withdrawDestination: withdrawDestination, + authority: wallet.publicKey, + }) + .rpc(); + + // Check that the withdrawal succeeded + const balance = await connection.getTokenAccountBalance(tokenPDA); + expect(balance.value.uiAmount).to.eq(0); +}); ``` +Run `anchor test` again. You should see that the unauthorized withdrawal now +fails, while the authorized one succeeds. + Run `anchor test` to see that the transaction using an incorrect authority account will now return an Anchor Error while the transaction using correct accounts completes successfully. @@ -514,20 +418,31 @@ problems before you deploy. If you want to take a look at the final solution code you can find it on the `solution` branch of -[the repository](https://github.com/Unboxed-Software/solana-account-data-matching/tree/solution). +[the repository](https://github.com/solana-developers/account-data-matching/tree/solution) -## Challenge +### Conclusion -Just as with other lessons in this unit, your opportunity to practice avoiding -this security exploit lies in auditing your own or other programs. +By implementing proper account data matching, we've significantly improved the +security of our vault program. The secure version ensures that only the +authorized user can withdraw funds, preventing potential exploits. -Take some time to review at least one program and ensure that proper data checks -are in place to avoid security exploits. +Remember, as your programs grow in complexity, it becomes increasingly important +to implement thorough data validation checks. Always consider what assumptions +your program is making about its inputs. Validate these assumptions explicitly. -Remember, if you find a bug or exploit in somebody else's program, please alert -them! If you find one in your own program, be sure to patch it right away. +Now that you've seen how to implement account data matching, take some time to +review one of your existing programs. Look for places where you might be making +assumptions about account data without explicitly checking it. Implement +appropriate checks using the techniques you've learned in this lesson. + +Remember, if you find a security vulnerability in someone else's program, +[responsibly disclose](https://en.wikipedia.org/wiki/Coordinated_vulnerability_disclosure) +it to the program's maintainers. If you find one in your own program, patch it +as soon as possible! + +After completing this lab and challenge, you'll have gained practical experience +in identifying and fixing security vulnerabilities related to account data +matching. This skill is crucial for developing secure Solana programs. - Push your code to GitHub and [tell us what you thought of this lesson](https://form.typeform.com/to/IPH0UGz7#answers-lesson=a107787e-ad33-42bb-96b3-0592efc1b92f)! - diff --git a/content/courses/program-security/arbitrary-cpi.md b/content/courses/program-security/arbitrary-cpi.md index 737cf8cb6..0cb5f4f87 100644 --- a/content/courses/program-security/arbitrary-cpi.md +++ b/content/courses/program-security/arbitrary-cpi.md @@ -3,7 +3,7 @@ title: Arbitrary CPI objectives: - Explain the security risks associated with invoking a CPI to an unknown program - - Showcase how Anchor’s CPI module prevents this from happening when making a + - Showcase how Anchor's CPI module prevents this from happening when making a CPI from one Anchor program to another - Safely and securely make a CPI from an Anchor program to an arbitrary non-anchor program @@ -13,9 +13,9 @@ description: "How to safely invoke Solana programs from other Solana programs." ## Summary - To generate a CPI, the target program must be passed into the invoking - instruction as an account. This means that any target program could be passed - into the instruction. Your program should check for incorrect or unexpected - programs. + instruction handler as an account. This means that any target program could be + passed into the instruction handler. Your program should check for incorrect + or unexpected programs. - Perform program checks in native programs by simply comparing the public key of the passed-in program to the progam you expected. - If a program is written in Anchor, then it may have a publicly available CPI @@ -25,27 +25,27 @@ description: "How to safely invoke Solana programs from other Solana programs." ## Lesson -A cross program invocation (CPI) is when one program invokes an instruction on -another program. An “arbitrary CPI” is when a program is structured to issue a -CPI to whatever program is passed into the instruction rather than expecting to -perform a CPI to one specific program. Given that the callers of your program's -instruction can pass any program they'd like into the instruction's list of -accounts, failing to verify the address of a passed-in program results in your -program performing CPIs to arbitrary programs. +A cross program invocation (CPI) is when one program invokes an instruction +handler on another program. An “arbitrary CPI” is when a program is structured +to issue a CPI to whatever program is passed into the instruction handler rather +than expecting to perform a CPI to one specific program. Given that the callers +of your program's instruction handler can pass any program they'd like into the +instruction's list of accounts, failing to verify the address of a passed-in +program results in your program performing CPIs to arbitrary programs. This lack of program checks creates an opportunity for a malicious user to pass in a different program than expected, causing the original program to call an -instruction on this mystery program. There’s no telling what the consequences of -this CPI could be. It depends on the program logic (both that of the original -program and the unexpected program), as well as what other accounts are passed -into the original instruction. +instruction handler on this mystery program. There’s no telling what the +consequences of this CPI could be. It depends on the program logic (both that of +the original program and the unexpected program), as well as what other accounts +are passed into the original instruction handler. -### Missing program checks +### Missing Program Checks -Take the following program as an example. The `cpi` instruction invokes the -`transfer` instruction on `token_program`, but there is no code that checks -whether or not the `token_program` account passed into the instruction is, in -fact, the SPL Token Program. +Take the following program as an example. The `cpi` instruction handler invokes +the `transfer` instruction handler on `token_program`, but there is no code that +checks whether or not the `token_program` account passed into the instruction +handler is, in fact, the SPL Token Program. ```rust use anchor_lang::prelude::*; @@ -85,14 +85,14 @@ pub struct Cpi<'info> { } ``` -An attacker could easily call this instruction and pass in a duplicate token -program that they created and control. +An attacker could easily call this instruction handler and pass in a duplicate +token program that they created and control. -### Add program checks +### Add Program Checks It's possible to fix this vulnerabilty by simply adding a few lines to the `cpi` -instruction to check whether or not `token_program`'s public key is that of the -SPL Token Program. +instruction handler to check whether or not `token_program`'s public key is that +of the SPL Token Program. ```rust pub fn cpi_secure(ctx: Context, amount: u64) -> ProgramResult { @@ -117,22 +117,24 @@ pub fn cpi_secure(ctx: Context, amount: u64) -> ProgramResult { } ``` -Now, if an attacker passes in a different token program, the instruction will -return the `ProgramError::IncorrectProgramId` error. +Now, if an attacker passes in a different token program, the instruction handler +will return the `ProgramError::IncorrectProgramId` error. -Depending on the program you’re invoking with your CPI, you can either hard code -the address of the expected program ID or use the program’s Rust crate to get +Depending on the program you're invoking with your CPI, you can either hard code +the address of the expected program ID or use the program's Rust crate to get the address of the program, if available. In the example above, the `spl_token` crate provides the address of the SPL Token Program. -### Use an Anchor CPI module +### Use an Anchor CPI Module -A simpler way to manage program checks is to use Anchor CPI modules. We learned -in a -[previous lesson](https://github.com/Unboxed-Software/solana-course/blob/main/content/anchor-cpi) +A simpler way to manage program checks is to use +[Anchor CPI](https://book.anchor-lang.com/anchor_in_depth/CPIs.html) module. We +learned in a +[previous lesson of Anchor CPI](/content/courses/onchain-development/anchor-cpi.md) that Anchor can automatically generate CPI modules to make CPIs into the program simpler. These modules also enhance security by verifying the public key of the -program that’s passed into one of its public instructions. +program that's passed into one of its public instructions using +[account constraints](https://www.anchor-lang.com/docs/account-constraints). Every Anchor program uses the `declare_id()` macro to define the address of the program. When a CPI module is generated for a specific program, it uses the @@ -182,12 +184,15 @@ impl<'info> Cpi<'info> { } ``` -Note that, like the example above, Anchor has created a few + + +Like the example above, Anchor has created a few [wrappers for popular native programs](https://github.com/coral-xyz/anchor/tree/master/spl/src) that allow you to issue CPIs into them as if they were Anchor programs. + -Additionally and depending on the program you’re making the CPI to, you may be -able to use Anchor’s +Additionally and depending on the program you're making the CPI to, you may be +able to use Anchor's [`Program` account type](https://docs.rs/anchor-lang/latest/anchor_lang/accounts/program/struct.Program.html) to validate the passed-in program in your account validation struct. Between the [`anchor_lang`](https://docs.rs/anchor-lang/latest/anchor_lang) and [`anchor_spl`](https://docs.rs/anchor_spl/latest/) crates, @@ -207,8 +212,8 @@ use other_program::program::OtherProgram; ## Lab -To show the importance of checking with program you use for CPIs, we're going to -work with a simplified and somewhat contrived game. This game represents +To show the importance of checking the program ID you use for CPIs, we're going +to work with a simplified and somewhat contrived game. This game represents characters with PDA accounts, and uses a separate "metadata" program to manage character metadata and attributes like health and power. @@ -218,10 +223,10 @@ mints, distribution, and transfers, and a separate metadata program is used to assign metadata to tokens. So the vulnerability we go through here could also be applied to real tokens. -#### 1. Setup +### 1. Setup -We'll start with the `starter` branch of -[this repository](https://github.com/Unboxed-Software/solana-arbitrary-cpi/tree/starter). +We'll start with the +[`starter` branch of this repository](https://github.com/solana-developers/arbitrary-cpi/tree/starter). Clone the repository and then open it on the `starter` branch. Notice that there are three programs: @@ -242,7 +247,7 @@ look at the program. It has two instructions: The second program, `character-metadata`, is meant to be the "approved" program for handling character metadata. Have a look at this program. It has a single -instruction for `create_metadata` that creates a new PDA and assigns a +instruction handler for `create_metadata` that creates a new PDA and assigns a pseudo-random value between 0 and 20 for the character's health and power. The last program, `fake-metadata` is a "fake" metadata program meant to @@ -250,59 +255,63 @@ illustrate what an attacker might make to exploit our `gameplay` program. This program is almost identical to the `character-metadata` program, only it assigns a character's initial health and power to be the max allowed: 255. -#### 2. Test `create_character_insecure` instruction +### 2. Test create_character_insecure Instruction Handler There is already a test in the `tests` directory for this. It's long, but take a minute to look at it before we talk through it together: ```typescript -it("Insecure instructions allow attacker to win every time", async () => { - // Initialize player one with real metadata program - await gameplayProgram.methods - .createCharacterInsecure() - .accounts({ - metadataProgram: metadataProgram.programId, - authority: playerOne.publicKey, - }) - .signers([playerOne]) - .rpc(); - - // Initialize attacker with fake metadata program - await gameplayProgram.methods - .createCharacterInsecure() - .accounts({ - metadataProgram: fakeMetadataProgram.programId, - authority: attacker.publicKey, - }) - .signers([attacker]) - .rpc(); - - // Fetch both player's metadata accounts - const [playerOneMetadataKey] = getMetadataKey( - playerOne.publicKey, - gameplayProgram.programId, - metadataProgram.programId, - ); - - const [attackerMetadataKey] = getMetadataKey( - attacker.publicKey, - gameplayProgram.programId, - fakeMetadataProgram.programId, - ); - - const playerOneMetadata = - await metadataProgram.account.metadata.fetch(playerOneMetadataKey); - - const attackerMetadata = - await fakeMetadataProgram.account.metadata.fetch(attackerMetadataKey); - - // The regular player should have health and power between 0 and 20 - expect(playerOneMetadata.health).to.be.lessThan(20); - expect(playerOneMetadata.power).to.be.lessThan(20); - - // The attacker will have health and power of 255 - expect(attackerMetadata.health).to.equal(255); - expect(attackerMetadata.power).to.equal(255); +it("Insecure instructions allow attacker to win every time successfully", async () => { + try { + // Initialize player one with real metadata program + await gameplayProgram.methods + .createCharacterInsecure() + .accounts({ + metadataProgram: metadataProgram.programId, + authority: playerOne.publicKey, + }) + .signers([playerOne]) + .rpc(); + + // Initialize attacker with fake metadata program + await gameplayProgram.methods + .createCharacterInsecure() + .accounts({ + metadataProgram: fakeMetadataProgram.programId, + authority: attacker.publicKey, + }) + .signers([attacker]) + .rpc(); + + // Fetch both player's metadata accounts + const [playerOneMetadataKey] = getMetadataKey( + playerOne.publicKey, + gameplayProgram.programId, + metadataProgram.programId, + ); + + const [attackerMetadataKey] = getMetadataKey( + attacker.publicKey, + gameplayProgram.programId, + fakeMetadataProgram.programId, + ); + + const playerOneMetadata = + await metadataProgram.account.metadata.fetch(playerOneMetadataKey); + + const attackerMetadata = + await fakeMetadataProgram.account.metadata.fetch(attackerMetadataKey); + // The regular player should have health and power between 0 and 20 + expect(playerOneMetadata.health).to.be.lessThan(20); + expect(playerOneMetadata.power).to.be.lessThan(20); + + // The attacker will have health and power of 255 + expect(attackerMetadata.health).to.equal(255); + expect(attackerMetadata.power).to.equal(255); + } catch (error) { + console.error("Test failed:", error); + throw error; + } }); ``` @@ -319,12 +328,12 @@ are each 255, making the attacker unbeatable. If you haven't already, run `anchor test` to see that this test in fact behaves as described. -#### 3. Create a `create_character_secure` instruction +### 3. Create a create_character_secure Instruction Handler -Let's fix this by creating a secure instruction for creating a new character. -This instruction should implement proper program checks and use the -`character-metadata` program's `cpi` crate to do the CPI rather than just using -`invoke`. +Let's fix this by creating a secure instruction handler for creating a new +character. This instruction handler should implement proper program checks and +use the `character-metadata` program's `cpi` crate to do the CPI rather than +just using `invoke`. If you want to test out your skills, try this on your own before moving ahead. @@ -352,7 +361,7 @@ pub struct CreateCharacterSecure<'info> { #[account( init, payer = authority, - space = 8 + 32 + 32 + 64, + space = DISCRIMINATOR_SIZE + Character::INIT_SPACE, seeds = [authority.key().as_ref()], bump )] @@ -363,25 +372,27 @@ pub struct CreateCharacterSecure<'info> { seeds::program = metadata_program.key(), bump, )] - /// CHECK: manual checks + /// CHECK: This account will not be checked by anchor pub metadata_account: AccountInfo<'info>, pub metadata_program: Program<'info, CharacterMetadata>, pub system_program: Program<'info, System>, } ``` -Lastly, we add the `create_character_secure` instruction. It will be the same as -before but will use the full functionality of Anchor CPIs rather than using -`invoke` directly: +Lastly, we add the `create_character_secure` instruction handler. It will be the +same as before but will use the full functionality of Anchor CPIs rather than +using `invoke` directly: ```rust pub fn create_character_secure(ctx: Context) -> Result<()> { + // Initialize character data let character = &mut ctx.accounts.character; character.metadata = ctx.accounts.metadata_account.key(); - character.auth = ctx.accounts.authority.key(); + character.authority = ctx.accounts.authority.key(); character.wins = 0; - let context = CpiContext::new( + // Prepare CPI context + let cpi_context = CpiContext::new( ctx.accounts.metadata_program.to_account_info(), CreateMetadata { character: ctx.accounts.character.to_account_info(), @@ -391,20 +402,21 @@ pub fn create_character_secure(ctx: Context) -> Result<() }, ); - create_metadata(context)?; + // Perform CPI to create metadata + create_metadata(cpi_context)?; Ok(()) } ``` -#### 4. Test `create_character_secure` +### 4. Test create_character_secure Instruction Handler Now that we have a secure way of initializing a new character, let's create a new test. This test just needs to attempt to initialize the attacker's character and expect an error to be thrown. ```typescript -it("Secure character creation doesn't allow fake program", async () => { +it("prevents secure character creation with fake program", async () => { try { await gameplayProgram.methods .createCharacterSecure() @@ -414,23 +426,25 @@ it("Secure character creation doesn't allow fake program", async () => { }) .signers([attacker]) .rpc(); + + throw new Error("Expected createCharacterSecure to throw an error"); } catch (error) { - expect(error); + expect(error).to.be.instanceOf(Error); console.log(error); } }); ``` Run `anchor test` if you haven't already. Notice that an error was thrown as -expected, detailing that the program ID passed into the instruction is not the -expected program ID: +expected, detailing that the program ID passed into the instruction handler is +not the expected program ID: ```bash 'Program log: AnchorError caused by account: metadata_program. Error Code: InvalidProgramId. Error Number: 3008. Error Message: Program ID was not as expected.', 'Program log: Left:', -'Program log: FKBWhshzcQa29cCyaXc1vfkZ5U985gD5YsqfCzJYUBr', +'Program log: HQqG7PxftCD5BB9WUWcYksrjDLUwCmbV8Smh1W8CEgQm', 'Program log: Right:', -'Program log: D4hPnYEsAx4u3EQMrKEXsY3MkfLndXbBKTEYTwwm25TE' +'Program log: 4FgVd2dgsFnXbSHz8fj9twNbfx8KWcBJkHa6APicU6KS' ``` That's all you need to do to protect against arbitrary CPIs! @@ -440,8 +454,7 @@ certainly won't stop you from architecting the program you need, but please take every precaution possible to ensure no vulnerabilities in your program. If you want to take a look at the final solution code you can find it on the -`solution` branch of -[the same repository](https://github.com/Unboxed-Software/solana-arbitrary-cpi/tree/solution). +[`solution` branch of the same repository](https://github.com/solana-developers/arbitrary-cpi/tree/solution). ## Challenge @@ -449,13 +462,14 @@ Just as with other lessons in this unit, your opportunity to practice avoiding this security exploit lies in auditing your own or other programs. Take some time to review at least one program and ensure that program checks are -in place for every program passed into the instructions, particularly those that -are invoked via CPI. +in place for every program passed into the instruction handlers, particularly +those that are invoked via CPI. Remember, if you find a bug or exploit in somebody else's program, please alert them! If you find one in your own program, be sure to patch it right away. + Push your code to GitHub and [tell us what you thought of this lesson](https://form.typeform.com/to/IPH0UGz7#answers-lesson=5bcaf062-c356-4b58-80a0-12cca99c29b0)! diff --git a/content/courses/program-security/bump-seed-canonicalization.md b/content/courses/program-security/bump-seed-canonicalization.md index e87e288fe..e24b0dccd 100644 --- a/content/courses/program-security/bump-seed-canonicalization.md +++ b/content/courses/program-security/bump-seed-canonicalization.md @@ -1,15 +1,19 @@ --- title: Bump Seed Canonicalization -objectives: - - Explain the vulnerabilities associated with using PDAs derived without the - canonical bump - - Initialize a PDA using Anchor’s `seeds` and `bump` constraints to - automatically use the canonical bump - - Use Anchor's `seeds` and `bump` constraints to ensure the canonical bump is - always used in future instructions when deriving a PDA description: - "Understand the need for consistent PDA calculation by storing and reusuing - the canonical bump." + Learn how to properly use canonical bump seeds when deriving and verifying + PDAs to ensure security and consistency in your Solana programs. +keywords: + - canonical bump + - PDA + - program derived address + - Anchor + - find_program_address + - create_program_address +tags: + - security + - pdas + - anchor --- ## Summary @@ -30,7 +34,7 @@ description: - Anchor allows you to **specify a bump** with the `bump = ` constraint when verifying the address of a PDA - Because `find_program_address` can be expensive, best practice is to store the - derived bump in an account’s data field to be referenced later on when + derived bump in an account's data field to be referenced later on when re-deriving the address for verification ```rust #[derive(Accounts)] @@ -71,6 +75,8 @@ in as instruction data to derive a PDA. The instruction then derives the PDA using `create_program_address` function and checks that the `address` matches the public key of the `data` account. +Here's an example of an insecure implementation: + ```rust use anchor_lang::prelude::*; @@ -119,12 +125,11 @@ different PDA. A simple way around this problem is to have the program expect only the canonical bump and use `find_program_address` to derive the PDA. -The -[`find_program_address`](https://docs.rs/solana-program/latest/solana_program/pubkey/struct.Pubkey.html#method.find_program_address) -_always uses the canonical bump_. This function iterates through calling -`create_program_address`, starting with a bump of 255 and decrementing the bump -by one with each iteration. As soon as a valid address is found, the function -returns both the derived PDA and the canonical bump used to derive it. +The `find_program_address` function always uses the canonical bump. It iterates +through calling `create_program_address`, starting with a bump of 255 and +decrementing the bump by one with each iteration. As soon as a valid address is +found, the function returns both the derived PDA and the canonical bump used to +derive it. This ensures a one-to-one mapping between your input seeds and the address they produce. @@ -151,12 +156,12 @@ pub fn set_value_secure( } ``` -### Use Anchor’s `seeds` and `bump` constraints +### Use Anchor's `seeds` and `bump` constraints Anchor provides a convenient way to derive PDAs in the account validation struct using the `seeds` and `bump` constraints. These can even be combined with the `init` constraint to initialize the account at the intended address. To protect -the program from the vulnerability we’ve been discussing throughout this lesson, +the program from the vulnerability we've been discussing throughout this lesson, Anchor does not even allow you to initialize an account at a PDA using anything but the canonical bump. Instead, it uses `find_program_address` to derive the PDA and subsequently performs the initialization. @@ -280,7 +285,7 @@ If you don't specify the bump on the `bump` constraint, Anchor will still use `find_program_address` to derive the PDA using the canonical bump. As a consequence, your instruction will incur a variable amount of compute budget. Programs that are already at risk of exceeding their compute budget should use -this with care since there is a chance that the program’s budget may be +this with care since there is a chance that the program's budget may be occasionally and unpredictably exceeded. On the other hand, if you only need to verify the address of a PDA passed in @@ -297,7 +302,7 @@ rewards on time. #### 1. Setup Start by getting the code on the `starter` branch of -[this repository](https://github.com/Unboxed-Software/solana-bump-seed-canonicalization/tree/starter). +[this repository](https://github.com/solana-developers/solana-bump-seed-canonicalization/tree/starter). Notice that there are two instructions on the program and a single test in the `tests` directory. @@ -595,7 +600,7 @@ careful to design your program to explicitly use the canonical bump! If you want to take a look at the final solution code you can find it on the `solution` branch of -[the same repository](https://github.com/Unboxed-Software/solana-bump-seed-canonicalization/tree/solution). +[the same repository](https://github.com/solana-developers/solana-bump-seed-canonicalization/tree/solution). ## Challenge @@ -609,6 +614,5 @@ Remember, if you find a bug or exploit in somebody else's program, please alert them! If you find one in your own program, be sure to patch it right away. -Push your code to GitHub and -[tell us what you thought of this lesson](https://form.typeform.com/to/IPH0UGz7#answers-lesson=d3f6ca7a-11c8-421f-b7a3-d6c08ef1aa8b)! +Push your code to GitHub and [tell us what you thought of this lesson](https://form.typeform.com/to/IPH0UGz7#answers-lesson=d3f6ca7a-11c8-421f-b7a3-d6c08ef1aa8b)! diff --git a/content/guides/getstarted/local-rust-hello-world.md b/content/guides/getstarted/local-rust-hello-world.md index a905ddc63..eaba5cf95 100644 --- a/content/guides/getstarted/local-rust-hello-world.md +++ b/content/guides/getstarted/local-rust-hello-world.md @@ -229,8 +229,7 @@ library. ### Install Node.js To use node in WSL2 on Windows, please follow this -[guide to installing node in WSL2](https://learn.microsoft.com/en-us/windows/dev-environment/javascript/nodejs-on-wsl) -to install node. +[guide to installing node in WSL2](https://learn.microsoft.com/en-us/windows/dev-environment/javascript/nodejs-on-wsl). ```shell sudo apt-get install curl diff --git a/docs/programs/examples.md b/docs/programs/examples.md index bd70d765d..afcb0b126 100644 --- a/docs/programs/examples.md +++ b/docs/programs/examples.md @@ -38,7 +38,8 @@ examples are self-contained and are available in native Rust (ie, with no framework), [Anchor](https://www.anchor-lang.com/docs/installation), [Seahorse](https://seahorse-lang.org/) and it also contains a list of examples that we would love to -[see as contributions](https://github.com/solana-developers/program-examples?tab=readme-ov-file#examples-wed-love-to-see). +[see as contributions](https://github.com/solana-developers/program-examples?tab=readme-ov-file#examples-wed-love-to-see). + Within the repo you will find the following subfolder, each with assorted example programs within them: