Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Example #2 Owner Checks #25

Open
iurage opened this issue Jan 17, 2023 · 2 comments
Open

Example #2 Owner Checks #25

iurage opened this issue Jan 17, 2023 · 2 comments

Comments

@iurage
Copy link

iurage commented Jan 17, 2023

In this example, the account was deserialized with the SplTokenAccount util and owner check is performed after. Why is this example insecure other than not deserializing with the Anchor TokenAccount struct?

@iurage
Copy link
Author

iurage commented Jan 17, 2023

I am guessing this is because there isn't a check that the account is owned by the TokenProgram?

@FrodoBaggins74524
Copy link

First, the code attempts to log a message after deserializing the provided account (token) using the SplTokenAccount::unpack utility from the spl_token crate. However, it does not verify that the token account is indeed owned by the expected token program (such as the SPL Token program).
Second, although the code performs an owner check using ctx.accounts.authority.key != &token.owner, where ctx.accounts.authority is expected to be the authorized signer, this check only verifies that the signer (ctx.accounts.authority) is the owner of the token account, not that the token account itself is a valid SPL Token account. This means that an attacker could create a malicious program that creates a fake account and sets any arbitrary owner address to pass this check.
As a result of these issues, a malicious actor could potentially pass any arbitrary account, and as long as they have control over the account (i.e., the private key corresponding to the authority), the program will not throw an error. This could lead to unintended consequences, such as reading sensitive data from unrelated accounts or performing unauthorized actions.
To ensure security, it's crucial to verify that the provided token account is a legitimate SPL Token account owned by the SPL Token program before performing any operations or checks on it. This can be achieved by checking the account's program_id field against the correct program ID for the SPL Token program. The correct program ID can be obtained using spl_token::id().
Here's an example of how to fix the code by adding the necessary checks:

use anchor_lang::prelude::*;
use anchor_lang::solana_program::program_error::ProgramError;
use anchor_lang::solana_program::program_pack::Pack;
use spl_token::state::Account as SplTokenAccount;

// ... (rest of the code)

#[program]
pub mod owner_checks_secure {
use super::*;

pub fn log_message(ctx: Context<LogMessage>) -> ProgramResult {
    let token = SplTokenAccount::unpack(&ctx.accounts.token.data.borrow())?;
    
    // Ensure the token account is owned by the correct Token program
    if *ctx.accounts.token.to_account_info().key != spl_token::id() {
        return Err(ProgramError::InvalidAccountData);
    }
    
    // Perform the authority check to ensure the signer is the owner of the token account.
    if ctx.accounts.authority.key != &token.owner {
        return Err(ProgramError::InvalidAccountData);
    }
    
    msg!("Your account balance is: {}", token.amount);
    Ok(())
}

}

// ... (rest of the code)

By adding the check to ensure the token account is owned by the correct Token program, the code becomes more secure and prevents unauthorized accounts from passing the checks. This way, we can avoid potential security vulnerabilities and maintain the integrity of the program's operations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants