Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,20 +15,20 @@ jobs:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- name: Cache cargo registry, git, bin, and target
- name: Cache cargo registry, git, bin, and target dirs
uses: actions/cache@v4
with:
path: |
~/.cargo/bin
~/.cargo/registry
~/.cargo/git
~/.cargo/bin
target
tests/target
key: ${{ runner.os }}-cargo-${{ hashFiles('**/Cargo.lock') }}
- name: Install nightly + rustfmt
run: rustup update nightly && rustup default nightly && rustup component add rustfmt clippy
- name: Install dylint
run: cargo install --locked cargo-dylint@5.0.0 dylint-link@5.0.0
run: cargo install --locked --force cargo-dylint@5.0.0 dylint-link@5.0.0
- name: Format
run: cargo fmt --check
- name: Build
Expand Down
10 changes: 10 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ cargo install cargo-dylint dylint-link
| [`direct_lamport_cpi_dos`](lints/direct_lamport_cpi_dos) |
| [`overconstrained_seed_account`](lints/overconstrained_seed_account) |
| [`unsafe_pyth_price_account`](lints/unsafe_pyth_price_account) |
| [`missing_mut_constraint`](lints/missing_mut_constraint) |

## Usage

Expand Down Expand Up @@ -68,4 +69,5 @@ cargo test ata_should_use_init_if_needed_tests
cargo test direct_lamport_cpi_dos_tests
cargo test overconstrained_seed_account_tests
cargo test unsafe_pyth_price_account_tests
cargo test missing_mut_constraint_tests
```
41 changes: 39 additions & 2 deletions anchor-lints-utils/src/mir_analyzer/account_extraction.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use clippy_utils::{source::HasSession, ty::is_type_diagnostic_item};
use rustc_middle::{
mir::{HasLocalDecls, Local},
ty::TyKind,
mir::{HasLocalDecls, Local, Place, Rvalue},
ty::{Mutability, TyKind},
};
use rustc_span::source_map::Spanned;
use rustc_span::sym;
Expand Down Expand Up @@ -183,6 +183,43 @@ impl<'cx, 'tcx> MirAnalyzer<'cx, 'tcx> {
);
account_name_and_locals.first().cloned()
}

/// Extracts the account name from a place or rvalue
pub fn account_name_from_place_or_rvalue(
&self,
place: &Place<'_>,
rvalue: &Rvalue<'_>,
) -> Option<String> {
let base_local = place.local;
let resolved = self.resolve_to_original_local(base_local, &mut HashSet::new());
if let Some(acc) = self.extract_account_name_from_local(&resolved, true) {
let name = acc
.account_name
.split('.')
.next()
.unwrap_or(&acc.account_name)
.to_string();
return Some(name);
}

if let Rvalue::Ref(_, borrow_kind, ref_place) = rvalue
&& borrow_kind.mutability() == Mutability::Mut
{
let base = ref_place.local;
let resolved = self.resolve_to_original_local(base, &mut HashSet::new());
if let Some(acc) = self.extract_account_name_from_local(&resolved, true) {
let name = acc
.account_name
.split('.')
.next()
.unwrap_or(&acc.account_name)
.to_string();
return Some(name);
}
}

None
}
}

fn push_account_name_and_return(
Expand Down
44 changes: 41 additions & 3 deletions lints/arbitrary_cpi_call/README.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# `arbitraty_cpi_call`
# `arbitrary_cpi_call`

### What it does
Identifies CPI calls made using user-controlled program IDs without validations.
Identifies CPI calls made using user-controlled program IDs (from accounts or parameters) without validations before the CPI call.

### Why is this bad?
Unvalidated program IDs in CPI calls let users to trigger arbitrary programs, leading to potential security breaches or fund loss.
Expand All @@ -12,4 +12,42 @@ To avoid heavy analysis, we skip nested function analysis when:
- **Cmps/switches threshold:** The number of program_id comparisons or if/else switches in the current function exceeds `MAX_CMPS_SWITCHES_RECURSION_THRESHOLD`.
- **If/else nesting level:** The current basic block is nested deeper than `MAX_IF_ELSE_NESTING_LEVEL` depth (number of dominating `SwitchInt` blocks).

When any one of the condition triggers, we still run CPI checks for the current function (e.g. we still report arbitrary CPI in that function). We only skip propagating validation from nested functions, so very large or deeply nested code may not get full inter-procedural analysis for now.
When any one of the condition triggers, we still run CPI checks for the current function (e.g. we still report arbitrary CPI in that function). We only skip propagating validation from nested functions, so very large or deeply nested code may not get full inter-procedural analysis for now.

### Example (worst case)

```rust
// BAD: program_id is user-controlled and never validated
pub fn invoke_unchecked_program(ctx: Context<DirectInvokeTransfer>) -> Result<()> {
use anchor_lang::solana_program::instruction::Instruction;
use anchor_lang::solana_program::program::invoke;
let instruction = Instruction {
program_id: ctx.accounts.unchecked_program.key(), // user can pass any program
accounts: vec![],
data: vec![],
};
let account_infos = vec![ctx.accounts.unchecked_program.to_account_info()];
invoke(&instruction, &account_infos)?; // CPI
Ok(())
}

// GOOD: program_id validated against a constant before CPI
pub fn invoke_validated_program(ctx: Context<DirectInvokeTransfer>) -> Result<()> {
use anchor_lang::solana_program::instruction::Instruction;
use anchor_lang::solana_program::program::invoke;
const ALLOWED_PROGRAM_ID: Pubkey = Pubkey::new_from_array([42u8; 32]);
require_keys_eq!(
ctx.accounts.unchecked_program.key(),
ALLOWED_PROGRAM_ID,
CustomError::InvalidProgram
);
let instruction = Instruction {
program_id: ctx.accounts.unchecked_program.key(),
accounts: vec![],
data: vec![],
};
let account_infos = vec![ctx.accounts.unchecked_program.to_account_info()];
invoke(&instruction, &account_infos)?; // CPI
Ok(())
}
```
2 changes: 1 addition & 1 deletion lints/arbitrary_cpi_call/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ fn analyze_arbitrary_cpi_call<'tcx>(
let mut switches: Vec<IfThen> = Vec::new();
let mut program_id_cmps: Vec<Cmp> = Vec::new();

let mut instruction_to_program_id: HashMap<Local, BasicBlock> = HashMap::new();
let mut instruction_to_program_id: HashMap<Local, (BasicBlock, Local)> = HashMap::new();

for (bb, bbdata) in mir.basic_blocks.iter_enumerated() {
for statement in &bbdata.statements {
Expand Down
18 changes: 9 additions & 9 deletions lints/arbitrary_cpi_call/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ pub fn record_instruction_creation<'tcx>(
mir_analyzer: &MirAnalyzer<'_, 'tcx>,
bb: BasicBlock,
statement: &Statement<'tcx>,
instruction_to_program_id: &mut HashMap<Local, BasicBlock>,
instruction_to_program_id: &mut HashMap<Local, (BasicBlock, Local)>,
) {
if let StatementKind::Assign(box (place, rvalue)) = &statement.kind
&& let Some(dest_local) = place.as_local()
Expand All @@ -94,7 +94,7 @@ pub fn record_instruction_creation<'tcx>(
&& let Some(program_id_local) = place.as_local()
&& mir_analyzer.is_pubkey_type(program_id_local)
{
instruction_to_program_id.insert(dest_local, bb);
instruction_to_program_id.insert(dest_local, (bb, program_id_local));
}
}

Expand All @@ -109,7 +109,7 @@ pub fn track_instruction_call<'tcx>(
bb: BasicBlock,
cpi_calls: &mut HashMap<BasicBlock, CpiCallsInfo>,
cpi_contexts: &mut HashMap<BasicBlock, CpiContextsInfo>,
instruction_to_program_id: &HashMap<Local, BasicBlock>,
instruction_to_program_id: &HashMap<Local, (BasicBlock, Local)>,
) {
let mir = mir_analyzer.mir;
let decl_ty = match mir
Expand All @@ -128,9 +128,9 @@ pub fn track_instruction_call<'tcx>(
let mut program_id_local = None;
let mut program_id_bb = None;

if let Some(&pid) = instruction_to_program_id.get(&instruction_local) {
program_id_local = Some(instruction_local);
program_id_bb = Some(pid);
if let Some(&(pid_bb, actual_pid)) = instruction_to_program_id.get(&instruction_local) {
program_id_local = Some(actual_pid);
program_id_bb = Some(pid_bb);
} else {
let mut to_check = vec![instruction_local];
let mut visited = HashSet::new();
Expand All @@ -140,9 +140,9 @@ pub fn track_instruction_call<'tcx>(
continue;
}

if let Some(&pid) = instruction_to_program_id.get(&current) {
program_id_local = Some(instruction_local);
program_id_bb = Some(pid);
if let Some(&(pid_bb, actual_pid)) = instruction_to_program_id.get(&current) {
program_id_local = Some(actual_pid);
program_id_bb = Some(pid_bb);
break;
}

Expand Down
4 changes: 2 additions & 2 deletions lints/arbitrary_cpi_call/tests/test_program/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,5 @@ workspace = "../../../../tests"
crate-type = ["cdylib"]

[dependencies]
anchor-lang = { git = "https://github.com/jamie-osec/anchor", rev = "939b843" }
anchor-spl = { git = "https://github.com/jamie-osec/anchor", rev = "939b843" }
anchor-lang = { workspace = true }
anchor-spl = { workspace = true }
118 changes: 118 additions & 0 deletions lints/arbitrary_cpi_call/tests/test_program/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -696,6 +696,98 @@ pub mod arbitrary_cpi_call_tests {
ctx.accounts.cpi_call_unsafe(amount)?;
Ok(())
}


// Case 36: Other CPI call(mint_to) with unchecked program ID - unsafe
pub fn cpi_mint_to_unchecked_program(
ctx: Context<CpiMintToAccounts>,
amount: u64,
) -> Result<()> {
let cpi_accounts = anchor_spl::token::MintTo {
mint: ctx.accounts.mint.to_account_info(),
to: ctx.accounts.to.to_account_info(),
authority: ctx.accounts.authority.to_account_info(),
};
let cpi_ctx = CpiContext::new(ctx.accounts.unchecked_program.key(), cpi_accounts);
anchor_spl::token::mint_to(cpi_ctx, amount)?; // [arbitrary_cpi_call]
Ok(())
}

// Case 37: Associated Token Program create_ata without validation - unsafe
pub fn cpi_create_ata_unchecked_program(
ctx: Context<CpiCreateAtaAccounts>,
) -> Result<()> {
use anchor_spl::associated_token::{self, Create};
let cpi_accounts = Create {
payer: ctx.accounts.payer.to_account_info(),
associated_token: ctx.accounts.associated_token.to_account_info(),
authority: ctx.accounts.authority.to_account_info(),
mint: ctx.accounts.mint.to_account_info(),
system_program: ctx.accounts.system_program.to_account_info(),
token_program: ctx.accounts.token_program.to_account_info(),
};
let cpi_ctx = CpiContext::new(ctx.accounts.unchecked_program.key(), cpi_accounts);
associated_token::create(cpi_ctx)?; // [arbitrary_cpi_call]
Ok(())
}

// Case 38: Associated Token Program create_ata with validated program ID - safe
pub fn cpi_create_ata_validated_program(
ctx: Context<CpiCreateAtaAccounts>,
) -> Result<()> {
use anchor_spl::associated_token::{self, Create};
require_keys_eq!(
ctx.accounts.unchecked_program.key(),
anchor_spl::associated_token::ID,
CustomError::InvalidProgram
);
let cpi_accounts = Create {
payer: ctx.accounts.payer.to_account_info(),
associated_token: ctx.accounts.associated_token.to_account_info(),
authority: ctx.accounts.authority.to_account_info(),
mint: ctx.accounts.mint.to_account_info(),
system_program: ctx.accounts.system_program.to_account_info(),
token_program: ctx.accounts.token_program.to_account_info(),
};
let cpi_ctx = CpiContext::new(ctx.accounts.unchecked_program.key(), cpi_accounts);
associated_token::create(cpi_ctx)?; // [safe_cpi_call]
Ok(())
}

// Case 39: Invoke CPI with unchecked program ID - unsafe
pub fn cpi_custom_program_unchecked(ctx: Context<DirectInvokeTransfer>) -> Result<()> {
use anchor_lang::solana_program::instruction::Instruction;
use anchor_lang::solana_program::program::invoke;

let instruction = Instruction {
program_id: ctx.accounts.unchecked_program.key(),
accounts: vec![],
data: vec![],
};
let account_infos = vec![ctx.accounts.unchecked_program.to_account_info()];
invoke(&instruction, &account_infos)?; // [arbitrary_cpi_call]
Ok(())
}

// Case 40: Raw invoke with validated program ID - safe
pub fn cpi_custom_program_checked(ctx: Context<DirectInvokeTransfer>) -> Result<()> {
use anchor_lang::solana_program::instruction::Instruction;
use anchor_lang::solana_program::program::invoke;
const VALIDATED_PROGRAM_ID: Pubkey = Pubkey::new_from_array([42u8; 32]); // Consider it as a constant program ID
require_keys_eq!(
ctx.accounts.unchecked_program.key(),
VALIDATED_PROGRAM_ID,
CustomError::InvalidProgram
);
let instruction = Instruction {
program_id: ctx.accounts.unchecked_program.key(),
accounts: vec![],
data: vec![],
};
let account_infos = vec![ctx.accounts.unchecked_program.to_account_info()];
invoke(&instruction, &account_infos)?; // [safe_cpi_call]
Ok(())
}
}

pub fn cpi_call_with_account<'info>(
Expand Down Expand Up @@ -1023,6 +1115,32 @@ pub struct InnerAccount {
pub data: u64,
}


#[derive(Accounts)]
pub struct CpiMintToAccounts<'info> {
pub mint: UncheckedAccount<'info>,
#[account(mut)]
pub to: UncheckedAccount<'info>,
pub authority: UncheckedAccount<'info>,
/// CHECK: Target program to validate
pub unchecked_program: UncheckedAccount<'info>,
}

#[derive(Accounts)]
pub struct CpiCreateAtaAccounts<'info> {
#[account(mut)]
pub payer: UncheckedAccount<'info>,
#[account(mut)]
pub associated_token: UncheckedAccount<'info>,
pub authority: UncheckedAccount<'info>,
pub mint: UncheckedAccount<'info>,
pub system_program: Program<'info, System>,
pub token_program: Program<'info, anchor_spl::token::Token>,
/// CHECK: Target program to validate
pub unchecked_program: UncheckedAccount<'info>,
}


#[error_code]
pub enum CustomError {
#[msg("Invalid program")]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,6 @@ workspace = "../../../../tests"
crate-type = ["cdylib"]

[dependencies]
anchor-lang = { git = "https://github.com/jamie-osec/anchor", rev = "939b843", features = ["init-if-needed"] }
anchor-spl = { git = "https://github.com/jamie-osec/anchor", rev = "939b843" }
anchor-lang = { workspace = true, features = ["init-if-needed"] }
anchor-spl = { workspace = true }

4 changes: 2 additions & 2 deletions lints/cpi_no_result/tests/test_program/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,6 @@ workspace = "../../../../tests"
crate-type = ["cdylib"]

[dependencies]
anchor-lang = { git = "https://github.com/jamie-osec/anchor", rev = "939b843" }
anchor-spl = { git = "https://github.com/jamie-osec/anchor", rev = "939b843" }
anchor-lang = { workspace = true }
anchor-spl = { workspace = true }

4 changes: 2 additions & 2 deletions lints/direct_lamport_cpi_dos/tests/test_program/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,6 @@ workspace = "../../../../tests"
crate-type = ["cdylib"]

[dependencies]
anchor-lang = { git = "https://github.com/jamie-osec/anchor", rev = "939b843" }
anchor-spl = { git = "https://github.com/jamie-osec/anchor", rev = "939b843" }
anchor-lang = { workspace = true }
anchor-spl = { workspace = true }

Loading
Loading