diff --git a/content/courses/program-security/signer-auth.md b/content/courses/program-security/signer-auth.md index ab0d6a7be..897291264 100644 --- a/content/courses/program-security/signer-auth.md +++ b/content/courses/program-security/signer-auth.md @@ -1,61 +1,61 @@ --- title: Signer Authorization objectives: - - Explain the security risks associated with not performing appropriate signer - checks - - Implement signer checks using long-form Rust + - Explain the security risks of not performing appropriate signer checks. + - Implement signer checks using native Rust - Implement signer checks using Anchor’s `Signer` type - Implement signer checks using Anchor’s `#[account(signer)]` constraint description: - "Ensure instructions are only ran by authorized accounts by implmementing - Signer checks." + "Ensure instructions are only executed by authorized accounts by implementing + signer checks." --- ## Summary -- Use **Signer Checks** to verify that specific accounts have signed a - transaction. Without appropriate signer checks, accounts may be able to - execute instructions they shouldn’t be authorized to perform. -- To implement a signer check in Rust, simply check that an account’s - `is_signer` property is `true` +- **Signer Checks** are essential to verify that specific accounts have signed a + transaction. Without proper signer checks, unauthorized accounts may execute + instructions they shouldn't be allowed to perform. +- In Anchor, you can use the `Signer` account type in your account validation + struct to automatically perform a signer check on a given account. +- Anchor also provides the + [`#[account(signer)]`](https://www.anchor-lang.com/docs/account-constraints) + constraint, which automatically verifies that a specified account has signed + the transaction. +- In native Rust, implement a signer check by verifying that an account's + `is_signer` property is `true`: + ```rust if !ctx.accounts.authority.is_signer { - return Err(ProgramError::MissingRequiredSignature.into()); + return Err(ProgramError::MissingRequiredSignature.into()); } ``` -- In Anchor, you can use the **`Signer`** account type in your account - validation struct to have Anchor automatically perform a signer check on a - given account -- Anchor also has an account constraint that will automatically verify that a - given account has signed a transaction ## Lesson -Signer checks are used to verify that a given account’s owner has authorized a -transaction. Without a signer check, operations whose execution should be -limited to only specific accounts can potentially be performed by any account. -In the worst case scenario, this could result in wallets being completely -drained by attackers passing in whatever account they want to an instruction. - -#### Missing Signer Check +**Signer checks** ensure that only authorized accounts can execute specific +instructions. Without these checks, any account might perform operations that +should be restricted, potentially leading to severe security vulnerabilities, +such as unauthorized access and control over program accounts. -The example below shows an oversimplified version of an instruction that updates -the `authority` field stored on a program account. +### Missing Signer Check -Notice that the `authority` field on the `UpdateAuthority` account validation -struct is of type `AccountInfo`. In Anchor, the `AccountInfo` account type -indicates that no checks are performed on the account prior to instruction -execution. +Below is an oversimplified instruction handler that updates the `authority` +field on a program account. Notice that the `authority` field in the +`UpdateAuthority` account validation struct is of type `UncheckedAccount`. In +Anchor, the +[`UncheckedAccount`](https://docs.rs/anchor-lang/latest/anchor_lang/accounts/unchecked_account/struct.UncheckedAccount.html) +type indicates that no checks are performed on the account before executing the +instruction handler. -Although the `has_one` constraint is used to validate the `authority` account -passed into the instruction matches the `authority` field stored on the `vault` -account, there is no check to verify the `authority` account authorized the +Although the `has_one` constraint ensures that the `authority` account passed to +the instruction handler matches the `authority` field on the `vault` account, +there is no verification that the `authority` account actually authorized the transaction. -This means an attacker can simply pass in the public key of the `authority` -account and their own public key as the `new_authority` account to reassign -themselves as the new authority of the `vault` account. At that point, they can -interact with the program as the new authority. +This omission allows an attacker to pass in the `authority` account’s public key +and their own public key as the `new_authority` account, effectively reassigning +themselves as the new authority of the `vault` account. Once they have control, +they can interact with the program as the new authority. ```rust use anchor_lang::prelude::*; @@ -79,8 +79,10 @@ pub struct UpdateAuthority<'info> { has_one = authority )] pub vault: Account<'info, Vault>, - pub new_authority: AccountInfo<'info>, - pub authority: AccountInfo<'info>, + /// CHECK: This account will not be checked by Anchor + pub new_authority: UncheckedAccount<'info>, + /// CHECK: This account will not be checked by Anchor + pub authority: UncheckedAccount<'info>, } #[account] @@ -90,23 +92,20 @@ pub struct Vault { } ``` -#### Add signer authorization checks +### Adding Signer Authorization Checks -All you need to do to validate that the `authority` account signed is to add a -signer check within the instruction. That simply means checking that -`authority.is_signer` is `true`, and returning a `MissingRequiredSignature` -error if `false`. +To validate that the `authority` account signed the transaction, add a signer +check within the instruction handler: -```typescript +```rust if !ctx.accounts.authority.is_signer { return Err(ProgramError::MissingRequiredSignature.into()); } ``` -By adding a signer check, the instruction would only process if the account -passed in as the `authority` account also signed the transaction. If the -transaction was not signed by the account passed in as the `authority` account, -then the transaction would fail. +By adding this check, the instruction handler will only proceed if the +`authority` account has signed the transaction. If the account is not signed, +the transaction will fail. ```rust use anchor_lang::prelude::*; @@ -134,8 +133,10 @@ pub struct UpdateAuthority<'info> { has_one = authority )] pub vault: Account<'info, Vault>, - pub new_authority: AccountInfo<'info>, - pub authority: AccountInfo<'info>, + /// CHECK: This account will not be checked by Anchor + pub new_authority: UncheckedAccount<'info>, + /// CHECK: This account will not be checked by Anchor + pub authority: UncheckedAccount<'info>, } #[account] @@ -145,20 +146,15 @@ pub struct Vault { } ``` -#### Use Anchor’s `Signer` account type - -However, putting this check into the instruction function muddles the separation -between account validation and instruction logic. +### Use Anchor’s Signer Account Type -Fortunately, Anchor makes it easy to perform signer checks by providing the -`Signer` account type. Simply change the `authority` account’s type in the -account validation struct to be of type `Signer`, and Anchor will check at -runtime that the specified account is a signer on the transaction. This is the -approach we generally recommend since it allows you to separate the signer check -from instruction logic. - -In the example below, if the `authority` account does not sign the transaction, -then the transaction will fail before even reaching the instruction logic. +Incorporating the +[`signer`](https://docs.rs/anchor-lang/latest/anchor_lang/accounts/signer/struct.Signer.html) +check directly within the instruction handler logic can blur the separation +between account validation and instruction handler execution. To maintain this +separation, use Anchor's `Signer` account type. By changing the `authority` +account's type to `Signer` in the validation struct, Anchor automatically checks +at runtime that the specified account signed the transaction. ```rust use anchor_lang::prelude::*; @@ -182,7 +178,8 @@ pub struct UpdateAuthority<'info> { has_one = authority )] pub vault: Account<'info, Vault>, - pub new_authority: AccountInfo<'info>, + /// CHECK: This account will not be checked by Anchor + pub new_authority: UncheckedAccount<'info>, pub authority: Signer<'info>, } @@ -193,33 +190,27 @@ pub struct Vault { } ``` -Note that when you use the `Signer` type, no other ownership or type checks are + +When you use the `Signer` type, no other ownership or type checks are performed. + -#### Use Anchor’s `#[account(signer)]` constraint - -While in most cases, the `Signer` account type will suffice to ensure an account -has signed a transaction, the fact that no other ownership or type checks are -performed means that this account can’t really be used for anything else in the -instruction. - -This is where the `signer` _constraint_ comes in handy. The `#[account(signer)]` -constraint allows you to verify the account signed the transaction, while also -getting the benefits of using the `Account` type if you wanted access to it’s -underlying data as well. - -As an example of when this would be useful, imagine writing an instruction that -you expect to be invoked via CPI that expects one of the passed in accounts to -be both a **\*\***signer**\*\*** on the transaciton and a \***\*\*\*\*\*\***data -source\***\*\*\*\*\*\***. Using the `Signer` account type here removes the -automatic deserialization and type checking you would get with the `Account` -type. This is both inconvenient, as you need to manually deserialize the account -data in the instruction logic, and may make your program vulnerable by not -getting the ownership and type checking performed by the `Account` type. - -In the example below, you can safely write logic to interact with the data -stored in the `authority` account while also verifying that it signed the -transaction. +### Using Anchor’s `#[account(signer)]` Constraint + +While the `Signer` account type is useful, it doesn't perform other ownership or +type checks, limiting its use in instruction handler logic. The +[anchor's `#[account(signer)]`](https://www.anchor-lang.com/docs/account-constraints) +constraint addresses this by verifying that the account signed the transaction +while allowing access to its underlying data. + +For example, if you expect an account to be both a signer and a data source, +using the `Signer` type would require manual deserialization, and you wouldn't +benefit from automatic ownership and type checking. Instead, the +`#[account(signer)]` constraint allows you to access the data and ensure the +account signed the transaction. + +In this example, you can safely interact with the data stored in the `authority` +account while ensuring that it signed the transaction. ```rust use anchor_lang::prelude::*; @@ -246,7 +237,8 @@ pub struct UpdateAuthority<'info> { has_one = authority )] pub vault: Account<'info, Vault>, - pub new_authority: AccountInfo<'info>, + /// CHECK: This account will not be checked by Anchor + pub new_authority: UncheckedAccount<'info>, #[account(signer)] pub authority: Account<'info, AuthState> } @@ -258,52 +250,53 @@ pub struct Vault { } #[account] pub struct AuthState{ - amount: u64, - num_depositors: u64, - num_vaults: u64 + amount: u64, + num_depositors: u64, + num_vaults: u64 } ``` ## Lab -Let’s practice by creating a simple program to demonstrate how a missing signer -check can allow an attacker to withdraw tokens that don’t belong to them. +In this lab, we'll create a simple program to demonstrate how a missing signer +check can allow an attacker to withdraw tokens that don't belong to them. This +program initializes a simplified token `vault` account and shows how the absence +of a signer check could result in the vault being drained. -This program initializes a simplified token “vault” account and demonstrates how -a missing signer check could allow the vault to be drained. +### 1. Starter -#### 1. Starter +To get started, download the starter code from the +[`starter` branch of this repository](https://github.com/solana-developers/signer-auth/tree/starter). +The starter code includes a program with two instruction handlers and the +boilerplate setup for the test file. -To get started, download the starter code from the `starter` branch of -[this repository](https://github.com/Unboxed-Software/solana-signer-auth/tree/starter). The -starter code includes a program with two instructions and the boilerplate setup -for the test file. +The `initialize_vault` instruction handler sets up two new accounts: `Vault` and +`TokenAccount`. The `Vault` account is initialized using a Program Derived +Address (PDA) and stores the address of a token account and the vault's +authority. The `vault` PDA will be the authority of the token account, enabling +the program to sign off on token transfers. -The `initialize_vault` instruction initializes two new accounts: `Vault` and -`TokenAccount`. The `Vault` account will be initialized using a Program Derived -Address (PDA) and store the address of a token account and the authority of the -vault. The authority of the token account will be the `vault` PDA which enables -the program to sign for the transfer of tokens. - -The `insecure_withdraw` instruction will transfer tokens in the `vault` +The `insecure_withdraw` instruction handler transfers tokens from the `vault` account’s token account to a `withdraw_destination` token account. However, the -`authority` account in the `InsecureWithdraw` struct has a type of -`UncheckedAccount`. This is a wrapper around `AccountInfo` to explicitly -indicate the account is unchecked. +`authority` account in the `InsecureWithdraw` struct is of type +`UncheckedAccount`, a wrapper around `AccountInfo` that explicitly indicates the +account is unchecked. -Without a signer check, anyone can simply provide the public key of the -`authority` account that matches `authority` stored on the `vault` account and -the `insecure_withdraw` instruction would continue to process. +Without a signer check, anyone can provide the public key of the `authority` +account that matches the `authority` stored on the `vault` account, and the +`insecure_withdraw` instruction handler will continue processing. -While this is somewhat contrived in that any DeFi program with a vault would be -more sophisticated than this, it will show how the lack of a signer check can -result in tokens being withdrawn by the wrong party. +Although this example is somewhat contrived, as any DeFi program with a vault +would be more sophisticated, it effectively illustrates how the lack of a signer +check can lead to unauthorized token withdrawals. ```rust use anchor_lang::prelude::*; use anchor_spl::token::{self, Mint, Token, TokenAccount}; -declare_id!("Fg6PaFpoGXkYsidMpWTK6W2BeZ7FEfcYkg476zPFsLnS"); +declare_id!("FeKh59XMh6BcN6UdekHnaFHsNH9NVE121GgDzSyYPKKS"); + +pub const DISCRIMINATOR_SIZE: usize = 8; #[program] pub mod signer_authorization { @@ -318,7 +311,7 @@ pub mod signer_authorization { 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 seeds = &[b"vault".as_ref(), &[ctx.bumps.vault]]; let signer = [&seeds[..]]; let cpi_ctx = CpiContext::new_with_signer( @@ -341,7 +334,7 @@ pub struct InitializeVault<'info> { #[account( init, payer = authority, - space = 8 + 32 + 32, + space = DISCRIMINATOR_SIZE + Vault::INIT_SPACE, seeds = [b"vault"], bump )] @@ -380,73 +373,91 @@ pub struct InsecureWithdraw<'info> { } #[account] +#[derive(Default, InitSpace)] pub struct Vault { token_account: Pubkey, authority: Pubkey, } ``` -#### 2. Test `insecure_withdraw` instruction +### 2. Test insecure_withdraw Instruction Handler -The test file includes the code to invoke the `initialize_vault` instruction -using `wallet` as the `authority` on the vault. The code then mints 100 tokens -to the `vault` token account. Theoretically, the `wallet` key should be the only -one that can withdraw the 100 tokens from the vault. +The test file includes code to invoke the `initialize_vault` instruction +handler, using `walletAuthority` as the `authority` on the vault. The code then +mints 100 tokens to the `vaultTokenAccount` token account. Ideally, only the +`walletAuthority` key should be able to withdraw these 100 tokens from the +vault. -Now, let’s add a test to invoke `insecure_withdraw` on the program to show that -the current version of the program allows a third party to in fact withdraw -those 100 tokens. +Next, we'll add a test to invoke `insecure_withdraw` on the program to +demonstrate that the current version allows a third party to withdraw those 100 +tokens. -In the test, we’ll still use the public key of `wallet` as the `authority` -account, but we’ll use a different keypair to sign and send the transaction. +In the test, we'll use the `walletAuthority` public key as the `authority` +account but sign and send the transaction with a different keypair. ```typescript -describe("signer-authorization", () => { +describe("Signer Authorization", () => { ... - it("Insecure withdraw", async () => { - const tx = await program.methods - .insecureWithdraw() - .accounts({ - vault: vaultPDA, - tokenAccount: tokenAccount.publicKey, - withdrawDestination: withdrawDestinationFake, - authority: wallet.publicKey, - }) - .transaction() - - await anchor.web3.sendAndConfirmTransaction(connection, tx, [walletFake]) - - const balance = await connection.getTokenAccountBalance( - tokenAccount.publicKey - ) - expect(balance.value.uiAmount).to.eq(0) - }) + it("performs insecure withdraw", async () => { + try { + const transaction = await program.methods + .insecureWithdraw() + .accounts({ + vault: vaultPDA, + tokenAccount: vaultTokenAccount.publicKey, + withdrawDestination: unauthorizedWithdrawDestination, + authority: walletAuthority.publicKey, + }) + .transaction(); + + await anchor.web3.sendAndConfirmTransaction(connection, transaction, [ + unauthorizedWallet, + ]); + + const tokenAccountInfo = await getAccount( + connection, + vaultTokenAccount.publicKey + ); + expect(Number(tokenAccountInfo.amount)).to.equal(0); + } catch (error) { + console.error("Insecure withdraw failed:", error); + throw error; + } + }); }) ``` -Run `anchor test` to see that both transactions will complete successfully. +Run `anchor test` to confirm that both transactions will be completed +successfully. ```bash -signer-authorization - ✔ Initialize Vault (810ms) - ✔ Insecure withdraw (405ms) +Signer Authorization + ✔ initializes vault and mints tokens (882ms) + ✔ performs insecure withdraw (435ms) ``` -Since there is no signer check for the `authority` account, the -`insecure_withdraw` instruction will transfer tokens from the `vault` token -account to the `withdrawDestinationFake` token account as long as the public key -of the`authority` account matches the public key stored on the authority field -of the `vault` account. Clearly, the `insecure_withdraw` instruction is as -insecure as the name suggests. - -#### 3. Add `secure_withdraw` instruction - -Let’s fix the problem in a new instruction called `secure_withdraw`. This -instruction will be identical to the `insecure_withdraw` instruction, except -we’ll use the `Signer` type in the Accounts struct to validate the `authority` -account in the `SecureWithdraw` struct. If the `authority` account is not a -signer on the transaction, then we expect the transaction to fail and return an -error. +The `insecure_withdraw` instruction handler demonstrates a security +vulnerability. Since there is no signer check for the `authority` account, this +handler will transfer tokens from the `vaultTokenAccount` to the +`unauthorizedWithdrawDestination`, as long as the public key of the `authority` +account matches the `walletAuthority.publicKey` stored in the `vault` account's +`authority` field. + +In the test, we use the `unauthorizedWallet` to sign the transaction, while +still specifying the `walletAuthority.publicKey` as the authority in the +instruction accounts. This mismatch between the signer and the specified +`authority` would normally cause a transaction to fail. However, due to the lack +of a proper signer check in the `insecure_withdraw` handler, the transaction +succeeds. + +### 3. Add secure_withdraw Instruction Handler + +To fix this issue, we'll create a new instruction handler called +`secure_withdraw`. This instruction handler will be identical to +`insecure_withdraw`, but we'll use the `Signer` type in the Accounts struct to +validate the authority account in the `SecureWithdraw` struct. If the +`authority` account isn't a signer on the transaction, the transaction should +fail with an error. ```rust use anchor_lang::prelude::*; @@ -461,7 +472,7 @@ pub mod signer_authorization { 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 seeds = &[b"vault".as_ref(), &[ctx.bumps.vault]]; let signer = [&seeds[..]]; let cpi_ctx = CpiContext::new_with_signer( @@ -497,73 +508,101 @@ pub struct SecureWithdraw<'info> { } ``` -#### 4. Test `secure_withdraw` instruction +### 4. Test secure_withdraw Instruction Handler + +With the new instruction handler in place, return to the test file to test the +`secureWithdraw` instruction handler. Invoke the `secureWithdraw` instruction +handler, using the `walletAuthority.publicKey` as the `authority` account, and +use the `unauthorizedWallet` keypair as the signer. Set the +`unauthorizedWithdrawDestination` as the withdraw destination. -With the instruction in place, return to the test file to test the -`secure_withdraw` instruction. Invoke the `secure_withdraw` instruction, again -using the public key of `wallet` as the `authority` account and the -`withdrawDestinationFake` keypair as the signer and withdraw destination. Since -the `authority` account is validated using the `Signer` type, we expect the -transaction to fail the signer check and return an error. +Since the `authority` account is validated using the `Signer` type, the +transaction should fail with a signature verification error. This is because the +`unauthorizedWallet` is attempting to sign the transaction, but it doesn't match +the `authority` specified in the instruction (which is +`walletAuthority.publicKey`). + +The test expects this transaction to fail, demonstrating that the secure +withdraw function properly validates the signer. If the transaction unexpectedly +succeeds, the test will throw an error indicating that the expected security +check did not occur. ```typescript -describe("signer-authorization", () => { +describe("Signer Authorization", () => { ... - it("Secure withdraw", async () => { + it("fails to perform secure withdraw with incorrect signer", async () => { try { - const tx = await program.methods + const transaction = await program.methods .secureWithdraw() .accounts({ vault: vaultPDA, - tokenAccount: tokenAccount.publicKey, - withdrawDestination: withdrawDestinationFake, - authority: wallet.publicKey, + tokenAccount: vaultTokenAccount.publicKey, + withdrawDestination: unauthorizedWithdrawDestination, + authority: walletAuthority.publicKey, }) - .transaction() - - await anchor.web3.sendAndConfirmTransaction(connection, tx, [walletFake]) - } catch (err) { - expect(err) - console.log(err) + .transaction(); + + await anchor.web3.sendAndConfirmTransaction(connection, transaction, [ + unauthorizedWallet, + ]); + throw new Error("Expected transaction to fail, but it succeeded"); + } catch (error) { + expect(error).to.be.an("error"); + console.log("Error message:", error.message); } - }) + }); }) ``` -Run `anchor test` to see that the transaction will now return a signature +Run `anchor test` to see that the transaction now returns a signature verification error. ```bash -Error: Signature verification failed +signer-authorization +Error message: Signature verification failed. +Missing signature for public key [`GprrWv9r8BMxQiWea9MrbCyK7ig7Mj8CcseEbJhDDZXM`]. + ✔ fails to perform secure withdraw with incorrect signer ``` -That’s it! This is a fairly simple thing to avoid, but incredibly important. -Make sure to always think through who should who should be authorizing -instructions and make sure that each is a signer on the transaction. +This example shows how important it is to think through who should authorize +instructions and ensure that each is a signer on the transaction. -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-signer-auth/tree/solution). +To review the final solution code, you can find it on the +[`solution` branch of the repository](https://github.com/solana-developers/signer-auth/tree/solution). ## Challenge -At this point in the course, we hope you've started to work on programs and -projects outside the labs and Challenges provided in these lessons. For this and -the remainder of the lessons on security vulnerabilities, the Challenge for each -lesson will be to audit your own code for the security vulnerability discussed -in the lesson. +Now that you've worked through the labs and challenges in this course, it's time +to apply your knowledge in a practical setting. For this challenge and those +that follow on security vulnerabilities, audit your own programs for the +specific vulnerability discussed in each lesson. + +### Steps -Alternatively, you can find open source programs to audit. There are plenty of -programs you can look at. A good start if you don't mind diving into native Rust -would be the -[SPL programs](https://github.com/solana-labs/solana-program-library). +1. **Audit Your Program or Find an Open Source Project**: -So for this lesson, take a look at a program (whether yours or one you've found -online) and audit it for signer checks. If you find a bug in somebody else's -program, please alert them! If you find a bug in your own program, be sure to -patch it right away. + - Begin by auditing your own code for missing signer checks, or find an open + source Solana program to audit. A great place to start is with the + [program examples](https://github.com/solana-developers/program-examples) + repository. + +2. **Look for Signer Check Issues**: + + - Focus on instruction handlers where signer authorization is crucial, + especially those that transfer tokens or modify sensitive account data. + - Review the program for any `UncheckedAccount` types where signer validation + should be enforced. + - Ensure that any accounts that should require user authorization are defined + as `Signer` in the instruction handler. + +3. **Patch or Report**: + - If you find a bug in your own code, fix it by using the `Signer` type for + accounts that require signer validation. + - If the issue exists in an open source project, notify the project + maintainers or submit a pull request. -Push your code to GitHub and + +After completing the challenge, push your code to GitHub and [tell us what you thought of this lesson](https://form.typeform.com/to/IPH0UGz7#answers-lesson=26b3f41e-8241-416b-9cfa-05c5ab519d80)!