526 lines
12 KiB
Markdown
526 lines
12 KiB
Markdown
# Common Vulnerability Patterns
|
|
|
|
Detailed examples of common Solana smart contract vulnerabilities with exploit scenarios and secure alternatives.
|
|
|
|
## 1. Missing Signer Validation
|
|
|
|
### Vulnerability
|
|
```rust
|
|
// ❌ VULNERABLE
|
|
pub fn withdraw(ctx: Context<Withdraw>, amount: u64) -> Result<()> {
|
|
// No check that caller is authorized!
|
|
let vault = &mut ctx.accounts.vault;
|
|
vault.balance -= amount;
|
|
Ok(())
|
|
}
|
|
|
|
#[derive(Accounts)]
|
|
pub struct Withdraw<'info> {
|
|
#[account(mut)]
|
|
pub vault: Account<'info, Vault>,
|
|
pub user: AccountInfo<'info>, // Not a Signer!
|
|
}
|
|
```
|
|
|
|
### Exploit Scenario
|
|
Attacker can drain the vault by calling `withdraw` with any account as the `user` parameter. No signature verification means anyone can execute the instruction.
|
|
|
|
### Secure Alternative
|
|
```rust
|
|
// ✅ SECURE
|
|
#[derive(Accounts)]
|
|
pub struct Withdraw<'info> {
|
|
#[account(
|
|
mut,
|
|
has_one = authority, // Ensures vault.authority == authority.key()
|
|
)]
|
|
pub vault: Account<'info, Vault>,
|
|
pub authority: Signer<'info>, // Must sign transaction
|
|
}
|
|
```
|
|
|
|
## 2. Integer Overflow/Underflow
|
|
|
|
### Vulnerability
|
|
```rust
|
|
// ❌ VULNERABLE
|
|
pub fn deposit(ctx: Context<Deposit>, amount: u64) -> Result<()> {
|
|
let vault = &mut ctx.accounts.vault;
|
|
vault.balance = vault.balance + amount; // Can overflow!
|
|
Ok(())
|
|
}
|
|
```
|
|
|
|
### Exploit Scenario
|
|
If `vault.balance = u64::MAX - 100` and `amount = 200`, the addition overflows and wraps to `99`, effectively stealing funds from the vault.
|
|
|
|
### Secure Alternative
|
|
```rust
|
|
// ✅ SECURE
|
|
pub fn deposit(ctx: Context<Deposit>, amount: u64) -> Result<()> {
|
|
let vault = &mut ctx.accounts.vault;
|
|
vault.balance = vault
|
|
.balance
|
|
.checked_add(amount)
|
|
.ok_or(ErrorCode::Overflow)?;
|
|
Ok(())
|
|
}
|
|
```
|
|
|
|
## 3. PDA Substitution Attack
|
|
|
|
### Vulnerability
|
|
```rust
|
|
// ❌ VULNERABLE
|
|
#[derive(Accounts)]
|
|
pub struct Transfer<'info> {
|
|
#[account(mut)]
|
|
pub config: Account<'info, Config>, // No PDA validation!
|
|
|
|
#[account(
|
|
mut,
|
|
seeds = [b"vault", config.key().as_ref()], // Uses unvalidated config
|
|
bump
|
|
)]
|
|
pub vault: Account<'info, Vault>,
|
|
}
|
|
```
|
|
|
|
### Exploit Scenario
|
|
Attacker creates a fake `config` account with malicious settings. The `vault` PDA is derived from this fake config, potentially accessing wrong vault or bypassing security checks.
|
|
|
|
### Secure Alternative
|
|
```rust
|
|
// ✅ SECURE
|
|
#[derive(Accounts)]
|
|
pub struct Transfer<'info> {
|
|
#[account(
|
|
seeds = [b"config"], // Global config PDA
|
|
bump,
|
|
)]
|
|
pub config: Account<'info, Config>,
|
|
|
|
#[account(
|
|
mut,
|
|
seeds = [b"vault", config.key().as_ref()],
|
|
bump
|
|
)]
|
|
pub vault: Account<'info, Vault>,
|
|
}
|
|
```
|
|
|
|
## 4. Type Cosplay Attack
|
|
|
|
### Vulnerability
|
|
```rust
|
|
// ❌ VULNERABLE
|
|
#[account(mut)]
|
|
pub user: AccountLoader<'info, User>, // Doesn't check discriminator!
|
|
```
|
|
|
|
### Exploit Scenario
|
|
Attacker passes a `UserAdmin` account instead of `User`. Since `AccountLoader` doesn't check discriminators by default, the program treats the admin account as a regular user, potentially bypassing privilege checks.
|
|
|
|
### Secure Alternative
|
|
```rust
|
|
// ✅ SECURE
|
|
#[account(mut)]
|
|
pub user: Account<'info, User>, // Enforces correct discriminator
|
|
```
|
|
|
|
## 5. Account Reloading Issues
|
|
|
|
### Vulnerability
|
|
```rust
|
|
// ❌ VULNERABLE
|
|
pub fn complex_operation(ctx: Context<ComplexOp>) -> Result<()> {
|
|
let initial_balance = ctx.accounts.vault.balance;
|
|
|
|
// CPI that modifies vault
|
|
transfer_tokens(&ctx)?;
|
|
|
|
// Still using stale balance!
|
|
require!(
|
|
ctx.accounts.vault.balance >= initial_balance,
|
|
ErrorCode::InvalidBalance
|
|
);
|
|
|
|
Ok(())
|
|
}
|
|
```
|
|
|
|
### Exploit Scenario
|
|
The `balance` value is cached from before the CPI. If the CPI modified the vault, the check uses stale data, potentially allowing invalid state transitions.
|
|
|
|
### Secure Alternative
|
|
```rust
|
|
// ✅ SECURE
|
|
pub fn complex_operation(ctx: Context<ComplexOp>) -> Result<()> {
|
|
transfer_tokens(&ctx)?;
|
|
|
|
// Reload account to get fresh data
|
|
ctx.accounts.vault.reload()?;
|
|
|
|
require!(
|
|
ctx.accounts.vault.balance >= expected_balance,
|
|
ErrorCode::InvalidBalance
|
|
);
|
|
|
|
Ok(())
|
|
}
|
|
```
|
|
|
|
## 6. Improper Account Closing
|
|
|
|
### Vulnerability
|
|
```rust
|
|
// ❌ VULNERABLE
|
|
pub fn close_account(ctx: Context<CloseAccount>) -> Result<()> {
|
|
**ctx.accounts.vault.to_account_info().lamports.borrow_mut() = 0;
|
|
// Data not zeroed, authority not reset!
|
|
Ok(())
|
|
}
|
|
```
|
|
|
|
### Exploit Scenario
|
|
Account data remains accessible within the same transaction even after lamports are zeroed. Attacker can read sensitive data or reuse the account in unexpected ways.
|
|
|
|
### Secure Alternative
|
|
```rust
|
|
// ✅ SECURE
|
|
#[derive(Accounts)]
|
|
pub struct CloseAccount<'info> {
|
|
#[account(
|
|
mut,
|
|
close = receiver // Properly closes: transfers lamports, zeros data
|
|
)]
|
|
pub vault: Account<'info, Vault>,
|
|
#[account(mut)]
|
|
pub receiver: SystemAccount<'info>,
|
|
}
|
|
```
|
|
|
|
## 7. Missing Lamports Check
|
|
|
|
### Vulnerability
|
|
```rust
|
|
// ❌ VULNERABLE
|
|
pub fn process(ctx: Context<Process>) -> Result<()> {
|
|
let data = ctx.accounts.user_data.load()?; // Can read closed account!
|
|
// ... use data
|
|
Ok(())
|
|
}
|
|
```
|
|
|
|
### Exploit Scenario
|
|
Account was closed earlier in the transaction but data is still readable. Processing closed account data can lead to inconsistent state or bypass business logic.
|
|
|
|
### Secure Alternative
|
|
```rust
|
|
// ✅ SECURE
|
|
pub fn process(ctx: Context<Process>) -> Result<()> {
|
|
require!(
|
|
**ctx.accounts.user_data.to_account_info().lamports.borrow() > 0,
|
|
ErrorCode::AccountClosed
|
|
);
|
|
|
|
let data = ctx.accounts.user_data.load()?;
|
|
// ... use data
|
|
Ok(())
|
|
}
|
|
```
|
|
|
|
## 8. Arbitrary CPI
|
|
|
|
### Vulnerability
|
|
```rust
|
|
// ❌ VULNERABLE
|
|
#[derive(Accounts)]
|
|
pub struct ArbitraryCPI<'info> {
|
|
pub token_program: AccountInfo<'info>, // Not validated!
|
|
}
|
|
|
|
pub fn transfer(ctx: Context<ArbitraryCPI>) -> Result<()> {
|
|
invoke(
|
|
&transfer_instruction,
|
|
&[
|
|
ctx.accounts.token_program.clone(), // Could be malicious!
|
|
// ...
|
|
]
|
|
)?;
|
|
Ok(())
|
|
}
|
|
```
|
|
|
|
### Exploit Scenario
|
|
Attacker passes malicious program instead of real Token program. Malicious program can emit fake events, return success without transferring, or drain funds.
|
|
|
|
### Secure Alternative
|
|
```rust
|
|
// ✅ SECURE
|
|
#[derive(Accounts)]
|
|
pub struct SecureCPI<'info> {
|
|
pub token_program: Program<'info, Token>, // Type-checked!
|
|
}
|
|
|
|
// Or manual validation
|
|
require_keys_eq!(
|
|
*ctx.accounts.token_program.key,
|
|
spl_token::ID,
|
|
ErrorCode::InvalidTokenProgram
|
|
);
|
|
```
|
|
|
|
## 9. Duplicate Mutable Accounts
|
|
|
|
### Vulnerability
|
|
```rust
|
|
// ❌ VULNERABLE
|
|
#[derive(Accounts)]
|
|
pub struct Transfer<'info> {
|
|
#[account(mut)]
|
|
pub from: Account<'info, TokenAccount>,
|
|
#[account(mut)]
|
|
pub to: Account<'info, TokenAccount>,
|
|
}
|
|
|
|
pub fn transfer(ctx: Context<Transfer>, amount: u64) -> Result<()> {
|
|
ctx.accounts.from.amount -= amount;
|
|
ctx.accounts.to.amount += amount; // Same account = double amount!
|
|
Ok(())
|
|
}
|
|
```
|
|
|
|
### Exploit Scenario
|
|
If `from` and `to` are the same account, the user can double their balance by transferring to themselves.
|
|
|
|
### Secure Alternative
|
|
```rust
|
|
// ✅ SECURE
|
|
#[derive(Accounts)]
|
|
pub struct Transfer<'info> {
|
|
#[account(
|
|
mut,
|
|
constraint = from.key() != to.key() @ ErrorCode::SameAccount
|
|
)]
|
|
pub from: Account<'info, TokenAccount>,
|
|
#[account(mut)]
|
|
pub to: Account<'info, TokenAccount>,
|
|
}
|
|
```
|
|
|
|
## 10. Bump Seed Canonicalization
|
|
|
|
### Vulnerability
|
|
```rust
|
|
// ❌ VULNERABLE
|
|
pub fn init_vault(ctx: Context<InitVault>, bump: u8) -> Result<()> {
|
|
// Accepts any bump from user!
|
|
let seeds = &[b"vault", user.key().as_ref(), &[bump]];
|
|
// Multiple PDAs possible for same seeds!
|
|
}
|
|
```
|
|
|
|
### Exploit Scenario
|
|
Attacker can create multiple vault PDAs with different bumps for the same user, fragmenting state or confusing off-chain systems.
|
|
|
|
### Secure Alternative
|
|
```rust
|
|
// ✅ SECURE
|
|
#[derive(Accounts)]
|
|
pub struct InitVault<'info> {
|
|
#[account(
|
|
init,
|
|
payer = user,
|
|
space = 8 + Vault::INIT_SPACE,
|
|
seeds = [b"vault", user.key().as_ref()],
|
|
bump // Anchor derives and stores canonical bump automatically
|
|
)]
|
|
pub vault: Account<'info, Vault>,
|
|
#[account(mut)]
|
|
pub user: Signer<'info>,
|
|
pub system_program: Program<'info, System>,
|
|
}
|
|
```
|
|
|
|
## 11. Missing Owner Check
|
|
|
|
### Vulnerability
|
|
```rust
|
|
// ❌ VULNERABLE
|
|
pub fn read_data(ctx: Context<ReadData>) -> Result<()> {
|
|
let oracle_data = ctx.accounts.oracle.try_borrow_data()?;
|
|
// No check that oracle is owned by Pyth program!
|
|
let price = parse_price(&oracle_data)?;
|
|
Ok(())
|
|
}
|
|
```
|
|
|
|
### Exploit Scenario
|
|
Attacker creates fake oracle account owned by their own program, filled with manipulated price data. Program trusts the fake data.
|
|
|
|
### Secure Alternative
|
|
```rust
|
|
// ✅ SECURE
|
|
pub fn read_data(ctx: Context<ReadData>) -> Result<()> {
|
|
require_keys_eq!(
|
|
*ctx.accounts.oracle.owner,
|
|
PYTH_PROGRAM_ID,
|
|
ErrorCode::InvalidOracleOwner
|
|
);
|
|
|
|
let oracle_data = ctx.accounts.oracle.try_borrow_data()?;
|
|
let price = parse_price(&oracle_data)?;
|
|
Ok(())
|
|
}
|
|
```
|
|
|
|
## 12. Precision Loss / Rounding Errors
|
|
|
|
### Vulnerability
|
|
```rust
|
|
// ❌ VULNERABLE
|
|
pub fn calculate_shares(collateral: u64, rate: Decimal) -> Result<u64> {
|
|
Decimal::from(collateral)
|
|
.try_div(rate)?
|
|
.try_round_u64() // Rounding can be exploited!
|
|
}
|
|
```
|
|
|
|
### Exploit Scenario
|
|
Attacker repeatedly deposits/withdraws small amounts. Rounding up gives slightly more shares each time, slowly draining the pool.
|
|
|
|
### Secure Alternative
|
|
```rust
|
|
// ✅ SECURE
|
|
pub fn calculate_shares(collateral: u64, rate: Decimal) -> Result<u64> {
|
|
Decimal::from(collateral)
|
|
.try_div(rate)?
|
|
.try_floor_u64() // Always round down in user's favor
|
|
}
|
|
```
|
|
|
|
## 13. Unchecked Error Returns
|
|
|
|
### Vulnerability
|
|
```rust
|
|
// ❌ VULNERABLE
|
|
spl_token::instruction::transfer(
|
|
token_program.key,
|
|
source.key,
|
|
destination.key,
|
|
authority.key,
|
|
&[],
|
|
amount,
|
|
); // Return value ignored!
|
|
```
|
|
|
|
### Exploit Scenario
|
|
Transfer instruction fails silently but program continues as if it succeeded. State becomes inconsistent with actual token balances.
|
|
|
|
### Secure Alternative
|
|
```rust
|
|
// ✅ SECURE
|
|
invoke(
|
|
&spl_token::instruction::transfer(
|
|
token_program.key,
|
|
source.key,
|
|
destination.key,
|
|
authority.key,
|
|
&[],
|
|
amount,
|
|
)?, // Propagates error
|
|
&[
|
|
source.clone(),
|
|
destination.clone(),
|
|
authority.clone(),
|
|
],
|
|
)?;
|
|
|
|
// Or use Anchor's CPI helpers
|
|
token::transfer(ctx, amount)?;
|
|
```
|
|
|
|
## 14. Init If Needed Vulnerability
|
|
|
|
### Vulnerability
|
|
```rust
|
|
// ❌ VULNERABLE
|
|
#[account(
|
|
init_if_needed,
|
|
payer = user,
|
|
space = 8 + Account::INIT_SPACE
|
|
)]
|
|
pub user_account: Account<'info, UserAccount>,
|
|
```
|
|
|
|
### Exploit Scenario
|
|
If account already exists, initialization is skipped but existing data might be inconsistent or malicious. Can bypass initialization checks.
|
|
|
|
### Secure Alternative
|
|
```rust
|
|
// ✅ SECURE - Explicit initialization
|
|
#[account(
|
|
init,
|
|
payer = user,
|
|
space = 8 + Account::INIT_SPACE
|
|
)]
|
|
pub user_account: Account<'info, UserAccount>,
|
|
|
|
// Or if init_if_needed is truly needed, add validation
|
|
pub fn init_or_validate(ctx: Context<InitAccount>) -> Result<()> {
|
|
if ctx.accounts.user_account.is_initialized {
|
|
// Validate existing data
|
|
require!(
|
|
ctx.accounts.user_account.owner == ctx.accounts.user.key(),
|
|
ErrorCode::InvalidOwner
|
|
);
|
|
} else {
|
|
// Initialize new account
|
|
ctx.accounts.user_account.is_initialized = true;
|
|
ctx.accounts.user_account.owner = ctx.accounts.user.key();
|
|
}
|
|
Ok(())
|
|
}
|
|
```
|
|
|
|
## 15. Stale Oracle Data
|
|
|
|
### Vulnerability
|
|
```rust
|
|
// ❌ VULNERABLE
|
|
pub fn get_price(pyth_account: &AccountInfo) -> Result<i64> {
|
|
let price_feed = load_price_feed(pyth_account)?;
|
|
Ok(price_feed.agg.price) // No staleness check!
|
|
}
|
|
```
|
|
|
|
### Exploit Scenario
|
|
Oracle stopped updating hours ago due to network issues. Attacker exploits stale price to buy/sell at favorable outdated rates.
|
|
|
|
### Secure Alternative
|
|
```rust
|
|
// ✅ SECURE
|
|
pub fn get_price(
|
|
pyth_account: &AccountInfo,
|
|
clock: &Clock
|
|
) -> Result<i64> {
|
|
let price_feed = load_price_feed(pyth_account)?;
|
|
|
|
// Check publishing time
|
|
let max_age_seconds = 60;
|
|
require!(
|
|
clock.unix_timestamp - price_feed.agg.publish_time <= max_age_seconds,
|
|
ErrorCode::StaleOraclePrice
|
|
);
|
|
|
|
// Check status
|
|
require!(
|
|
price_feed.agg.status == PriceStatus::Trading,
|
|
ErrorCode::OracleNotTrading
|
|
);
|
|
|
|
Ok(price_feed.agg.price)
|
|
}
|
|
```
|