Skip to content

Commit c57edcc

Browse files
authored
Merge pull request #29 from lazor-kit/fix/wallet-discriminator-check
Fix/wallet discriminator check
2 parents 17cbf43 + 622022b commit c57edcc

6 files changed

Lines changed: 85 additions & 2 deletions

File tree

program/src/processor/create_session.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,12 @@ pub fn process(
110110
return Err(ProgramError::IllegalOwner);
111111
}
112112

113+
// Validate Wallet Discriminator (Issue #7)
114+
let wallet_data = unsafe { wallet_pda.borrow_data_unchecked() };
115+
if wallet_data.is_empty() || wallet_data[0] != AccountDiscriminator::Wallet as u8 {
116+
return Err(ProgramError::InvalidAccountData);
117+
}
118+
113119
// Verify Authorizer
114120
// Check removed: conditional writable check inside match
115121

program/src/processor/execute.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use crate::{
44
},
55
compact::parse_compact_instructions,
66
error::AuthError,
7-
state::authority::AuthorityAccountHeader,
7+
state::{authority::AuthorityAccountHeader, AccountDiscriminator},
88
};
99
use pinocchio::{
1010
account_info::AccountInfo,
@@ -61,6 +61,11 @@ pub fn process(
6161
if wallet_pda.owner() != program_id || authority_pda.owner() != program_id {
6262
return Err(ProgramError::IllegalOwner);
6363
}
64+
// Validate Wallet Discriminator (Issue #7)
65+
let wallet_data = unsafe { wallet_pda.borrow_data_unchecked() };
66+
if wallet_data.is_empty() || wallet_data[0] != AccountDiscriminator::Wallet as u8 {
67+
return Err(ProgramError::InvalidAccountData);
68+
}
6469

6570
if !authority_pda.is_writable() {
6671
return Err(ProgramError::InvalidAccountData);

program/src/processor/manage_authority.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,17 @@ pub fn process_add_authority(
133133
if admin_auth_pda.owner() != program_id {
134134
return Err(ProgramError::IllegalOwner);
135135
}
136+
// Validate Wallet Discriminator (Issue #7)
137+
let wallet_data = unsafe { wallet_pda.borrow_data_unchecked() };
138+
if wallet_data.is_empty() || wallet_data[0] != AccountDiscriminator::Wallet as u8 {
139+
return Err(ProgramError::InvalidAccountData);
140+
}
141+
142+
// Validate Wallet Discriminator (Issue #7)
143+
let wallet_data = unsafe { wallet_pda.borrow_data_unchecked() };
144+
if wallet_data.is_empty() || wallet_data[0] != AccountDiscriminator::Wallet as u8 {
145+
return Err(ProgramError::InvalidAccountData);
146+
}
136147

137148
// Validate system_program is the correct System Program (audit N2)
138149
if !sol_assert_bytes_eq(

program/src/processor/transfer_ownership.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,11 @@ pub fn process(
111111
if wallet_pda.owner() != program_id || current_owner.owner() != program_id {
112112
return Err(ProgramError::IllegalOwner);
113113
}
114+
// Validate Wallet Discriminator (Issue #7)
115+
let wallet_data = unsafe { wallet_pda.borrow_data_unchecked() };
116+
if wallet_data.is_empty() || wallet_data[0] != AccountDiscriminator::Wallet as u8 {
117+
return Err(ProgramError::InvalidAccountData);
118+
}
114119

115120
// Validate system_program is the correct System Program (audit N2)
116121
if !sol_assert_bytes_eq(

tests-e2e/TEST_ISSUES.md

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,15 @@
3030
**Status**: ✅ Fixed
3131
**Fix**: Replaced hardcoded rent calculations with `Rent::minimum_balance(space)` in `create_wallet.rs` and `manage_authority.rs`. Verified by tests.
3232

33+
### Issue #8 (Validation): Wallet Discriminator Check
34+
**Status**: ✅ Fixed
35+
**Fix**: Added `wallet_data[0] == AccountDiscriminator::Wallet` check in `create_session.rs`, `manage_authority.rs`, `execute.rs`, and `transfer_ownership.rs`.
36+
**Verification**: Added `Scenario 6: Wallet Discriminator Check` in `failures.rs`. Tested passing Authority PDA as Wallet PDA (Rejected).
37+
3338
## Current Status
3439
All E2E scenarios are PASSING.
3540
- Happy Path
36-
- Failures (5/5)
41+
- Failures (6/6)
3742
- Cross Wallet (3/3)
3843
- DoS Attack
3944
- Audit Validations

tests-e2e/src/scenarios/failures.rs

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -360,5 +360,56 @@ pub fn run(ctx: &mut TestContext) -> Result<()> {
360360
ctx.execute_tx_expect_error(remove_owner_tx)?;
361361
println!("✅ Admin Removing Owner Rejected.");
362362

363+
// Scenario 6: Wallet Discriminator Check (Issue #7)
364+
// Attempt to use an Authority PDA (owned by program, but wrong discriminator) as the Wallet PDA
365+
println!("\n[6/6] Testing Wallet Discriminator Validation...");
366+
367+
// We will try to call CreateSession using the Owner Authority PDA as the "Wallet PDA"
368+
// The Owner Authority PDA is owned by the program, so it passes the owner check.
369+
// However, it has Discriminator::Authority (2), not Wallet (1), so it should fail the new check.
370+
371+
let fake_wallet_pda = owner_auth_pda; // This is actually an Authority account
372+
373+
let bad_session_keypair = Keypair::new();
374+
let (bad_session_pda, _) = Pubkey::find_program_address(
375+
&[
376+
b"session",
377+
fake_wallet_pda.as_ref(), // Derived from the "fake" wallet
378+
bad_session_keypair.pubkey().as_ref(),
379+
],
380+
&ctx.program_id,
381+
);
382+
383+
let mut bad_session_data = Vec::new();
384+
bad_session_data.push(5); // CreateSession
385+
bad_session_data.extend_from_slice(bad_session_keypair.pubkey().as_ref());
386+
bad_session_data.extend_from_slice(&(current_slot + 100).to_le_bytes());
387+
388+
let bad_discriminator_ix = Instruction {
389+
program_id: ctx.program_id.to_address(),
390+
accounts: vec![
391+
AccountMeta::new(Signer::pubkey(&ctx.payer).to_address(), true),
392+
AccountMeta::new_readonly(fake_wallet_pda.to_address(), false), // FAKE WALLET (Authority Account)
393+
AccountMeta::new_readonly(owner_auth_pda.to_address(), false), // Authorizer (Using same account as auth is technically weird but valid for this test)
394+
AccountMeta::new(bad_session_pda.to_address(), false),
395+
AccountMeta::new_readonly(solana_system_program::id().to_address(), false),
396+
AccountMeta::new_readonly(solana_sysvar::rent::ID.to_address(), false),
397+
AccountMeta::new_readonly(Signer::pubkey(&owner_keypair).to_address(), true),
398+
],
399+
data: bad_session_data,
400+
};
401+
402+
let message = Message::new(
403+
&[bad_discriminator_ix],
404+
Some(&Signer::pubkey(&ctx.payer).to_address()),
405+
);
406+
let mut bad_disc_tx = Transaction::new_unsigned(message);
407+
bad_disc_tx.sign(&[&ctx.payer, &owner_keypair], ctx.svm.latest_blockhash());
408+
409+
// Expect InvalidAccountData (which is often generic error or custom depending on implementation)
410+
// Our fix returns InvalidAccountData
411+
ctx.execute_tx_expect_error(bad_disc_tx)?;
412+
println!("✅ Invalid Wallet Discriminator Rejected.");
413+
363414
Ok(())
364415
}

0 commit comments

Comments
 (0)