Skip to content

Commit e79ade0

Browse files
onspeedhponspeedhp
committed
fix: prevent cross-wallet authority deletion in RemoveAuthority (Issue #3)
- Add wallet validation for target authority in process_remove_authority - Ensures target_header.wallet matches wallet_pda before any role checks - Prevents malicious owners from deleting authorities from other wallets - Add comprehensive E2E test coverage for cross-wallet attack scenarios - Tests verify Owner cannot remove/add authorities or execute on other wallets Security Impact: - CRITICAL: Fixes cross-wallet authority deletion vulnerability - Blocks unauthorized access to other wallet's authorities - Prevents DoS via mass authority deletion - Eliminates privilege escalation attack vector Fixes #3
1 parent 0ba7526 commit e79ade0

3 files changed

Lines changed: 221 additions & 12 deletions

File tree

‎program/src/processor/manage_authority.rs‎

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -356,19 +356,27 @@ pub fn process_remove_authority(
356356
_ => return Err(AuthError::InvalidAuthenticationKind.into()),
357357
}
358358

359-
// Authorization
360-
if admin_header.role != 0 {
361-
let target_data = unsafe { target_auth_pda.borrow_data_unchecked() };
362-
// Safe copy target header
363-
let mut target_h_bytes = [0u8; std::mem::size_of::<AuthorityAccountHeader>()];
364-
target_h_bytes
365-
.copy_from_slice(&target_data[..std::mem::size_of::<AuthorityAccountHeader>()]);
366-
let target_header =
367-
unsafe { std::mem::transmute::<&[u8; 48], &AuthorityAccountHeader>(&target_h_bytes) };
368-
if target_header.discriminator != AccountDiscriminator::Authority as u8 {
369-
return Err(ProgramError::InvalidAccountData);
370-
}
359+
// Authorization - ALWAYS validate target authority
360+
let target_data = unsafe { target_auth_pda.borrow_data_unchecked() };
361+
// Safe copy target header
362+
let mut target_h_bytes = [0u8; std::mem::size_of::<AuthorityAccountHeader>()];
363+
target_h_bytes.copy_from_slice(&target_data[..std::mem::size_of::<AuthorityAccountHeader>()]);
364+
let target_header =
365+
unsafe { std::mem::transmute::<&[u8; 48], &AuthorityAccountHeader>(&target_h_bytes) };
366+
367+
// ALWAYS verify discriminator
368+
if target_header.discriminator != AccountDiscriminator::Authority as u8 {
369+
return Err(ProgramError::InvalidAccountData);
370+
}
371371

372+
// ALWAYS verify target belongs to THIS wallet (CRITICAL SECURITY CHECK)
373+
if target_header.wallet != *wallet_pda.key() {
374+
return Err(ProgramError::InvalidAccountData);
375+
}
376+
377+
// Role-based permission check
378+
if admin_header.role != 0 {
379+
// Admin can only remove Spender
372380
if admin_header.role != 1 || target_header.role != 2 {
373381
return Err(AuthError::PermissionDenied.into());
374382
}

‎tests-e2e/src/scenarios/cross_wallet_attacks.rs‎

Lines changed: 200 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,200 @@
1+
use crate::common::{TestContext, ToAddress};
2+
use anyhow::Result;
3+
use solana_instruction::{AccountMeta, Instruction};
4+
use solana_keypair::Keypair;
5+
use solana_message::Message;
6+
use solana_pubkey::Pubkey;
7+
use solana_signer::Signer;
8+
use solana_system_program;
9+
use solana_transaction::Transaction;
10+
11+
pub fn run(ctx: &mut TestContext) -> Result<()> {
12+
println!("\n🔒 Running Cross-Wallet Attack Scenarios...");
13+
14+
// Setup: Create TWO wallets
15+
let user_seed_a = rand::random::<[u8; 32]>();
16+
let owner_a = Keypair::new();
17+
let (wallet_a, _) = Pubkey::find_program_address(&[b"wallet", &user_seed_a], &ctx.program_id);
18+
let (vault_a, _) =
19+
Pubkey::find_program_address(&[b"vault", wallet_a.as_ref()], &ctx.program_id);
20+
let (owner_a_auth, _) = Pubkey::find_program_address(
21+
&[
22+
b"authority",
23+
wallet_a.as_ref(),
24+
Signer::pubkey(&owner_a).as_ref(),
25+
],
26+
&ctx.program_id,
27+
);
28+
29+
let user_seed_b = rand::random::<[u8; 32]>();
30+
let owner_b = Keypair::new();
31+
let (wallet_b, _) = Pubkey::find_program_address(&[b"wallet", &user_seed_b], &ctx.program_id);
32+
let (vault_b, _) =
33+
Pubkey::find_program_address(&[b"vault", wallet_b.as_ref()], &ctx.program_id);
34+
let (owner_b_auth, _) = Pubkey::find_program_address(
35+
&[
36+
b"authority",
37+
wallet_b.as_ref(),
38+
Signer::pubkey(&owner_b).as_ref(),
39+
],
40+
&ctx.program_id,
41+
);
42+
43+
// Create Wallet A
44+
println!("\n[Setup] Creating Wallet A...");
45+
let mut data_a = vec![0];
46+
data_a.extend_from_slice(&user_seed_a);
47+
data_a.push(0);
48+
data_a.push(0);
49+
data_a.extend_from_slice(&[0; 6]);
50+
data_a.extend_from_slice(Signer::pubkey(&owner_a).as_ref());
51+
52+
let create_a_ix = Instruction {
53+
program_id: ctx.program_id.to_address(),
54+
accounts: vec![
55+
AccountMeta::new(Signer::pubkey(&ctx.payer).to_address(), true),
56+
AccountMeta::new(wallet_a.to_address(), false),
57+
AccountMeta::new(vault_a.to_address(), false),
58+
AccountMeta::new(owner_a_auth.to_address(), false),
59+
AccountMeta::new_readonly(solana_system_program::id().to_address(), false),
60+
],
61+
data: data_a,
62+
};
63+
64+
let message_a = Message::new(
65+
&[create_a_ix],
66+
Some(&Signer::pubkey(&ctx.payer).to_address()),
67+
);
68+
let mut create_a_tx = Transaction::new_unsigned(message_a);
69+
create_a_tx.sign(&[&ctx.payer, &owner_a], ctx.svm.latest_blockhash());
70+
71+
ctx.execute_tx(create_a_tx)?;
72+
73+
// Create Wallet B
74+
println!("[Setup] Creating Wallet B...");
75+
let mut data_b = vec![0];
76+
data_b.extend_from_slice(&user_seed_b);
77+
data_b.push(0);
78+
data_b.push(0);
79+
data_b.extend_from_slice(&[0; 6]);
80+
data_b.extend_from_slice(Signer::pubkey(&owner_b).as_ref());
81+
82+
let create_b_ix = Instruction {
83+
program_id: ctx.program_id.to_address(),
84+
accounts: vec![
85+
AccountMeta::new(Signer::pubkey(&ctx.payer).to_address(), true),
86+
AccountMeta::new(wallet_b.to_address(), false),
87+
AccountMeta::new(vault_b.to_address(), false),
88+
AccountMeta::new(owner_b_auth.to_address(), false),
89+
AccountMeta::new_readonly(solana_system_program::id().to_address(), false),
90+
],
91+
data: data_b,
92+
};
93+
94+
let message_b = Message::new(
95+
&[create_b_ix],
96+
Some(&Signer::pubkey(&ctx.payer).to_address()),
97+
);
98+
let mut create_b_tx = Transaction::new_unsigned(message_b);
99+
create_b_tx.sign(&[&ctx.payer, &owner_b], ctx.svm.latest_blockhash());
100+
101+
ctx.execute_tx(create_b_tx)?;
102+
103+
// Scenario 1: Owner A tries to Add Authority to Wallet B
104+
println!("\n[1/3] Testing Cross-Wallet Authority Addition...");
105+
let attacker_keypair = Keypair::new();
106+
let (attacker_auth_b, _) = Pubkey::find_program_address(
107+
&[
108+
b"authority",
109+
wallet_b.as_ref(),
110+
Signer::pubkey(&attacker_keypair).as_ref(),
111+
],
112+
&ctx.program_id,
113+
);
114+
115+
let mut add_cross_data = vec![1];
116+
add_cross_data.push(0);
117+
add_cross_data.push(1); // Admin
118+
add_cross_data.extend_from_slice(&[0; 6]);
119+
add_cross_data.extend_from_slice(Signer::pubkey(&attacker_keypair).as_ref());
120+
add_cross_data.extend_from_slice(Signer::pubkey(&attacker_keypair).as_ref());
121+
122+
let cross_add_ix = Instruction {
123+
program_id: ctx.program_id.to_address(),
124+
accounts: vec![
125+
AccountMeta::new(Signer::pubkey(&ctx.payer).to_address(), true),
126+
AccountMeta::new(wallet_b.to_address(), false), // Target: Wallet B
127+
AccountMeta::new(owner_a_auth.to_address(), false), // Auth: Owner A (WRONG WALLET)
128+
AccountMeta::new(attacker_auth_b.to_address(), false),
129+
AccountMeta::new_readonly(solana_system_program::id().to_address(), false),
130+
AccountMeta::new_readonly(Signer::pubkey(&owner_a).to_address(), true), // Owner A signing
131+
],
132+
data: vec![1, 1],
133+
};
134+
135+
let message_cross = Message::new(
136+
&[cross_add_ix],
137+
Some(&Signer::pubkey(&ctx.payer).to_address()),
138+
);
139+
let mut cross_add_tx = Transaction::new_unsigned(message_cross);
140+
cross_add_tx.sign(&[&ctx.payer, &owner_a], ctx.svm.latest_blockhash());
141+
142+
ctx.execute_tx_expect_error(cross_add_tx)?;
143+
println!("✅ Cross-Wallet Authority Addition Rejected.");
144+
145+
// Scenario 2: Owner A tries to Remove Owner from Wallet B
146+
println!("\n[2/3] Testing Cross-Wallet Authority Removal...");
147+
let remove_cross_ix = Instruction {
148+
program_id: ctx.program_id.to_address(),
149+
accounts: vec![
150+
AccountMeta::new(Signer::pubkey(&ctx.payer).to_address(), true),
151+
AccountMeta::new(wallet_b.to_address(), false), // Target: Wallet B
152+
AccountMeta::new(owner_a_auth.to_address(), false), // Auth: Owner A (WRONG)
153+
AccountMeta::new(owner_b_auth.to_address(), false), // Target: Owner B
154+
AccountMeta::new(Signer::pubkey(&ctx.payer).to_address(), false),
155+
AccountMeta::new_readonly(Signer::pubkey(&owner_a).to_address(), true),
156+
],
157+
data: vec![2],
158+
};
159+
160+
let message_remove = Message::new(
161+
&[remove_cross_ix],
162+
Some(&Signer::pubkey(&ctx.payer).to_address()),
163+
);
164+
let mut remove_cross_tx = Transaction::new_unsigned(message_remove);
165+
remove_cross_tx.sign(&[&ctx.payer, &owner_a], ctx.svm.latest_blockhash());
166+
167+
ctx.execute_tx_expect_error(remove_cross_tx)?;
168+
println!("✅ Cross-Wallet Authority Removal Rejected.");
169+
170+
// Scenario 3: Owner A tries to Execute on Wallet B's Vault
171+
println!("\n[3/3] Testing Cross-Wallet Execution...");
172+
let mut exec_cross_data = vec![4];
173+
exec_cross_data.push(0); // Empty compact
174+
175+
let exec_cross_ix = Instruction {
176+
program_id: ctx.program_id.to_address(),
177+
accounts: vec![
178+
AccountMeta::new(Signer::pubkey(&ctx.payer).to_address(), true),
179+
AccountMeta::new(wallet_b.to_address(), false), // Target: Wallet B
180+
AccountMeta::new(owner_a_auth.to_address(), false), // Auth: Owner A (WRONG)
181+
AccountMeta::new(vault_b.to_address(), false), // Vault B
182+
AccountMeta::new_readonly(solana_system_program::id().to_address(), false),
183+
AccountMeta::new_readonly(Signer::pubkey(&owner_a).to_address(), true),
184+
],
185+
data: exec_cross_data,
186+
};
187+
188+
let message_exec = Message::new(
189+
&[exec_cross_ix],
190+
Some(&Signer::pubkey(&ctx.payer).to_address()),
191+
);
192+
let mut exec_cross_tx = Transaction::new_unsigned(message_exec);
193+
exec_cross_tx.sign(&[&ctx.payer, &owner_a], ctx.svm.latest_blockhash());
194+
195+
ctx.execute_tx_expect_error(exec_cross_tx)?;
196+
println!("✅ Cross-Wallet Execution Rejected.");
197+
198+
println!("\n✅ All Cross-Wallet Attack Scenarios Passed!");
199+
Ok(())
200+
}

‎tests-e2e/src/scenarios/mod.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,3 @@
1+
pub mod cross_wallet_attacks;
12
pub mod failures;
23
pub mod happy_path;

0 commit comments

Comments
 (0)